Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FIX: Deverification logic mess on badges and admin actions #4792

Merged
merged 1 commit into from
Sep 27, 2024

Conversation

mateodaza
Copy link
Member

@mateodaza mateodaza commented Sep 27, 2024

Summary by CodeRabbit

  • New Features

    • Added a new title for GIVbacks notifications indicating reward percentages for verified public donations.
    • Enhanced messaging clarity for GIVbacks and project eligibility across multiple languages (Catalan, English, Spanish).
  • Bug Fixes

    • Improved logic for displaying eligibility messages and buttons based on GIVback status.
  • Refactor

    • Updated properties in components to reflect GIVback eligibility instead of verification status, enhancing clarity in the user interface.

Copy link

vercel bot commented Sep 27, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
giveth-dapps-v2 🔄 Building (Inspect) Visit Preview 💬 Add feedback Sep 27, 2024 4:48pm

Copy link
Contributor

coderabbitai bot commented Sep 27, 2024

Walkthrough

The changes involve updates to language files and component logic related to GIVbacks notifications and project verification status. New keys have been added for clarity, existing keys have been reformatted, and properties in components have been modified to reflect GIVback eligibility instead of verification status. These adjustments aim to enhance the consistency and clarity of messaging across multiple languages and components.

Changes

File Change Summary
lang/ca.json, lang/en.json, lang/es.json Updates to GIVbacks notification keys, including new title additions and reformatting for clarity.
src/components/VerificationBadge.tsx, src/components/views/project/ProjectBadges.tsx, src/components/views/project/ProjectGIVbackToast.tsx, src/components/views/project/projectActionCard/ProjectStats.tsx, src/components/views/userProfile/projectsTab/ProjectItem.tsx Modifications to properties passed to components, changing isVerified to isGivbackEligible and updating logic for rendering based on GIVback eligibility.

Possibly related PRs

Suggested reviewers

  • Meriem-BM
  • mohammadranjbarz
  • RamRamez

Poem

🐰 In the garden of code, changes bloom bright,
GIVbacks now clearer, a wonderful sight!
With titles and messages, all in a row,
Eligibility shines, like a soft, gentle glow.
Hopping with joy, we celebrate this spree,
A project well-tended, for all to see! 🌼


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@mateodaza mateodaza merged commit 91956a3 into develop Sep 27, 2024
2 of 3 checks passed
@mateodaza mateodaza deleted the admin-actions-fix-2 branch September 27, 2024 16:52
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Outside diff range and nitpick comments (3)
src/components/VerificationBadge.tsx (1)

14-17: LGTM! Consider simplifying the logic.

The updated logic correctly uses isGivbackEligible instead of isVerified. However, we could simplify this further for better readability:

Consider applying this change:

-	const verStatus = isGivbackEligible
-		? EVerificationStatus.VERIFIED
-		: verificationStatus !== EVerificationStatus.VERIFIED
-			? verificationStatus
-			: '';
+	const verStatus = isGivbackEligible
+		? EVerificationStatus.VERIFIED
+		: verificationStatus || '';

This simplification maintains the same logic but reduces nesting and improves clarity.

src/components/views/project/ProjectBadges.tsx (2)

30-35: Approved: Enhanced conditional logic for rendering

The updated conditional check correctly includes isGivbackEligible, ensuring that GIVback eligible projects are properly displayed. This aligns well with the PR objectives.

Consider refactoring the condition for improved readability:

