Fix referrer account id handling issues #355
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis PR updates how referral parameters are handled and improves type safety and input signatures across several wallet and pot-related modules. Changes include the introduction of a new hook and memoized logic for calculating the referral account ID, renaming query parameters from Changes
Sequence Diagram(s)sequenceDiagram
participant Q as QueryParams
participant WP as WalletProvider
participant UM as useMemo Hook
participant UE as useEffect Hook
participant SS as Session Setter
Q->>WP: Provide referrerAccountId/referrerId
WP->>UM: Compute referrerAccountIdUrlParameter
UM-->>WP: Return computed referral parameter
WP->>UE: Trigger effect with updated dependency
UE->>SS: Validate and update wallet session with new referrerAccountId
Possibly related PRs
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Pro Plan! Simply use the command ✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/features/pot-configuration/hooks/middlewares.ts (1)
19-22: Enhanced validation prevents empty transaction hash issues.Adding the check for
transactionHashes.length > 0is a good defensive programming practice that prevents empty strings from being treated as valid transaction hashes, which could lead to subtle bugs.For even more robust validation, consider using a helper function for this transaction hash validation pattern since it's also implemented in
src/features/donation/hooks/redirects.ts.src/common/wallet/components.tsx (1)
21-21: Remove console.log statementsDebug console logs should be removed before merging to production.
- console.log(query);- console.log(referrerAccountIdUrlParameter);Also applies to: 89-89
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/common/wallet/components.tsx(4 hunks)src/common/wallet/model.ts(2 hunks)src/entities/pot/hooks/forms.ts(1 hunks)src/features/donation/hooks/redirects.ts(1 hunks)src/features/matching-pool-contribution/components/MatchingPoolContributionModal.tsx(6 hunks)src/features/matching-pool-contribution/hooks/forms.ts(3 hunks)src/features/pot-application/hooks/forms.ts(0 hunks)src/features/pot-configuration/hooks/middlewares.ts(1 hunks)src/layout/pot/components/layout-hero.tsx(1 hunks)src/layout/profile/components/summary.tsx(1 hunks)
💤 Files with no reviewable changes (1)
- src/features/pot-application/hooks/forms.ts
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Fleek - broad-haze-5260
🔇 Additional comments (21)
src/layout/pot/components/layout-hero.tsx (1)
71-71: Query parameter name change improves clarity.The change from
referrerIdtoreferrerAccountIdmakes the parameter name more descriptive of what it actually contains (an account ID). This aligns with the broader renaming effort across the codebase as mentioned in the summary.src/layout/profile/components/summary.tsx (1)
39-39: Consistent parameter naming improves code maintainability.The change from
referrerIdtoreferrerAccountIdmaintains consistency with similar changes made in other files, such as src/layout/pot/components/layout-hero.tsx.src/features/donation/hooks/redirects.ts (1)
23-27: Consistent validation pattern for transaction hashes.This validation enhancement matches the one in
src/features/pot-configuration/hooks/middlewares.ts, ensuring that empty strings aren't treated as valid transaction hashes. This consistency in validation patterns across the codebase is excellent.src/entities/pot/hooks/forms.ts (1)
12-12: Simplified function signature to remove unused parameterThe
referrerIdparameter has been removed fromuseChallengeForm, streamlining the API surface. This is a good simplification since the parameter wasn't being used in the function implementation.src/common/wallet/model.ts (2)
12-12: Improved type safety by using AccountId typeChanging
referrerAccountIdfromstringtoAccountIdimproves type safety and alignment with the domain model. This ensures that only valid account IDs can be assigned to this property.
54-54: Updated method signature for type consistencyThe parameter name has been changed from
recipientAccountIdtoaccountId, which better reflects its purpose. The type has also been updated toAccountId | undefinedfor consistency with the property type.src/common/wallet/components.tsx (4)
24-29: Good backward compatibility approachAdding comments for backward compatibility with the deprecated
referrerIdparameter is a good practice. This helps other developers understand why both parameters are being handled.
31-34: Well-implemented memoization of referrer parameterUsing
useMemoto consolidate the values of bothreferrerAccountIdand the deprecatedreferrerIdparameters is an efficient approach that ensures backward compatibility.
95-103: Improved validation logic for referrer account IDThe updated logic correctly validates the referrer account ID using
isAccountIdbefore setting it, and checks it against both the current referrer account ID and the user's own account ID to avoid unnecessary updates.
104-112: Complete dependency array for useEffectThe dependency array has been correctly updated to include all variables used in the effect, which prevents potential stale closure issues.
src/features/matching-pool-contribution/components/MatchingPoolContributionModal.tsx (5)
17-17: Good refactoring to use session dataReplacing router query access with the
useWalletUserSessionhook simplifies the code and centralizes the logic for accessing user session data.Also applies to: 33-33
40-40: Fixed variable name typoThe variable name has been corrected from
yoctoMinimumAmouttoyoctoMinimumAmount.
61-63: Simplified referrer fee calculationUsing
viewer.referrerAccountIddirectly to determine if a referrer fee should be applied improves code clarity and maintainability.
174-178: Improved conditional rendering for referrer fee displayThe conditional rendering now uses
viewer.referrerAccountIdto determine whether to show the referrer fee information.
194-203: Enhanced button text displayThe button text now clearly differentiates between regular contributions and DAO proposal creations based on
viewer.isDaoRepresentative. This provides better user guidance.src/features/matching-pool-contribution/hooks/forms.ts (6)
11-11: Import change looks good.The new import statement for
useWalletUserSessioncorrectly brings in the hook that provides centralized access to user session data.
15-17: Good refactoring to use centralized session management.Streamlining the function parameters by removing
accountId,referrerId, andasDaoin favor of theviewerobject fromuseWalletUserSessionimproves code maintainability and follows the single source of truth principle.
30-30: Consistent naming for referrer ID.Replacing
referrerIdwithviewer.referrerAccountIdaligns with the PR's objective to standardize referrer parameter naming across the application.
64-65: Proper DAO representation check.Using
viewer.isDaoRepresentativeandviewer.daoAccountIdfrom the centralized session improves consistency and maintainability compared to passing these values as separate parameters.
76-81: Contract call implementation is correctly maintained.The implementation of the non-DAO path maintains the same functionality while using the centralized parameters from the viewer object.
90-96: Correctly updated dependencies array.The dependencies array has been properly updated to include all the relevant properties from the viewer object, ensuring the callback function reacts appropriately to changes in these values.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/common/api/indexer/hooks.ts(1 hunks)src/common/wallet/components.tsx(3 hunks)src/layout/pot/hooks/tab-navigation.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/common/wallet/components.tsx
🔇 Additional comments (5)
src/layout/pot/hooks/tab-navigation.ts (5)
1-2: Consider creating a separate ticket for the TODO commentThis TODO comment about storage optimizations appears to be unrelated to the current PR's focus on referrer handling. Consider creating a dedicated ticket to track this work separately.
181-185: Good improvement in tab detection logicThe changes to split the current path at "?" and compare only the base path ensures that query parameters don't affect which tab is considered active. This is a solid improvement that helps properly handle URLs with varying query parameters.
189-203: Well-implemented query parameter preservationExcellent implementation for preserving query parameters during tab navigation. The code now extracts query parameters from the current URL and appends them to the target tab's href, ensuring referrer information and other query parameters are maintained when switching tabs.
206-206: Correct dependency array updateGood addition of
currentPathto the dependency array, as it's now used within the callback function. This prevents potential stale closure issues.
212-214: Consistent path comparison for default tab navigationThe updated logic properly uses the same base path comparison technique for the default tab navigation, ensuring consistency with the other path handling changes in this file.
* Implement generalized solution for cross-field validation (#352) * Fix referrer account id handling issues (#355) * Campaign Fixes (#354) * fixed issue with start date * es lint fix * change field to number field * es lint fix --------- Co-authored-by: Carina Akaia <cvo.akaia@gmail.com> --------- Co-authored-by: Carina.Akaia.org <cvo.akaia@gmail.com>
Summary by CodeRabbit