Merged
Conversation
📝 WalkthroughWalkthroughThis PR introduces centralized metadata management and length validation for the memes submission form. A new Changes
Sequence DiagramsequenceDiagram
actor User
participant Form as AdditionalInfoStep
participant Validation as submissionMetadata<br/>(Validation Logic)
participant Components as Form Field<br/>Components
participant Hints as MetadataLength<br/>Hint UI
participant Hook as useArtworkSubmission<br/>Mutation
User->>Form: Enter submission data (traits, commentary, etc.)
activate Form
Form->>Validation: useMemo: compute metadata & validate lengths
activate Validation
Validation->>Validation: buildSubmissionMetadata(traits, operationalData)
Validation->>Validation: getSubmissionMetadataLengthValidation()
Validation-->>Form: MetadataLengthValidationResult (statuses per field)
deactivate Validation
Form->>Components: Pass length-status props<br/>(airdropLengthStatus, paymentInfoLengthStatus, etc.)
activate Components
Components->>Hints: Render MetadataLengthHint
activate Hints
Hints-->>Components: Display character count & warnings/errors
deactivate Hints
deactivate Components
User->>Form: Click Submit
Form->>Form: Check validation.hasErrors
alt Metadata exceeds limits
Form-->>User: Show error (prevent submission)
else Valid
Form->>Hook: submitArtwork()
activate Hook
Hook->>Validation: Final validation before API call
Hook->>Hook: buildSubmissionMetadata() for payload
Hook-->>User: Submit to API
deactivate Hook
end
deactivate Form
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 137cfcc509
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
components/waves/memes/submission/ui/MetadataLengthHint.tsx (1)
31-35: Consider adding accessibility attributes to the warning state.The error state correctly uses
role="alert"andaria-live="polite"for screen reader announcements, but the warning state lacks these attributes. Users relying on assistive technology won't be notified when approaching the character limit.♻️ Proposed fix
return ( - <p className={`tw-text-amber-400 ${baseClassName}`}> + <p + role="status" + aria-live="polite" + className={`tw-text-amber-400 ${baseClassName}`} + > {status.length}/{status.maxLength} characters. </p> );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/memes/submission/ui/MetadataLengthHint.tsx` around lines 31 - 35, The warning paragraph in MetadataLengthHint is missing screen-reader announcement attributes; update the JSX returned by the warning branch (the <p className={`tw-text-amber-400 ${baseClassName}`}> that renders {status.length}/{status.maxLength}) to include role="alert" and aria-live="polite" so assistive tech is notified when the user approaches the limit; keep the same classes and content and avoid changing the error branch (which already has these attributes).__tests__/components/waves/memes/submission/utils/submissionMetadata.test.ts (1)
10-14: Type assertion may hide missing required fields.Using
as TraitsDatabypasses type checking for required fields. IfTraitsDatahas mandatory fields beyondtitleanddescription, this test helper won't catch regressions when fields are added.💡 Consider using a more complete fixture or satisfies operator
If TraitsData has many required fields, consider creating a complete fixture or importing a factory function that provides all defaults:
// Option 1: Use satisfies for partial validation (if TraitsData allows partials) const buildTraits = (): TraitsData => ({ title: "Title", description: "Description", // ... add other required fields with defaults }); // Option 2: Import a shared test fixture factory import { createMockTraitsData } from "../fixtures/traitsData"; const buildTraits = () => createMockTraitsData({ title: "Title", description: "Description" });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/components/waves/memes/submission/utils/submissionMetadata.test.ts` around lines 10 - 14, The test helper buildTraits currently uses a type assertion ("as TraitsData") which hides missing required fields; update buildTraits to return a fully-typed TraitsData without assertions by either (a) populating all required fields on the returned object to match the TraitsData shape (replace the "as TraitsData" with a real object literal containing defaults for any missing required properties), or (b) use an imported factory/fixture (e.g., createMockTraitsData) and call it like buildTraits = () => createMockTraitsData({ title: "Title", description: "Description" }); alternatively you can use the TypeScript satisfies operator to ensure the literal conforms to TraitsData (e.g., const obj = { ... } satisfies TraitsData) so the compiler will catch missing fields; reference the buildTraits function and the TraitsData type when making the change.__tests__/components/waves/memes/submission/MemesArtSubmissionContainer.test.tsx (1)
205-222: Strengthen submit-path assertion by checking mutation invocation.
expect(res).toBeTruthy()is weak on its own; verifysubmitArtworkis actually called to catch wiring regressions.Suggested assertion hardening
const res = await artworkProps.onSubmit(); expect(res).toBeTruthy(); + expect(submitArtwork).toHaveBeenCalledTimes(1);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/components/waves/memes/submission/MemesArtSubmissionContainer.test.tsx` around lines 205 - 222, The test currently only asserts the truthiness of the submit result; also assert that the mocked mutation handler was invoked to ensure the submit path is wired: after calling artworkProps.handleFileSelect(file) and awaiting artworkProps.onSubmit(), add an assertion such as expect(submitArtwork).toHaveBeenCalled() (or expect(submitArtwork).toHaveBeenCalledTimes(1) and/or expect(submitArtwork).toHaveBeenCalledWith(expect.objectContaining({/* file or payload */})) ) to verify that the submitArtwork mock returned by mockMutation was actually called.__tests__/components/waves/memes/submission/steps/AdditionalInfoStep.test.tsx (1)
48-57: Add a boundary test for exactly 5000 characters.This covers overflow well; add a companion case at the exact limit to guard against accidental
>=/>regressions in disable logic.Suggested test addition
+ it("keeps preview and submit enabled at exactly 5000 chars", () => { + render( + <AdditionalInfoStep {...baseProps} artworkCommentary={"x".repeat(5000)} /> + ); + + expect(screen.getByRole("button", { name: "Preview" })).toBeEnabled(); + expect( + screen.getByRole("button", { name: "Submit Artwork" }) + ).toBeEnabled(); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/components/waves/memes/submission/steps/AdditionalInfoStep.test.tsx` around lines 48 - 57, Add a boundary test alongside the existing one to verify behavior at exactly 5000 characters: render AdditionalInfoStep with {...baseProps} and artworkCommentary={"x".repeat(5000)} and assert that the "Preview" and "Submit Artwork" buttons are not disabled (use getByRole with name "Preview" and "Submit Artwork" and expect(...).not.toBeDisabled()). This ensures the disable logic treats 5000 as allowed rather than blocked.components/waves/memes/submission/steps/AdditionalInfoStep.tsx (1)
88-89: Consider adding a clarifying comment for the hardcoded empty array.
artist_profile_mediais hardcoded as an empty array. If this field is intentionally not validated in this step (because it's managed elsewhere), a brief comment would help future maintainers understand this is by design rather than an oversight.📝 Suggested clarification
additional_media: { + // artist_profile_media is managed in a separate step and not validated here artist_profile_media: [], artwork_commentary_media: supportingMedia,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/memes/submission/steps/AdditionalInfoStep.tsx` around lines 88 - 89, Add a brief clarifying comment where the object literal sets additional_media.artist_profile_media = [] in AdditionalInfoStep (or the variable/prop named additional_media) explaining that the empty array is intentional and validated/managed elsewhere (e.g., "artist_profile_media intentionally left empty here because X is handled in Y") so future maintainers don't treat it as an oversight.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@__tests__/components/waves/memes/submission/utils/submissionMetadata.test.ts`:
- Around line 3-8: The test imports METADATA_VALUE_WARNING_THRESHOLD but that
symbol isn't exported from submissionMetadata; update the module that defines
METADATA_VALUE_WARNING_THRESHOLD so it is exported (i.e., export the existing
constant) so the test import succeeds; locate the constant near
buildSubmissionMetadata and getSubmissionMetadataLengthValidation in
submissionMetadata and add an export for METADATA_VALUE_WARNING_THRESHOLD (or
alternatively remove its use from tests if intentional).
In `@components/waves/memes/submission/utils/submissionMetadata.ts`:
- Around line 7-8: Export the missing constant METADATA_VALUE_WARNING_THRESHOLD
so tests can import it; update the declaration of
METADATA_VALUE_WARNING_THRESHOLD in submissionMetadata.ts to be exported
(similar to METADATA_VALUE_MAX_LENGTH) and keep its numeric value unchanged,
ensuring the symbol name METADATA_VALUE_WARNING_THRESHOLD is available for
import in the test suite and any other modules that depend on it.
---
Nitpick comments:
In
`@__tests__/components/waves/memes/submission/MemesArtSubmissionContainer.test.tsx`:
- Around line 205-222: The test currently only asserts the truthiness of the
submit result; also assert that the mocked mutation handler was invoked to
ensure the submit path is wired: after calling
artworkProps.handleFileSelect(file) and awaiting artworkProps.onSubmit(), add an
assertion such as expect(submitArtwork).toHaveBeenCalled() (or
expect(submitArtwork).toHaveBeenCalledTimes(1) and/or
expect(submitArtwork).toHaveBeenCalledWith(expect.objectContaining({/* file or
payload */})) ) to verify that the submitArtwork mock returned by mockMutation
was actually called.
In
`@__tests__/components/waves/memes/submission/steps/AdditionalInfoStep.test.tsx`:
- Around line 48-57: Add a boundary test alongside the existing one to verify
behavior at exactly 5000 characters: render AdditionalInfoStep with
{...baseProps} and artworkCommentary={"x".repeat(5000)} and assert that the
"Preview" and "Submit Artwork" buttons are not disabled (use getByRole with name
"Preview" and "Submit Artwork" and expect(...).not.toBeDisabled()). This ensures
the disable logic treats 5000 as allowed rather than blocked.
In
`@__tests__/components/waves/memes/submission/utils/submissionMetadata.test.ts`:
- Around line 10-14: The test helper buildTraits currently uses a type assertion
("as TraitsData") which hides missing required fields; update buildTraits to
return a fully-typed TraitsData without assertions by either (a) populating all
required fields on the returned object to match the TraitsData shape (replace
the "as TraitsData" with a real object literal containing defaults for any
missing required properties), or (b) use an imported factory/fixture (e.g.,
createMockTraitsData) and call it like buildTraits = () =>
createMockTraitsData({ title: "Title", description: "Description" });
alternatively you can use the TypeScript satisfies operator to ensure the
literal conforms to TraitsData (e.g., const obj = { ... } satisfies TraitsData)
so the compiler will catch missing fields; reference the buildTraits function
and the TraitsData type when making the change.
In `@components/waves/memes/submission/steps/AdditionalInfoStep.tsx`:
- Around line 88-89: Add a brief clarifying comment where the object literal
sets additional_media.artist_profile_media = [] in AdditionalInfoStep (or the
variable/prop named additional_media) explaining that the empty array is
intentional and validated/managed elsewhere (e.g., "artist_profile_media
intentionally left empty here because X is handled in Y") so future maintainers
don't treat it as an oversight.
In `@components/waves/memes/submission/ui/MetadataLengthHint.tsx`:
- Around line 31-35: The warning paragraph in MetadataLengthHint is missing
screen-reader announcement attributes; update the JSX returned by the warning
branch (the <p className={`tw-text-amber-400 ${baseClassName}`}> that renders
{status.length}/{status.maxLength}) to include role="alert" and
aria-live="polite" so assistive tech is notified when the user approaches the
limit; keep the same classes and content and avoid changing the error branch
(which already has these attributes).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b4c1845b-dd7e-40b2-91fd-c92353528639
📒 Files selected for processing (17)
__tests__/components/waves/memes/submission/MemesArtSubmissionContainer.test.tsx__tests__/components/waves/memes/submission/components/AdditionalMediaUpload.test.tsx__tests__/components/waves/memes/submission/components/AllowlistBatchManager.test.tsx__tests__/components/waves/memes/submission/components/PaymentConfig.test.tsx__tests__/components/waves/memes/submission/hooks/useArtworkSubmissionMutation.test.tsx__tests__/components/waves/memes/submission/steps/AdditionalInfoStep.test.tsx__tests__/components/waves/memes/submission/utils/submissionMetadata.test.tscomponents/waves/memes/submission/MemesArtSubmissionContainer.tsxcomponents/waves/memes/submission/components/AdditionalMediaUpload.tsxcomponents/waves/memes/submission/components/AirdropConfig.tsxcomponents/waves/memes/submission/components/AllowlistBatchManager.tsxcomponents/waves/memes/submission/components/PaymentConfig.tsxcomponents/waves/memes/submission/hooks/useArtworkSubmissionMutation.tscomponents/waves/memes/submission/steps/AdditionalInfoStep.tsxcomponents/waves/memes/submission/ui/MetadataLengthHint.tsxcomponents/waves/memes/submission/utils/buildPreviewDrop.tscomponents/waves/memes/submission/utils/submissionMetadata.ts
|
GelatoGenesis
approved these changes
Mar 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Tests