-	if (
-		!verified &&
-		!isGivbackEligible &&
-		!isQF &&
-		(!campaigns || campaigns.length === 0)
-	) {
+	if (!verified && !isGivbackEligible && !isQF && !campaigns?.length) {
		return null;
	}

This refactoring simplifies the check for campaigns and makes the condition more concise.


Line range hint 1-138: Summary: Improved logic for project badges, clarification needed

The changes in this file effectively address the PR objectives by refining the logic for project verification and GIVback eligibility. The code is more clear and the conditional rendering has been improved. However, it's crucial to confirm that the new logic for the verified variable (line 28) aligns with the intended business rules for project verification.

Consider adding a comment explaining the business logic behind the verified variable to improve code maintainability and prevent future confusion.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 4f8fb2c and 9643a5e.

📒 Files selected for processing (8)
  • lang/ca.json (1 hunks)
  • lang/en.json (1 hunks)
  • lang/es.json (2 hunks)
  • src/components/VerificationBadge.tsx (1 hunks)
  • src/components/views/project/ProjectBadges.tsx (1 hunks)
  • src/components/views/project/ProjectGIVbackToast.tsx (2 hunks)
  • src/components/views/project/projectActionCard/ProjectStats.tsx (1 hunks)
  • src/components/views/userProfile/projectsTab/ProjectItem.tsx (1 hunks)
🔇 Additional comments (22)
src/components/VerificationBadge.tsx (3)

10-13: LGTM! Consistent with interface changes.

The parameter destructuring has been correctly updated to use isGivbackEligible instead of isVerified, maintaining consistency with the IProps interface changes.


Line range hint 1-45: Overall, the changes look good and align with the PR objective.

The VerificationBadge component has been successfully updated to use isGivbackEligible instead of isVerified. This change is consistent throughout the component, from the IProps interface to the internal logic. The modifications align well with the PR's goal of fixing deverification logic.

A few suggestions for improvement:

  1. Verify that all usages of VerificationBadge in the codebase have been updated to pass isGivbackEligible.
  2. Consider simplifying the verStatus determination logic as suggested earlier.

These changes enhance the component's clarity and maintainability while addressing the intended functionality updates.


6-6: LGTM! Verify usage in the codebase.

The addition of isGivbackEligible aligns with the PR objective. However, ensure that all components using VerificationBadge have been updated to pass isGivbackEligible instead of isVerified.

Run the following script to verify the prop usage:

✅ Verification successful

[/run_scripts]


VerificationBadge prop update confirmed

All instances of <VerificationBadge now use isGivbackEligible instead of isVerified.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all usages of VerificationBadge component

# Test: Search for VerificationBadge usage. Expect: Only occurrences of isGivbackEligible, no isVerified.
rg --type typescript --type tsx -A 5 $'<VerificationBadge'

Length of output: 94


Script:

#!/bin/bash
# Description: Verify all usages of VerificationBadge component

# Test: Search for VerificationBadge usage. Expect: Only occurrences of isGivbackEligible, no isVerified.
rg -g '*.ts' -g '*.tsx' -A 5 '<VerificationBadge'

Length of output: 1161

src/components/views/project/projectActionCard/ProjectStats.tsx (1)

71-71: LGTM! Consider updating related components and text.

The change from isVerified to isGivbackEligible aligns well with the PR objective of fixing the deverification logic. This update more accurately represents the project's status in terms of GIVback eligibility.

However, to ensure consistency across the application:

  1. Verify that the VerificationBadge component has been updated to handle the new isGivbackEligible prop correctly.
  2. Consider updating the tooltip text (around line 38-40) to reflect the change from verification status to GIVback eligibility.

To help verify the impact of this change across the codebase, I can run the following script:

This script will help identify other areas of the codebase that might need updates to maintain consistency with this change. Would you like me to run this script?

src/components/views/project/ProjectBadges.tsx (2)

20-25: LGTM: Improved clarity in destructuring

The changes in destructuring projectData enhance clarity by renaming verified to projectVerified and adding isGivbackEligible. This aligns well with the PR's objective of addressing deverification logic issues.


28-28: Please clarify the business logic for verification

The new verified variable combines projectVerified and isGivbackEligible. Could you please confirm if this change intentionally broadens the criteria for a project to be considered verified? It's important to ensure this aligns with the intended business logic for project verification.

lang/en.json (5)

1664-1679: Improved project status and GIVbacks messages

The new and modified entries provide more detailed and specific messages for various project statuses and GIVbacks scenarios. These changes improve clarity and user communication.


1677-1677: Flexible GIVbacks reward message

This message uses a placeholder for the percentage value, allowing for dynamic content. This approach is flexible and consistent with best practices for internationalization.


1678-1679: Clear messages with dynamic content for project status

These messages effectively communicate project vouching status and GIVbacks eligibility. The use of placeholders for dynamic values (e.g., {percent}, ${value}) allows for flexible and personalized content.


Line range hint 1680-1743: Comprehensive additions for tooltips and project categories

The new entries at the end of the file provide clear and concise explanations for various features, including stream balance, matching donations, and project verification. These additions will improve user understanding and enhance the overall user experience.


Line range hint 1-1743: Comprehensive update to English translations

This update to the English translations file is comprehensive and well-executed. The new entries and modifications provide more detailed and specific information across various features of the application. The consistent style and clear language will enhance user understanding and improve the overall user experience.

Some notable improvements include:

  1. More detailed project status and GIVbacks eligibility messages
  2. New tooltips for stream balance and donation matching
  3. Additional project categories and descriptions

These changes will contribute to a more informative and user-friendly interface.

lang/es.json (4)

1664-1666: Update translations for verified project owners

The translations for verified project owners have been updated to include more specific information about GIVbacks and project visibility. These changes improve clarity for project owners.


1667-1668: New translation for non-eligible verified projects

A new translation has been added for verified projects that are not eligible for GIVbacks. This provides clear information to users about the project's status and the benefits of boosting with GIVpower.


1677-1680: Updated translations for verified public projects

The translations for verified public projects have been updated to include more specific information about reward percentages and GIVbacks eligibility. These changes provide clearer information to potential donors.


Line range hint 1-1680: Overall improvements to Spanish translations

The changes made to this file enhance the Spanish translations by:

  1. Providing more specific information about GIVbacks and project visibility for verified project owners.
  2. Adding clear explanations for non-eligible verified projects.
  3. Updating translations for verified public projects with more detailed information about reward percentages and eligibility.

These improvements will help Spanish-speaking users better understand the GIVbacks system and project statuses on the platform.

lang/ca.json (6)

1664-1665: New translations added for verified project descriptions

These new translations provide clear descriptions for verified project owners and public viewers. They explain the benefits of boosting projects and how it affects GIVbacks percentages and project visibility.


1666-1667: New translations for non-verified project owners

These translations encourage project owners to consider if their projects are creating or supporting public goods, and explain the benefits of becoming GIVbacks eligible.


1668-1675: Various project status translations added

New translations have been added for different project statuses such as cancelled, deactivated, draft, incomplete application, and rejected. These provide clear information to project owners about their project's current state and next steps.


1676-1679: Reward percentage translations updated

The translations for reward percentages have been updated to include dynamic values. This allows for more flexible and accurate communication of reward percentages to users.


1680-1681: New translations for verified projects not eligible for GIVbacks

These translations explain that while a project has been vouched for and can benefit from GIVpower, donations to it won't generate GIVbacks for donors. This provides important clarity for both project owners and potential donors.


Line range hint 1-1679: Overall translation quality is high

The new translations and updates in this file maintain a consistent style and terminology with the existing content. They provide clear and informative messages for various project statuses, verification states, and GIVbacks eligibility scenarios. The translations effectively communicate important information to both project owners and potential donors in Catalan.

src/components/views/project/ProjectGIVbackToast.tsx (1)

95-123: ⚠️ Potential issue

Uncomment the value parameter if required by the message.

In the formatMessage call on line 103, the value parameter is commented out. If the internationalization message ${useIntlTitle}verified_public_3 requires the value parameter, please uncomment it to ensure the message displays correctly. If it's not needed, consider removing the commented line to keep the code clean.

Run the following script to verify the parameters required by the message ID verified_public_3:

✅ Verification successful

Unnecessary Parameter Removed

The value parameter is not required by the verified_public_3 message. Please remove the commented line to maintain clean code.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify if 'verified_public_3' message requires the 'value' parameter.

# Test: Search for 'verified_public_3' in localization JSON files.
# Expect: Output should indicate whether 'value' is used in the message.

rg "verified_public_3" --glob "*.json" -A 3

Length of output: 1399

@@ -91,7 +91,7 @@ const ProjectItem: FC<IProjectItem> = props => {
</P>
<div>
<VerificationBadge
isVerified={project?.isGivbackEligible}
isGivbackEligible={project?.isGivbackEligible}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Verification of VerificationBadge Prop Update Reveals Issues

The VerificationBadge component does not appear to have been updated to handle the new isGivbackEligible prop. While other components are passing this prop, the VerificationBadge itself may not be correctly receiving or utilizing it.

  • VerificationBadge Component:
    • Ensure that VerificationBadge accepts and appropriately uses the isGivbackEligible prop.
🔗 Analysis chain

Verify the impact of changing isVerified to isGivbackEligible

The prop passed to VerificationBadge has been updated from isVerified to isGivbackEligible, which aligns with the PR objective of fixing the deverification logic. This change shifts the focus from general verification to specific GIVback eligibility.

To ensure this change is implemented correctly and consistently:

  1. Confirm that the VerificationBadge component has been updated to handle the new isGivbackEligible prop correctly.
  2. Verify that the project object includes the isGivbackEligible property and that it's being set correctly.
  3. Check other components using VerificationBadge to ensure consistent prop usage across the codebase.

Run the following script to verify the changes:


Re-running Verification Scripts with Adjusted Commands

The previous verification attempt failed because the ripgrep tool did not recognize the typescript file type. Adjusting the scripts to target TypeScript files using file extensions instead.

[/run_scripts]


Re-running Verification Scripts with Updated File Patterns

The previous verification did not find any usages of VerificationBadge because .tsx files were not included in the search. Updating the glob pattern to include both .ts and .tsx files to ensure all relevant files are checked.

[/run_scripts]

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the VerificationBadge component and its usage

# Test 1: Check if VerificationBadge component has been updated
echo "Checking VerificationBadge component:"
ast-grep --lang typescript --pattern $'const VerificationBadge = ({ isGivbackEligible, $_* }) => {
  $$$
}'

# Test 2: Verify project object includes isGivbackEligible property
echo "Checking project object for isGivbackEligible property:"
rg --type typescript 'interface IProject' -A 20 | rg 'isGivbackEligible'

# Test 3: Check other usages of VerificationBadge
echo "Checking other usages of VerificationBadge:"
rg --type typescript 'VerificationBadge' -A 3 -B 1

Length of output: 605


Script:

#!/bin/bash
# Description: Verify the VerificationBadge component and its usage without specifying file types

# Test 1: Check if VerificationBadge component has been updated
echo "Checking VerificationBadge component:"
ast-grep --lang typescript --pattern $'const VerificationBadge = ({ isGivbackEligible, $_* }) => {
  $$$
}'

