Fixed member import tier mapping#27612
Conversation
WalkthroughThis pull request introduces configurable support for member tier import functionality. The import modal now calls 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
Restored the feature-flagged tier mapping option in the React member import flow so sites with importMemberTier enabled can map CSV tier columns again.
832235b to
a485e9d
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/posts/test/unit/views/members/import-members/mapping.test.ts (1)
58-62: Optional: add a disabled-flag assertion for symmetryA small companion test asserting Tier is absent when
importMemberTieris false would harden regression coverage in both directions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/posts/test/unit/views/members/import-members/mapping.test.ts` around lines 58 - 62, Add a companion test in mapping.test.ts that calls getFieldMappings({importMemberTier: false}) and asserts that the result does not contain {label: 'Tier', value: 'import_tier'} (use toEqual/contains negation like not.toContainEqual) so the behaviour is covered for the disabled flag; locate the existing test that uses getFieldMappings({importMemberTier: true}) and mirror its structure/name (e.g., "does not add tier as an available field mapping when disabled") to keep symmetry and clarity.apps/posts/src/views/members/components/bulk-action-modals/import-members/components/mapping-step.tsx (1)
23-23: Consider reusing a shared field-mapping type
fieldMappings: {label: string; value: string}[]is duplicated locally. Exporting and reusing a shared type from the mapping module would reduce drift risk.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/posts/src/views/members/components/bulk-action-modals/import-members/components/mapping-step.tsx` at line 23, Replace the local inline type for fieldMappings with a shared exported type: export a FieldMapping (or FieldMappingOption) type from the existing mapping module (the module that already defines mapping logic/types) and import that type into mapping-step.tsx, then change the prop/type declaration fieldMappings: {label: string; value: string}[] to fieldMappings: FieldMapping[] (or the chosen name) so both files reuse the same exported type and avoid duplication.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@apps/posts/src/views/members/components/bulk-action-modals/import-members/components/mapping-step.tsx`:
- Line 23: Replace the local inline type for fieldMappings with a shared
exported type: export a FieldMapping (or FieldMappingOption) type from the
existing mapping module (the module that already defines mapping logic/types)
and import that type into mapping-step.tsx, then change the prop/type
declaration fieldMappings: {label: string; value: string}[] to fieldMappings:
FieldMapping[] (or the chosen name) so both files reuse the same exported type
and avoid duplication.
In `@apps/posts/test/unit/views/members/import-members/mapping.test.ts`:
- Around line 58-62: Add a companion test in mapping.test.ts that calls
getFieldMappings({importMemberTier: false}) and asserts that the result does not
contain {label: 'Tier', value: 'import_tier'} (use toEqual/contains negation
like not.toContainEqual) so the behaviour is covered for the disabled flag;
locate the existing test that uses getFieldMappings({importMemberTier: true})
and mirror its structure/name (e.g., "does not add tier as an available field
mapping when disabled") to keep symmetry and clarity.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4edac582-bb73-482c-a7e1-18182c61048f
📒 Files selected for processing (5)
apps/posts/src/views/members/components/bulk-action-modals/import-members-modal.tsxapps/posts/src/views/members/components/bulk-action-modals/import-members/components/mapping-step.tsxapps/posts/src/views/members/components/bulk-action-modals/import-members/mapping.tsapps/posts/test/unit/views/members/import-members/mapping.test.tsapps/posts/test/unit/views/members/import-members/modal.test.tsx
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a485e9d654
ℹ️ 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".
| } | ||
| }; | ||
| }, [state.file]); | ||
| }, [importMemberTier, state.file]); |
There was a problem hiding this comment.
Avoid resetting mapping when feature flag data arrives
Including importMemberTier in the file-read effect dependency causes the CSV to be re-parsed whenever useBrowseConfig() resolves or refetches, which dispatches PARSE_SUCCESS and overwrites any manual field selections the user has already made in the mapping step. This is reproducible when a user drops a file before the config request completes (or if config refetches), and the mapping UI unexpectedly resets mid-flow.
Useful? React with 👍 / 👎.
Summary
Restores the feature-flagged tier mapping behavior in the React member import flow.
Root Cause
The legacy Ember import UI added the
Tier/import_tiermapping option whenimportMemberTierwas enabled. The recently ported React member import flow used a static field mapping list and static supported type list, so customers with the flag enabled could no longer select or auto-detect tier columns.Changes
config.labs.importMemberTierin the React member import modal.import_tierdetection and the mapped Tier UI state.Validation
pnpm --filter @tryghost/posts exec vitest run test/unit/views/members/import-memberspnpm --filter @tryghost/posts buildpnpm --filter @tryghost/posts lint:code