feat: wallet card test #1275
feat: wallet card test #1275
Conversation
WalkthroughWalkthroughThis update enhances the Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant T as Test Suite
participant C as CardsWallet Component
participant CA as CashoutAddress Component
U->>T: Run Wallet tests
T->>C: Render CardsWallet with mock data
C-->>T: Return rendered component
T->>CA: Render CashoutAddress with wallet info
CA-->>T: Validate presence of elements based on KYC status
T->>U: Validate presence of all UI elements
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? 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 as PR comments)
Additionally, you can add Documentation and Community
|
✅ Deploy Preview for staging-dacade ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- tests/components/cards/Wallet.test.tsx (1 hunks)
Additional comments not posted (1)
__tests__/components/cards/Wallet.test.tsx (1)
11-25: Review of the Wallet Component Test Suite
Test Description Clarity: The test description "renders the wallet component" is clear but could be more descriptive regarding what specific aspects or states of the
Walletcomponent it tests.Mock Data Usage: Mock data for wallet, user, and referrals are used appropriately to simulate the component's environment. This is good as it isolates the test from external dependencies.
Redux State Integration: The
renderWithReduxutility is correctly used, ensuring that the component is tested within a pseudo-Redux environment. However, theas anycasting on lines 13-22 is risky as it bypasses TypeScript's type safety. It would be beneficial to define a proper type for the state object used in tests to catch potential mismatches or errors at compile time.Commented Code: There's commented code on lines 6-8 related to mocking a Redux slice. If this is not needed, it should be removed to keep the test file clean and maintainable.
Test Coverage: Currently, only the rendering of the component is tested. Consider adding more tests to cover user interactions, state transitions, or props changes to ensure comprehensive coverage.
While the test setup looks good, consider the above points for enhancing the test suite.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- tests/components/cards/Wallet.test.tsx (1 hunks)
- src/components/cards/Wallet.tsx (3 hunks)
Additional comments not posted (6)
__tests__/components/cards/Wallet.test.tsx (3)
1-5: Imports look good!The necessary modules and mock data are correctly imported.
8-11: Mock setup looks good!The jest mock for the
EditAddresscomponent is correctly implemented.
13-21: Test case looks good!The test case correctly checks if the
CardsWalletcomponent renders with all required elements.src/components/cards/Wallet.tsx (3)
23-23: OptionaltestIdprop looks good!The
testIdprop is correctly defined as an optional string.
32-32: DestructuringtestIdprop looks good!The
testIdprop is correctly destructured with a default value of'cardWalletId'.
78-78: ApplyingtestIdprop to JSX looks good!The
testIdprop is correctly applied to thedata-testidattribute of the main div.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- src/components/cards/Wallet.tsx (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- src/components/cards/Wallet.tsx
There was a problem hiding this comment.
Actionable comments posted: 3
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (10)
- mocks/fixtures/wallet.ts (1 hunks)
- tests/components/cards/Wallet.test.tsx (1 hunks)
- jest.config.js (1 hunks)
- jest.polyfills.js (2 hunks)
- src/components/cards/wallet/CashoutAddress.tsx (1 hunks)
- src/components/cards/wallet/Overview.tsx (1 hunks)
- src/components/cards/wallet/WalletHint.tsx (1 hunks)
- src/components/cards/wallet/index.tsx (1 hunks)
- src/components/sections/profile/modals/Payout.tsx (3 hunks)
- src/pages/profile/wallets.tsx (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- tests/components/cards/Wallet.test.tsx
Additional context used
Biome
src/components/cards/wallet/CashoutAddress.tsx
[error] 78-78: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
Additional comments not posted (20)
__mocks__/fixtures/wallet.ts (1)
21-21: Verify the impact of the updated address.The address field in
mockWallethas been updated. Ensure that this change does not break any tests or functionality relying on this mock data.Verification successful
The updated address in
mockWalletdoes not break any tests or functionality.The test case in
Wallet.test.tsxdoes not directly assert on theaddressfield ofmockWallet, so the updated address should not impact the test results.
__tests__/components/cards/Wallet.test.tsxScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the updated address in mockWallet does not break any tests or functionality. # Test: Run all tests to ensure they pass after the address update. npm testLength of output: 81
Script:
#!/bin/bash # Search for occurrences of `mockWallet` in the codebase to identify where this mock data is used. rg -A 5 'mockWallet'Length of output: 2027
Script:
#!/bin/bash # Search for test cases in Wallet.test.tsx to inspect how mockWallet is being utilized. rg 'test\(' __tests__/components/cards/Wallet.test.tsx -A 10Length of output: 62
Script:
#!/bin/bash # Search for all function definitions in Wallet.test.tsx to inspect how mockWallet is being utilized. ast-grep --lang typescript --pattern 'function $_($_) { $$$ }' __tests__/components/cards/Wallet.test.tsxLength of output: 107
Script:
#!/bin/bash # Search for occurrences of `mockWallet` in Wallet.test.tsx to inspect how it is being utilized. rg 'mockWallet' __tests__/components/cards/Wallet.test.tsx -A 10Length of output: 860
src/components/cards/wallet/WalletHint.tsx (1)
1-4: Verify localization keys.Ensure that the localization key
profile.wallet.payout.textis correctly defined in the translation files.Verification successful
Localization key is correctly defined and utilized.
The localization key
profile.wallet.payout.textis correctly defined in multiple translation files and is used in theWalletHint.tsxcomponent.
public/locales/hr/common.jsonpublic/locales/es/common.jsonpublic/locales/en/common.jsonpublic/locales/bg/common.jsonScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the localization key `profile.wallet.payout.text` is defined in the translation files. # Test: Search for the localization key in the translation files. rg 'profile\.wallet\.payout\.text'Length of output: 888
jest.polyfills.js (1)
24-24: Verify the impact of addingclearImmediate.The
clearImmediatefunction has been added to the global scope. Ensure that this addition does not introduce any conflicts or issues.Verification successful
No conflicts found with adding
clearImmediateto the global scope.The addition of
clearImmediateto the global scope injest.polyfills.jsdoes not introduce any conflicts or issues. It is defined and added in a consistent manner with other polyfills.
jest.polyfills.js:
clearImmediateis added to the global scope without conflicts.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the addition of `clearImmediate` to the global scope does not introduce any conflicts or issues. # Test: Run all tests to ensure they pass after adding `clearImmediate`. npm testLength of output: 81
Script:
#!/bin/bash # Search for existing usages of `clearImmediate` in the codebase rg 'clearImmediate' -A 5 # Check if `clearImmediate` is being added to the global scope in jest.polyfills.js rg 'global.clearImmediate' jest.polyfills.js -A 5Length of output: 453
Script:
#!/bin/bash # Search for other polyfills or global definitions in jest.polyfills.js rg 'Object.defineProperties(globalThis' jest.polyfills.js -A 10Length of output: 174
Script:
#!/bin/bash # Search for other polyfills or global definitions in jest.polyfills.js with corrected regex pattern rg 'Object\.defineProperties\(globalThis' jest.polyfills.js -A 10Length of output: 762
jest.config.js (1)
31-31: LGTM!The addition of the alias mapping for
@/to themoduleNameMapperobject is a good practice. It enhances the flexibility of module imports in tests and aligns with the project's structure.src/components/cards/wallet/Overview.tsx (3)
1-6: LGTM!The imports are well-organized and necessary for the
Overviewcomponent. The usage of the@/alias is consistent with the new alias mapping injest.config.js.
7-10: LGTM!The
OverviewPropsinterface is well-defined, including the optionaltestIdprop which enhances testability.
12-37: LGTM!The
Overviewcomponent is well-structured, making good use of theuseTranslationhook and rendering the wallet details effectively. The inclusion of thetestIdprop for testing purposes is a good practice.src/components/cards/wallet/index.tsx (3)
1-10: LGTM!The imports are well-organized and necessary for the
CardsWalletcomponent. The usage of the@/alias is consistent with the new alias mapping injest.config.js.
11-18: LGTM!The
CardsWalletPropsinterface is well-defined, including the optionaltestIdprop which enhances testability.
20-47: LGTM!The
CardsWalletcomponent is well-structured, making good use of hooks and rendering the wallet card with modals effectively. The inclusion of thetestIdprop for testing purposes is a good practice.src/pages/profile/wallets.tsx (1)
9-9: Verify the correctness of the import path.The import path for the
Walletcomponent has been changed from@/components/cards/Walletto@/components/cards/wallet. Ensure that the new path is correct and does not break the functionality.src/components/sections/profile/modals/Payout.tsx (2)
23-23: LGTM!The addition of the optional
testIdproperty to thePayoutPropsinterface is correctly defined and enhances the component's testability.
38-38: LGTM!The
Payoutcomponent has been correctly updated to include thetestIdproperty in its parameter list and its usage within the returned JSX, enhancing the component's testability.src/components/cards/wallet/CashoutAddress.tsx (7)
1-8: LGTM!The import statements are correctly defined and include all necessary modules for the
CashoutAddresscomponent.
11-17: LGTM!The
CashoutAddressPropsinterface is correctly defined and includes all necessary properties, including the optionaltestIdproperty.
28-32: LGTM!The
triggerEditAddressfunction is correctly defined and handles the necessary actions to set the current wallet and show the edit modal.
34-37: LGTM!The
triggerCashoutfunction is correctly defined and handles the necessary actions to show the payout modal and toggle body scrolling.
39-49: LGTM!The
triggerKYCVerificationfunction is correctly defined and handles the necessary actions to open the verification modal with specific parameters.
50-53: LGTM!The
cashoutfunction is correctly defined and handles the necessary actions based on the user's KYC verification status.
56-74: LGTM!The conditionally rendered elements for cashable wallets are correctly defined and handle the necessary functionality for address display and edit.
| interface HintProps { | ||
| wallet: Wallet; | ||
| } |
There was a problem hiding this comment.
Add prop types validation.
Consider adding prop types validation to ensure that the wallet prop conforms to the expected shape.
import PropTypes from 'prop-types';
interface HintProps {
wallet: Wallet;
}
WalletHint.propTypes = {
wallet: PropTypes.shape({
payouts: PropTypes.arrayOf(
PropTypes.shape({
amount: PropTypes.number.isRequired,
token: PropTypes.string.isRequired,
})
).isRequired,
}).isRequired,
};| triggerCashout(); | ||
| }; | ||
| return ( | ||
| <div className="px-7 pt-6 flex-1 pb-24 lg:pb-24" data-testId={testId}> |
There was a problem hiding this comment.
Fix typo in data-testid attribute.
The data-testid attribute should be written in lowercase to ensure consistency and avoid potential issues.
- <div className="px-7 pt-6 flex-1 pb-24 lg:pb-24" data-testId={testId}>
+ <div className="px-7 pt-6 flex-1 pb-24 lg:pb-24" data-testid={testId}>Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div className="px-7 pt-6 flex-1 pb-24 lg:pb-24" data-testId={testId}> | |
| <div className="px-7 pt-6 flex-1 pb-24 lg:pb-24" data-testid={testId}> |
| <div className="prose"> | ||
| <p | ||
| dangerouslySetInnerHTML={{ | ||
| __html: t("profile.wallets.uncashable", { | ||
| token: `${wallet.title}`, | ||
| link: "https://discord.gg/5yDZvVnpQQ", | ||
| }), | ||
| }} | ||
| /> | ||
| </div> |
There was a problem hiding this comment.
Avoid using dangerouslySetInnerHTML.
Using dangerouslySetInnerHTML can expose users to cross-site scripting (XSS) attacks. Consider using a safer alternative to render the message.
- <p
- dangerouslySetInnerHTML={{
- __html: t("profile.wallets.uncashable", {
- token: `${wallet.title}`,
- link: "https://discord.gg/5yDZvVnpQQ",
- }),
- }}
- />
+ <p>
+ {t("profile.wallets.uncashable", {
+ token: `${wallet.title}`,
+ link: <a href="https://discord.gg/5yDZvVnpQQ">https://discord.gg/5yDZvVnpQQ</a>,
+ })}
+ </p>Committable suggestion was skipped due to low confidence.
Tools
Biome
[error] 78-78: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (5)
__tests__/components/cards/wallet/Overview.test.tsx (1)
12-20: Add descriptive messages to expect statements.Adding descriptive messages to the
expectstatements will make the test output more informative and easier to debug.- expect(screen.getByTestId("overviewId")).toBeInTheDocument(); - expect(screen.getByText(wallet.title)).toBeInTheDocument(); - expect(screen.getByText("profile.wallets.balance")).toBeInTheDocument(); - expect(screen.getByTestId("currencyId")).toBeInTheDocument(); - expect(screen.getByTestId("coin")).toBeInTheDocument(); - expect(screen.getByTestId("tag")).toBeInTheDocument(); + expect(screen.getByTestId("overviewId")).toBeInTheDocument("Overview component should be in the document"); + expect(screen.getByText(wallet.title)).toBeInTheDocument("Wallet title should be in the document"); + expect(screen.getByText("profile.wallets.balance")).toBeInTheDocument("Balance text should be in the document"); + expect(screen.getByTestId("currencyId")).toBeInTheDocument("Currency component should be in the document"); + expect(screen.getByTestId("coin")).toBeInTheDocument("Coin component should be in the document"); + expect(screen.getByTestId("tag")).toBeInTheDocument("Tag component should be in the document");__tests__/components/cards/wallet/indext.test.tsx (1)
17-23: Add descriptive messages to expect statements.Adding descriptive messages to the
expectstatements will make the test output more informative and easier to debug.- expect(screen.getByTestId("cardWalletId")).toBeInTheDocument(); - expect(screen.getByTestId("overviewId")).toBeInTheDocument(); - expect(screen.getByTestId("cashoutAddressId")).toBeInTheDocument(); - expect(screen.queryByText("profile.wallet.payout.text")).not.toBeNull(); + expect(screen.getByTestId("cardWalletId")).toBeInTheDocument("Card Wallet component should be in the document"); + expect(screen.getByTestId("overviewId")).toBeInTheDocument("Overview component should be in the document"); + expect(screen.getByTestId("cashoutAddressId")).toBeInTheDocument("Cashout Address component should be in the document"); + expect(screen.queryByText("profile.wallet.payout.text")).not.toBeNull("Payout text should not be null");__tests__/components/cards/wallet/WalletHint.test.tsx (3)
16-20: Add descriptive messages to expect statements.Adding descriptive messages to the
expectstatements will make the test output more informative and easier to debug.- expect(textElements).toHaveLength(wallet.payouts.length); + expect(textElements).toHaveLength(wallet.payouts.length, "Number of Hint components should match the number of payouts");
22-26: Add descriptive messages to expect statements.Adding descriptive messages to the
expectstatements will make the test output more informative and easier to debug.- expect(hints).toHaveLength(wallet.payouts.length); + expect(hints).toHaveLength(wallet.payouts.length, "Number of Hint components should match the number of payouts");
28-35: Add descriptive messages to expect statements.Adding descriptive messages to the
expectstatements will make the test output more informative and easier to debug.- expect(currencyElements).toHaveLength(wallet.payouts.length); - wallet.payouts.forEach((payout, index) => { - const element = currencyElements[index]; - expect(element).toHaveTextContent(new RegExp(`${payout.token}`)); - }); + expect(currencyElements).toHaveLength(wallet.payouts.length, "Number of Currency components should match the number of payouts"); + wallet.payouts.forEach((payout, index) => { + const element = currencyElements[index]; + expect(element).toHaveTextContent(new RegExp(`${payout.token}`), `Currency component at index ${index} should contain text ${payout.token}`); + });
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (3)
- tests/components/cards/wallet/Overview.test.tsx (1 hunks)
- tests/components/cards/wallet/WalletHint.test.tsx (1 hunks)
- tests/components/cards/wallet/indext.test.tsx (1 hunks)
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (3)
- mocks/fixtures/wallet.ts (1 hunks)
- tests/components/cards/wallet/CashoutAddress.test.tsx (1 hunks)
- tests/components/cards/wallet/indext.test.tsx (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- mocks/fixtures/wallet.ts
- tests/components/cards/wallet/indext.test.tsx
Additional comments not posted (5)
__tests__/components/cards/wallet/CashoutAddress.test.tsx (5)
1-9: Imports look good.The import statements are appropriate and necessary for the test file.
11-13: Jest mocks are correctly set up.The jest mocks for
useTypedDispatchandkyc.sliceare correctly set up.Also applies to: 24-30
15-22: Mock data is well-structured.The mock data for
walletMock,mockUserStore, andverifiedUserStoreis well-structured and appropriate for the tests.Also applies to: 32-43, 45-59
60-60: Address formatting is appropriate.The address is formatted by splitting it into chunks of 4 characters, which is a good approach for readability.
62-110: Test cases are comprehensive and well-written.The test cases cover various scenarios, including rendering with props, triggering modals, and handling user interactions.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- tests/components/cards/wallet/indext.test.tsx (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- tests/components/cards/wallet/indext.test.tsx
| }; | ||
| }); | ||
|
|
||
| const mockUserStore = { |
There was a problem hiding this comment.
This is a fixtured instead:
| const mockUserStore = { | |
| const FixtureUserStore = { |
| describe("CashoutAddress component", () => { | ||
| let setShowEditModal: () => void; | ||
| let setShowPayoutModal: () => void; | ||
| const mockDispatch = jest.fn(); |
There was a problem hiding this comment.
This is already mocked I guess
| beforeEach(() => { | ||
| setShowEditModal = jest.fn(); | ||
| setShowPayoutModal = jest.fn(); | ||
| (useDispatch as jest.Mock).mockReturnValue(mockDispatch); |
There was a problem hiding this comment.
You already mocked useDispatch; why again redoing it before each test?
There was a problem hiding this comment.
The code in here is not formatted
| expect(textElements).toHaveLength(wallet.payouts.length); | ||
| }); | ||
|
|
||
| it("renders the correct number of Hint components", () => { | ||
| render(<WalletHint wallet={wallet} />); | ||
| const hints = screen.getAllByText("profile.wallet.payout.text"); | ||
| expect(hints).toHaveLength(wallet.payouts.length); |
There was a problem hiding this comment.
It good already that you are checking the length; it might also be better if you can check the amount and the token is well rendered in the component
| <div className="text-sm text-gray-700"> | ||
| {address ? ( | ||
| <p className="leading-5 text-sm flex gap-x-2 gap-y-1 flex-wrap font-mono font-normal"> | ||
| {address.map((part, k) => ( |
There was a problem hiding this comment.
| {address.map((part, k) => ( | |
| {address.map((part, i) => ( |
| {address ? ( | ||
| <p className="leading-5 text-sm flex gap-x-2 gap-y-1 flex-wrap font-mono font-normal"> | ||
| {address.map((part, k) => ( | ||
| <span key={`address-${k}`} className="mr-2"> |
There was a problem hiding this comment.
| <span key={`address-${k}`} className="mr-2"> | |
| <span key={`address-${i}`} className="mr-2"> |
There was a problem hiding this comment.
Would you refactor this component in way that we have less of if else ? : ? : ? :
There was a problem hiding this comment.
we don't have any if else, we are using a ternary operator to avoid many if conditions
| import Hint from "@/components/ui/Hint"; | ||
| import { useTranslation } from "next-i18next"; | ||
|
|
||
| interface HintProps { |
There was a problem hiding this comment.
I think we use type for components props
There was a problem hiding this comment.
We are using interface in all components, I think we can stick to same approach
| export default function WalletHint({ wallet }: HintProps) { | ||
| const { t } = useTranslation(); | ||
| return ( | ||
| <> |
There was a problem hiding this comment.
There is no need of fragements
There was a problem hiding this comment.
we need them to return single JSX
Submit a pull request
Pull Request Details
Breaking Changes
none
Issues Fixed
#1108
Other Relevant Information