# Test 2: Verify project object includes isGivbackEligible property
echo "Checking project object for isGivbackEligible property:"
rg 'interface IProject' -A 20 | rg 'isGivbackEligible'

# Test 3: Check other usages of VerificationBadge
echo "Checking other usages of VerificationBadge:"
rg 'VerificationBadge' --glob '*.ts' -A 3 -B 1

Length of output: 628


Script:

#!/bin/bash
# Description: Verify the VerificationBadge component and its usage, including .tsx files

# Test 1: Check if VerificationBadge component has been updated
echo "Checking VerificationBadge component:"
ast-grep --lang typescript --pattern $'const VerificationBadge = ({ isGivbackEligible, $_* }) => {
  $$$
}'

# Test 2: Verify project object includes isGivbackEligible property
echo "Checking project object for isGivbackEligible property:"
rg 'interface IProject' | rg 'isGivbackEligible'

# Test 3: Check other usages of VerificationBadge
echo "Checking other usages of VerificationBadge:"
rg 'VerificationBadge' --glob '*.{ts,tsx}' -A 3 -B 1

Length of output: 2888

@@ -45,7 +45,7 @@ const ProjectGIVbackToast = () => {
const isGivbackEligible = projectData?.isGivbackEligible;
const { givbackFactor } = projectData || {};
const isOwnerGivbackEligible = isGivbackEligible && isAdmin;
const isOwnerNotVerified = !isVerified && isAdmin;
const isOwnerNotVerified = !isGivbackEligible && isAdmin;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider renaming isOwnerNotVerified for clarity.

The variable isOwnerNotVerified is defined as !isGivbackEligible && isAdmin;. Since it reflects the GIVback eligibility status rather than the verification status, consider renaming it to isOwnerNotGivbackEligible or isOwnerGivbackNotEligible for better clarity and to prevent confusion with isVerified.

Comment on lines +95 to +123
if (isPublicGivbackEligible) {
if (givbackFactor !== 0) {
title = formatMessage(
{
id: `${useIntlTitle}verified_public_3`,
},
{
percent: givbackFactorPercent,
// value: GIVBACKS_DONATION_QUALIFICATION_VALUE_USD,
},
);
}
description = formatMessage(
{
id: `${useIntlDescription}verified_public`,
},
{
value: GIVBACKS_DONATION_QUALIFICATION_VALUE_USD,
},
);
link = links.GIVPOWER_DOC;
Button = (
<OutlineButton
onClick={handleBoostClick}
label='Boost'
icon={<IconRocketInSpace16 />}
/>
);
} else if (isOwnerGivbackEligible) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Refactor nested conditionals to improve readability.

The block of code from lines 95 to 123 contains nested conditionals that handle various eligibility states. Consider refactoring to simplify the logic and enhance readability. One approach is to use a mapping of conditions to their corresponding title, description, link, and Button components.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: QA
Development

Successfully merging this pull request may close these issues.

1 participant