fix: reload table after archive/unarchive rows#938
Open
tomrndom wants to merge 8 commits into
Open
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR implements safe pagination across item archive/unarchive operations. A new utility function computes valid page indices after item removal, and is integrated into 9 list-page archive handlers across global sponsors and sponsor-scoped sections. Reducers and tests are updated to track total count changes and validate the new behavior. ChangesSafe Pagination After Archive/Unarchive
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 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)
Comment |
smarcet
reviewed
May 15, 2026
smarcet
requested changes
May 15, 2026
smarcet
reviewed
May 15, 2026
|
[HIGH]
What the reducers do today: All case FORM_TEMPLATE_ARCHIVED: {
const updatedFormTemplates = state.formTemplates.map((item) =>
item.id === updatedFormTemplate.id ? { ...item, is_archived: true } : item
);
return { ...state, formTemplates: updatedFormTemplates };
// ↑ totalFormTemplates is untouched
}Compare with case FORM_TEMPLATE_DELETED:
return { ...state, formTemplates: [...], totalFormTemplates: state.totalFormTemplates - 1 };Why this matters for this PR:
Under normal single-click usage this is masked by the follow-up Correct fix: decrement case FORM_TEMPLATE_ARCHIVED: {
const updatedFormTemplates = state.formTemplates.map((item) =>
item.id === updatedFormTemplate.id ? { ...item, is_archived: true } : item
);
return {
...state,
formTemplates: updatedFormTemplates,
totalFormTemplates: state.totalFormTemplates - 1 // ← add this
};
}This applies to all domains: This finding is secondary to the direction×filter bug already commented on |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
src/reducers/sponsors/__tests__/sponsor-form-items-list-reducer.test.js (1)
311-311: ⚡ Quick winAdd a regression test for no-op archive/unarchive count behavior.
Good update on expected decrements; please also add a case where the target item is missing or already in the target state and assert
totalCountdoes not change.Also applies to: 380-380
🤖 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/reducers/sponsors/__tests__/sponsor-form-items-list-reducer.test.js` at line 311, Add regression tests to assert totalCount is unchanged when archive/unarchive is a no-op: in the sponsor-form-items-list-reducer test suite add two cases calling the reducer (via the same action creators used elsewhere in the file) where the target item is either missing or already in the target state (already archived when archiving, already unarchived when unarchiving) and assert that sponsorFormItemsListReducer returns state with the same totalCount value as the input; reuse the existing test helpers/setup and the same action types used around the existing totalCount decrement tests so the new cases mirror those scenarios but expect no change.
🤖 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.
Inline comments:
In `@src/reducers/sponsors_inventory/form-template-item-list-reducer.js`:
- Around line 109-113: The reducer is unconditionally decrementing
totalFormTemplateItems when returning the new state, which causes undercounts on
no-op archive/unarchive actions; update the logic in the form-template-item-list
reducer (the code that builds updatedFormTemplatesItems and sets
totalFormTemplateItems) to detect whether the target item's archived state
actually changed (compare the previous item's archived flag to the new value on
the item in updatedFormTemplatesItems) and only decrement/increment
totalFormTemplateItems when a real transition occurred, leaving
totalFormTemplateItems unchanged for no-op/replayed actions.
In `@src/reducers/sponsors_inventory/form-template-list-reducer.js`:
- Around line 108-112: The reducer currently decrements totalFormTemplates
unconditionally when returning the new state; change it to only decrement when
the template was actually removed from the current set by comparing lengths (or
checking existence) of updatedFormTemplates versus state.formTemplates or by
verifying the removed id from action.payload existed; update the return so
totalFormTemplates = state.totalFormTemplates - 1 only if removal occurred,
otherwise leave totalFormTemplates unchanged (apply the same guard for both
occurrences where totalFormTemplates is decremented).
In `@src/reducers/sponsors_inventory/inventory-item-list-reducer.js`:
- Around line 204-208: The reducer currently unconditionally decrements
totalInventoryItems by 1 (at the return that spreads state and sets
inventoryItems: updatedInventoryItems and totalInventoryItems:
state.totalInventoryItems - 1), which can desync counts on no-ops or duplicate
actions; change it to compute the actual change (e.g., const delta =
state.inventoryItems.length - updatedInventoryItems.length or check whether the
removed item was found) and set totalInventoryItems: Math.max(0,
state.totalInventoryItems - delta) or directly set totalInventoryItems:
updatedInventoryItems.length; apply the same conditional update logic to the
other similar return (lines around the second occurrence using
updatedInventoryItems) so totals only change when inventoryItems length actually
changes.
In `@src/reducers/sponsors_inventory/page-template-list-reducer.js`:
- Around line 112-116: The reducer is unconditionally decrementing
totalPageTemplates which undercounts when no row actually changed; instead
compute how many items were removed/changed by comparing state.pageTemplates and
updatedPageTemplates (e.g., removedCount = state.pageTemplates.length -
updatedPageTemplates.length or detect the specific id change) and subtract only
that removedCount from totalPageTemplates (or leave it unchanged if removedCount
is 0); update the return that currently sets totalPageTemplates:
state.totalPageTemplates - 1 to use the computed removedCount (and apply the
same guard in the other similar block that updates totalPageTemplates).
In `@src/reducers/sponsors/show-pages-list-reducer.js`:
- Around line 116-117: The reducer updates totalCount by subtracting 1 which can
go negative; change the assignment(s) that set totalCount (the object returned
in the reducer around the lines updating totalCount, e.g., where totalCount:
state.totalCount - 1 is used) to clamp the result to a minimum of 0 (use
Math.max(0, state.totalCount - 1) or equivalent) and apply the same fix to the
other occurrence mentioned (the second totalCount update at lines ~127-128).
In `@src/reducers/sponsors/sponsor-page-pages-list-reducer.js`:
- Around line 243-244: When removing a page you currently always decrement
state.customizedPages.totalItems; change the reducer logic so totalItems is only
decremented when a page was actually removed. Concretely, compute the new pages
array (e.g., newPages = pages.filter(...)) and compare lengths (or check a found
flag) — if newPages.length < pages.length then set totalItems =
state.customizedPages.totalItems - 1 (clamped to >= 0); otherwise leave
totalItems unchanged. Apply this change to both places that update pages and
totalItems (the blocks that build pages and set totalItems currently using
pages: [...pages], totalItems: state.customizedPages.totalItems - 1) so
duplicate/out-of-order actions do not drift the count.
---
Nitpick comments:
In `@src/reducers/sponsors/__tests__/sponsor-form-items-list-reducer.test.js`:
- Line 311: Add regression tests to assert totalCount is unchanged when
archive/unarchive is a no-op: in the sponsor-form-items-list-reducer test suite
add two cases calling the reducer (via the same action creators used elsewhere
in the file) where the target item is either missing or already in the target
state (already archived when archiving, already unarchived when unarchiving) and
assert that sponsorFormItemsListReducer returns state with the same totalCount
value as the input; reuse the existing test helpers/setup and the same action
types used around the existing totalCount decrement tests so the new cases
mirror those scenarios but expect no change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 761db084-fe43-4aab-b9a6-d7df7025054a
📒 Files selected for processing (21)
src/pages/sponsors-global/form-templates/form-template-item-list-page.jssrc/pages/sponsors-global/form-templates/form-template-list-page.jssrc/pages/sponsors-global/inventory/inventory-list-page.jssrc/pages/sponsors-global/page-templates/page-template-list-page.jssrc/pages/sponsors/show-pages-list-page/index.jssrc/pages/sponsors/sponsor-form-item-list-page/index.jssrc/pages/sponsors/sponsor-forms-list-page/index.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-forms-tab/components/manage-items/sponsor-forms-manage-items.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-forms-tab/index.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-pages-tab/index.jssrc/reducers/sponsors/__tests__/sponsor-form-items-list-reducer.test.jssrc/reducers/sponsors/show-pages-list-reducer.jssrc/reducers/sponsors/sponsor-customized-form-items-list-reducer.jssrc/reducers/sponsors/sponsor-form-items-list-reducer.jssrc/reducers/sponsors/sponsor-forms-list-reducer.jssrc/reducers/sponsors/sponsor-page-forms-list-reducer.jssrc/reducers/sponsors/sponsor-page-pages-list-reducer.jssrc/reducers/sponsors_inventory/form-template-item-list-reducer.jssrc/reducers/sponsors_inventory/form-template-list-reducer.jssrc/reducers/sponsors_inventory/inventory-item-list-reducer.jssrc/reducers/sponsors_inventory/page-template-list-reducer.js
🚧 Files skipped from review as they are similar to previous changes (7)
- src/pages/sponsors/sponsor-page/tabs/sponsor-forms-tab/index.js
- src/pages/sponsors/sponsor-form-item-list-page/index.js
- src/pages/sponsors/sponsor-page/tabs/sponsor-pages-tab/index.js
- src/pages/sponsors-global/form-templates/form-template-item-list-page.js
- src/pages/sponsors/show-pages-list-page/index.js
- src/pages/sponsors/sponsor-forms-list-page/index.js
- src/pages/sponsors-global/page-templates/page-template-list-page.js
smarcet
reviewed
May 20, 2026
smarcet
requested changes
May 20, 2026
smarcet
left a comment
smarcet
left a comment
There was a problem hiding this comment.
@tomrndom please review comment
#938 (comment)
tomrndom
force-pushed
the
fix/reload-table-on-archive
branch
from
cd7b812 to
b8545c5
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/reducers/sponsors/sponsor-page-forms-list-reducer.js (1)
234-252:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid unconditional decrement in
SPONSOR_CUSTOMIZED_FORM_ARCHIVED_CHANGED.Line 251 should decrement only if the targeted form actually changes
is_archived.Proposed fix
case SPONSOR_CUSTOMIZED_FORM_ARCHIVED_CHANGED: { const { formId, isArchived } = payload; + const shouldDecrement = state.customizedForms.forms.some( + (form) => form.id === formId && form.is_archived !== isArchived + ); const forms = state.customizedForms.forms.map((form) => { if (form.id === formId) { return { ...form, is_archived: isArchived }; } return form; }); return { ...state, customizedForms: { ...state.customizedForms, forms, - totalCount: state.customizedForms.totalCount - 1 + totalCount: shouldDecrement + ? Math.max(0, state.customizedForms.totalCount - 1) + : state.customizedForms.totalCount } }; }🤖 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/reducers/sponsors/sponsor-page-forms-list-reducer.js` around lines 234 - 252, The reducer handling SPONSOR_CUSTOMIZED_FORM_ARCHIVED_CHANGED unconditionally subtracts 1 from state.customizedForms.totalCount; change it to first detect whether the matched form's is_archived actually changes by locating the form with form.id === formId (use the existing map or a prior find), compare its current is_archived to payload.isArchived, and only decrement totalCount when the value changed and payload.isArchived is true (leave totalCount unchanged otherwise); update the return to use this conditional delta when constructing customizedForms.totalCount.src/reducers/sponsors/sponsor-form-items-list-reducer.js (1)
118-135:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPrevent
totalCountdrift on no-op archive/unarchive actions.Line 125 and Line 134 always decrement. If the item is absent or already in target state, pagination totals become inconsistent.
Proposed fix
case SPONSOR_FORM_ITEM_ARCHIVED: { const { id: itemId } = payload.response; + const shouldDecrement = state.items.some( + (item) => item.id === itemId && item.is_archived !== true + ); const items = state.items.map((item) => item.id === itemId ? { ...item, is_archived: true } : item ); - return { ...state, items, totalCount: state.totalCount - 1 }; + return { + ...state, + items, + totalCount: shouldDecrement + ? Math.max(0, state.totalCount - 1) + : state.totalCount + }; } case SPONSOR_FORM_ITEM_UNARCHIVED: { const { itemId } = payload; + const shouldDecrement = state.items.some( + (item) => item.id === itemId && item.is_archived !== false + ); const items = state.items.map((item) => item.id === itemId ? { ...item, is_archived: false } : item ); - return { ...state, items, totalCount: state.totalCount - 1 }; + return { + ...state, + items, + totalCount: shouldDecrement + ? Math.max(0, state.totalCount - 1) + : state.totalCount + }; }🤖 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/reducers/sponsors/sponsor-form-items-list-reducer.js` around lines 118 - 135, In the SPONSOR_FORM_ITEM_ARCHIVED and SPONSOR_FORM_ITEM_UNARCHIVED reducer cases, avoid blindly changing totalCount — determine whether an item was actually found and its archived state changed before adjusting totals: for SPONSOR_FORM_ITEM_ARCHIVED (use payload.response.id) map state.items and detect if an item with that id existed and its is_archived transitioned from false to true, then decrement totalCount by 1 only in that case; for SPONSOR_FORM_ITEM_UNARCHIVED (use payload.itemId) map state.items and detect if an item existed and its is_archived transitioned from true to false, then increment totalCount by 1 only in that case; if no item is found or no state change occurred leave totalCount unchanged.src/reducers/sponsors/sponsor-forms-list-reducer.js (1)
151-168:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMake
totalCountupdates conditional on actual state change.Line 158 and Line 167 should only decrement when the matched form actually transitions archive state.
Proposed fix
case SPONSOR_FORM_ARCHIVED: { const { id: formId } = payload.response; + const shouldDecrement = state.sponsorForms.some( + (item) => item.id === formId && item.is_archived !== true + ); const sponsorForms = state.sponsorForms.map((item) => item.id === formId ? { ...item, is_archived: true } : item ); - return { ...state, sponsorForms, totalCount: state.totalCount - 1 }; + return { + ...state, + sponsorForms, + totalCount: shouldDecrement + ? Math.max(0, state.totalCount - 1) + : state.totalCount + }; } case SPONSOR_FORM_UNARCHIVED: { const { formId } = payload; + const shouldDecrement = state.sponsorForms.some( + (item) => item.id === formId && item.is_archived !== false + ); const sponsorForms = state.sponsorForms.map((item) => item.id === formId ? { ...item, is_archived: false } : item ); - return { ...state, sponsorForms, totalCount: state.totalCount - 1 }; + return { + ...state, + sponsorForms, + totalCount: shouldDecrement + ? Math.max(0, state.totalCount - 1) + : state.totalCount + }; }🤖 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/reducers/sponsors/sponsor-forms-list-reducer.js` around lines 151 - 168, The reducer currently always adjusts totalCount when handling SPONSOR_FORM_ARCHIVED and SPONSOR_FORM_UNARCHIVED; change both branches to only mutate totalCount when the matched form actually changes archive state: for SPONSOR_FORM_ARCHIVED (use payload.response.id as formId) find the item in state.sponsorForms, if item.exists and item.is_archived is false then set is_archived: true and decrement totalCount by 1, otherwise leave totalCount unchanged; for SPONSOR_FORM_UNARCHIVED (use payload.formId) similarly only set is_archived: false and increment totalCount by 1 if the item was previously archived, otherwise keep totalCount as-is; return the updated sponsorForms array and the conditionally updated totalCount.src/reducers/sponsors/sponsor-customized-form-items-list-reducer.js (1)
123-140:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard
totalCountdecrement to real archive-state transitions only.Line 130 and Line 139 decrement unconditionally. If the target row is missing/already in target state,
totalCountdrifts.Proposed fix
case SPONSOR_CUSTOMIZED_FORM_ITEM_ARCHIVED: { const { id: itemId } = payload.response; + const shouldDecrement = state.items.some( + (item) => item.id === itemId && item.is_archived !== true + ); const items = state.items.map((item) => item.id === itemId ? { ...item, is_archived: true } : item ); - return { ...state, items, totalCount: state.totalCount - 1 }; + return { + ...state, + items, + totalCount: shouldDecrement + ? Math.max(0, state.totalCount - 1) + : state.totalCount + }; } case SPONSOR_CUSTOMIZED_FORM_ITEM_UNARCHIVED: { const { itemId } = payload; + const shouldDecrement = state.items.some( + (item) => item.id === itemId && item.is_archived !== false + ); const items = state.items.map((item) => item.id === itemId ? { ...item, is_archived: false } : item ); - return { ...state, items, totalCount: state.totalCount - 1 }; + return { + ...state, + items, + totalCount: shouldDecrement + ? Math.max(0, state.totalCount - 1) + : state.totalCount + }; }🤖 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/reducers/sponsors/sponsor-customized-form-items-list-reducer.js` around lines 123 - 140, The SPONSOR_CUSTOMIZED_FORM_ITEM_ARCHIVED and SPONSOR_CUSTOMIZED_FORM_ITEM_UNARCHIVED cases unconditionally change totalCount; instead, first locate the target item from state.items (use payload.response.id for ARCHIVED and payload.itemId for UNARCHIVED), check its current is_archived flag, and only modify totalCount when the flag actually flips: for ARCHIVED, set is_archived true and decrement totalCount by 1 only if the item existed and was previously false; for UNARCHIVED, set is_archived false and increment totalCount by 1 only if the item existed and was previously true; return updated items and the conditionally adjusted totalCount.
🤖 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.
Inline comments:
In
`@src/pages/sponsors/sponsor-page/tabs/sponsor-forms-tab/components/manage-items/sponsor-forms-manage-items.js`:
- Around line 154-185: The handler handleArchiveItem currently uses
getSafePageAfterRemove for both archive and unarchive flows; change it so
getSafePageAfterRemove(...) is used only when performing a removal
(archiveSponsorCustomizedFormItem) and for the unarchive path call
getSponsorCustomizedFormItems with the existing currentPage (no safe-page
adjustment), keeping the catch behavior consistent by reloading the appropriate
page for the attempted action; update calls around
archiveSponsorCustomizedFormItem(formId, item.id) and
unarchiveSponsorCustomizedFormItem(formId, item.id) to branch the then() logic
so the page argument passed to getSponsorCustomizedFormItems is safePage only
for archive and currentPage for unarchive.
---
Outside diff comments:
In `@src/reducers/sponsors/sponsor-customized-form-items-list-reducer.js`:
- Around line 123-140: The SPONSOR_CUSTOMIZED_FORM_ITEM_ARCHIVED and
SPONSOR_CUSTOMIZED_FORM_ITEM_UNARCHIVED cases unconditionally change totalCount;
instead, first locate the target item from state.items (use payload.response.id
for ARCHIVED and payload.itemId for UNARCHIVED), check its current is_archived
flag, and only modify totalCount when the flag actually flips: for ARCHIVED, set
is_archived true and decrement totalCount by 1 only if the item existed and was
previously false; for UNARCHIVED, set is_archived false and increment totalCount
by 1 only if the item existed and was previously true; return updated items and
the conditionally adjusted totalCount.
In `@src/reducers/sponsors/sponsor-form-items-list-reducer.js`:
- Around line 118-135: In the SPONSOR_FORM_ITEM_ARCHIVED and
SPONSOR_FORM_ITEM_UNARCHIVED reducer cases, avoid blindly changing totalCount —
determine whether an item was actually found and its archived state changed
before adjusting totals: for SPONSOR_FORM_ITEM_ARCHIVED (use
payload.response.id) map state.items and detect if an item with that id existed
and its is_archived transitioned from false to true, then decrement totalCount
by 1 only in that case; for SPONSOR_FORM_ITEM_UNARCHIVED (use payload.itemId)
map state.items and detect if an item existed and its is_archived transitioned
from true to false, then increment totalCount by 1 only in that case; if no item
is found or no state change occurred leave totalCount unchanged.
In `@src/reducers/sponsors/sponsor-forms-list-reducer.js`:
- Around line 151-168: The reducer currently always adjusts totalCount when
handling SPONSOR_FORM_ARCHIVED and SPONSOR_FORM_UNARCHIVED; change both branches
to only mutate totalCount when the matched form actually changes archive state:
for SPONSOR_FORM_ARCHIVED (use payload.response.id as formId) find the item in
state.sponsorForms, if item.exists and item.is_archived is false then set
is_archived: true and decrement totalCount by 1, otherwise leave totalCount
unchanged; for SPONSOR_FORM_UNARCHIVED (use payload.formId) similarly only set
is_archived: false and increment totalCount by 1 if the item was previously
archived, otherwise keep totalCount as-is; return the updated sponsorForms array
and the conditionally updated totalCount.
In `@src/reducers/sponsors/sponsor-page-forms-list-reducer.js`:
- Around line 234-252: The reducer handling
SPONSOR_CUSTOMIZED_FORM_ARCHIVED_CHANGED unconditionally subtracts 1 from
state.customizedForms.totalCount; change it to first detect whether the matched
form's is_archived actually changes by locating the form with form.id === formId
(use the existing map or a prior find), compare its current is_archived to
payload.isArchived, and only decrement totalCount when the value changed and
payload.isArchived is true (leave totalCount unchanged otherwise); update the
return to use this conditional delta when constructing
customizedForms.totalCount.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f5ea9f81-7c69-43d0-b4fc-710abd128e57
📒 Files selected for processing (23)
src/pages/sponsors-global/form-templates/form-template-item-list-page.jssrc/pages/sponsors-global/form-templates/form-template-list-page.jssrc/pages/sponsors-global/inventory/inventory-list-page.jssrc/pages/sponsors-global/page-templates/page-template-list-page.jssrc/pages/sponsors/show-pages-list-page/index.jssrc/pages/sponsors/sponsor-form-item-list-page/index.jssrc/pages/sponsors/sponsor-forms-list-page/index.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-forms-tab/components/manage-items/sponsor-forms-manage-items.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-forms-tab/index.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-pages-tab/index.jssrc/reducers/sponsors/__tests__/sponsor-form-items-list-reducer.test.jssrc/reducers/sponsors/show-pages-list-reducer.jssrc/reducers/sponsors/sponsor-customized-form-items-list-reducer.jssrc/reducers/sponsors/sponsor-form-items-list-reducer.jssrc/reducers/sponsors/sponsor-forms-list-reducer.jssrc/reducers/sponsors/sponsor-page-forms-list-reducer.jssrc/reducers/sponsors/sponsor-page-pages-list-reducer.jssrc/reducers/sponsors_inventory/form-template-item-list-reducer.jssrc/reducers/sponsors_inventory/form-template-list-reducer.jssrc/reducers/sponsors_inventory/inventory-item-list-reducer.jssrc/reducers/sponsors_inventory/page-template-list-reducer.jssrc/utils/__tests__/methods.test.jssrc/utils/methods.js
santipalenque
requested changes
May 26, 2026
santipalenque
left a comment
santipalenque
left a comment
There was a problem hiding this comment.
@tomrndom I think there is a much easier way of achieving this. When user archives a template, you whould check in the reducer if the "Show archived " is false , this means there is a chance the user is archiving the only item in the current page, so we need to refresh to the previous page. So if "Show archived" is false AND totalCount/perPage is an integer AND currentPage is not 1, then we change currentPage -1 . You can do all this in the archiveItem reducer action, and then just call getFormTemplateItems normally here archiveFormTemplateItem in the actions file
smarcet
reviewed
Jun 9, 2026
smarcet
left a comment
smarcet
left a comment
There was a problem hiding this comment.
Review focusing on the open threads from @smarcet (Thread 1) and @santipalenque (Thread 11), plus two correctness findings in the implementation.
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
tomrndom
force-pushed
the
fix/reload-table-on-archive
branch
from
b8545c5 to
62a4588
Compare
… the page reload Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@src/reducers/sponsors_inventory/form-template-item-list-reducer.js`:
- Around line 111-114: When toggling an item's is_archived in the reducer,
update totalFormTemplateItems only when the archive state actually changes: find
the previous item (by id) in state.formTemplateItems, compare its is_archived to
the new value in updatedFormTemplatesItems, and if it changed from false->true
decrement totalFormTemplateItems with Math.max(0, state.totalFormTemplateItems -
1); if it changed from true->false increment totalFormTemplateItems by 1. Apply
this logic in both places that build updatedFormTemplatesItems (the blocks
around the current is_archived toggles) so duplicate/replayed dispatches won't
drive totals negative or leave totals stale.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d3e4cbf3-c9f3-4edc-a8c4-bcc3727f7e0f
📒 Files selected for processing (17)
src/pages/sponsors-global/form-templates/form-template-item-list-page.jssrc/pages/sponsors-global/form-templates/form-template-list-page.jssrc/pages/sponsors-global/inventory/inventory-list-page.jssrc/pages/sponsors-global/page-templates/page-template-list-page.jssrc/pages/sponsors/show-pages-list-page/index.jssrc/pages/sponsors/sponsor-form-item-list-page/index.jssrc/pages/sponsors/sponsor-forms-list-page/index.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-forms-tab/components/manage-items/sponsor-forms-manage-items.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-forms-tab/index.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-pages-tab/index.jssrc/reducers/sponsors/__tests__/sponsor-form-items-list-reducer.test.jssrc/reducers/sponsors_inventory/form-template-item-list-reducer.jssrc/reducers/sponsors_inventory/form-template-list-reducer.jssrc/reducers/sponsors_inventory/inventory-item-list-reducer.jssrc/reducers/sponsors_inventory/page-template-list-reducer.jssrc/utils/__tests__/methods.test.jssrc/utils/methods.js
✅ Files skipped from review due to trivial changes (3)
- src/pages/sponsors/sponsor-page/tabs/sponsor-forms-tab/components/manage-items/sponsor-forms-manage-items.js
- src/reducers/sponsors_inventory/form-template-list-reducer.js
- src/reducers/sponsors_inventory/page-template-list-reducer.js
🚧 Files skipped from review as they are similar to previous changes (12)
- src/utils/methods.js
- src/pages/sponsors-global/form-templates/form-template-list-page.js
- src/pages/sponsors/sponsor-page/tabs/sponsor-forms-tab/index.js
- src/pages/sponsors/sponsor-page/tabs/sponsor-pages-tab/index.js
- src/utils/tests/methods.test.js
- src/pages/sponsors-global/page-templates/page-template-list-page.js
- src/pages/sponsors/sponsor-forms-list-page/index.js
- src/pages/sponsors/show-pages-list-page/index.js
- src/pages/sponsors-global/form-templates/form-template-item-list-page.js
- src/pages/sponsors-global/inventory/inventory-list-page.js
- src/reducers/sponsors_inventory/inventory-item-list-reducer.js
- src/pages/sponsors/sponsor-form-item-list-page/index.js
… reducers Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
santipalenque
left a comment
santipalenque
left a comment
There was a problem hiding this comment.
Seems that you have unnecessary changes @tomrndom
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
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
3 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.
ref: https://app.clickup.com/t/86b7v230m
Signed-off-by: Tomás Castillo tcastilloboireau@gmail.com
Summary by CodeRabbit
Bug Fixes
Tests