feat(promo-codes): Tier 2 — search, type filter & multi-select delete for allowed email domains#950
Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Also adds a regression test that confirms searchInput (not just search) is checked so a typed-but-not-yet-debounced search term is cleared before new entries are added, keeping additions visible.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ders Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe modal now supports debounced full-text search, type-based filtering (at-domain, TLD, or email), multi-select with checkboxes, and bulk delete. Filters reset when the modal opens or domains are added; selection clears when filters change. Autoscroll defers until after the filtered list re-renders. Tests verify debounce interactions, filter composition, selection behavior, and scroll timing. ChangesEmail Domain Modal Search, Filter, and Bulk Selection
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/components/forms/promocode-form/forms/domain-authorized/__tests__/manage-modal.test.jsx (1)
417-464: ⚡ Quick winEnsure fake timers are always restored, even on assertion failure.
If this test fails before the final cleanup call, fake timers can leak into subsequent tests. Wrap the test body in
try/finallysojest.useRealTimers()always runs.Proposed fix
it("closing the modal within the debounce window cancels the pending setSearch", () => { - jest.useFakeTimers(); - const { rerender } = render( - <ManageAllowedEmailDomainsModal - show - onHide={onHide} - onApply={onApply} - existing={["`@a.com`"]} - /> - ); + jest.useFakeTimers(); + try { + const { rerender } = render( + <ManageAllowedEmailDomainsModal + show + onHide={onHide} + onApply={onApply} + existing={["`@a.com`"]} + /> + ); - fireEvent.change(screen.getByTestId("manage-modal-search"), { - target: { value: "acme" } - }); - expect(screen.getByTestId("manage-modal-search")).toHaveValue("acme"); + fireEvent.change(screen.getByTestId("manage-modal-search"), { + target: { value: "acme" } + }); + expect(screen.getByTestId("manage-modal-search")).toHaveValue("acme"); - rerender( - <ManageAllowedEmailDomainsModal - show={false} - onHide={onHide} - onApply={onApply} - existing={["`@a.com`"]} - /> - ); + rerender( + <ManageAllowedEmailDomainsModal + show={false} + onHide={onHide} + onApply={onApply} + existing={["`@a.com`"]} + /> + ); - expect(() => jest.runAllTimers()).not.toThrow(); + expect(() => jest.runAllTimers()).not.toThrow(); - rerender( - <ManageAllowedEmailDomainsModal - show - onHide={onHide} - onApply={onApply} - existing={["`@a.com`"]} - /> - ); - expect(screen.getByTestId("manage-modal-search")).toHaveValue(""); - - jest.useRealTimers(); + rerender( + <ManageAllowedEmailDomainsModal + show + onHide={onHide} + onApply={onApply} + existing={["`@a.com`"]} + /> + ); + expect(screen.getByTestId("manage-modal-search")).toHaveValue(""); + } finally { + jest.useRealTimers(); + } });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/forms/promocode-form/forms/domain-authorized/__tests__/manage-modal.test.jsx` around lines 417 - 464, The test "closing the modal within the debounce window cancels the pending setSearch" currently calls jest.useFakeTimers() and jest.useRealTimers() but may leak fake timers on assertion failure; wrap the test body (the code that renders ManageAllowedEmailDomainsModal, fires change, rerenders, advances timers, and assertions) in a try/finally block so that jest.useRealTimers() is invoked in the finally, ensuring timers are always restored even if an assertion throws.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@src/components/forms/promocode-form/forms/domain-authorized/__tests__/manage-modal.test.jsx`:
- Around line 417-464: The test "closing the modal within the debounce window
cancels the pending setSearch" currently calls jest.useFakeTimers() and
jest.useRealTimers() but may leak fake timers on assertion failure; wrap the
test body (the code that renders ManageAllowedEmailDomainsModal, fires change,
rerenders, advances timers, and assertions) in a try/finally block so that
jest.useRealTimers() is invoked in the finally, ensuring timers are always
restored even if an assertion throws.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0aa563fc-5072-4af7-bd8d-f94d59277ad8
📒 Files selected for processing (3)
src/components/forms/promocode-form/forms/domain-authorized/ManageAllowedEmailDomainsModal.jsxsrc/components/forms/promocode-form/forms/domain-authorized/__tests__/manage-modal.test.jsxsrc/i18n/en.json
There was a problem hiding this comment.
Pull request overview
Extends the promo code “Manage Allowed Email Domains” modal UI to better handle large lists by adding debounced search, an entry-type filter, and multi-select bulk delete, while keeping the allowed_email_domains: string[] contract unchanged.
Changes:
- Added debounced substring search and a type filter (All /
@domain/ .tld / user@email) to narrow the visible list. - Added per-row selection, “Select All” (scoped to the filtered view), and “Delete Selected”.
- Expanded the modal test suite to cover the new filtering, selection, and deferred autoscroll behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/i18n/en.json | Adds new i18n strings for search/filter and bulk actions in the manage modal. |
| src/components/forms/promocode-form/forms/domain-authorized/ManageAllowedEmailDomainsModal.jsx | Implements search, type filter, selection state, bulk delete, and deferred autoscroll. |
| src/components/forms/promocode-form/forms/domain-authorized/tests/manage-modal.test.jsx | Adds Tier 2 tests for debounced search, filter composition, selection semantics, and autoscroll timing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Address PR #950 review feedback (Copilot + CodeRabbit): - Add aria-label to row checkbox, search input, type-filter select, and add-domains textarea — none had an accessible name (label/aria-label). Mirrors the aria-label pattern already used in AllowedEmailDomainsRow. - Correct the misleading comment on the debounce-cancel test and wrap its body in try/finally so jest.useRealTimers() always runs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Addressed all review feedback in a5ea195. Copilot (4 inline comments) — all valid, all fixed:
CodeRabbit (1 nitpick) — valid, fixed:
All i18n labels go through |
ref: https://app.clickup.com/t/86ba1pt2t
Domain-Authorized Promo Codes — Bulk Input Tier 2
Extends
ManageAllowedEmailDomainsModalso admins can find and remove entrieswithin a large
allowed_email_domainslist. Pure frontend — no API change, theallowed_email_domains: string[]wire contract is untouched. Independent of thesummit-api Track 2 work.
What's new
All/@domain/.tld/user@email; composes with search.view), and Delete Selected.
Tier 1 shipped the modal with a read-only virtualized list; Tier 2 adds three
modal-scoped state slices (debounced search, type filter, a selection
Setkeyed by original array index). A
visiblememo derives the filtered view,carrying
originalIndexso selection and deletion stay correct under filtering.Implementation notes
Built task-by-task with per-task spec + code-quality review and a Codex review
pass after each task, plus a whole-branch Codex pass. Review-driven fixes folded
in: deferred post-add autoscroll (react-window had the stale
itemCountwhenscrollToItemran synchronously after a batchedsetWorking), a debounce-windowrace on Add, removal of an out-of-scope search-flush, the search debounce effect
gated on modal
show, and a strengthened filtered-Select-All test that guardsthe
originalIndexcontract.Testing
manage-modal.test.jsx); fullpromocode-formsuite green (121); 0 ESLint errors.domains and drove it in a browser — search narrowing, type filter + search
composition, multi-select delete, autoscroll-on-add, filtered-add-clears-filter,
Done/Cancel. No console errors.
api.dev.fnopen.com— opened aDOMAIN_AUTHORIZED_*promo code, exercised search / type filter / multi-selectdelete, Saved, reloaded, and confirmed the persisted
allowed_email_domainsround-trips correctly.
Summary by CodeRabbit