fix: add missing file remove calls from inventory popup - #1002
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 adds authenticated sponsor form image deletion thunks, expands customized form item image retrieval, routes image deletion callbacks through upload components and dialogs, and updates Redux state after deletion. ChangesImage deletion feature
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant MuiFormikUpload
participant SponsorInventoryDialog
participant Page
participant SponsorFormsActions
participant API
participant ReduxStore
MuiFormikUpload->>SponsorInventoryDialog: delete image id
SponsorInventoryDialog->>Page: invoke onImageDeleted with image id
Page->>SponsorFormsActions: invoke deletion thunk with item context
SponsorFormsActions->>API: send authenticated DELETE request
API-->>SponsorFormsActions: return deletion response
SponsorFormsActions->>ReduxStore: dispatch image-deleted action
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/actions/sponsor-forms-actions.js (1)
1448-1465: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winCall
saveItemImageshere —saveImagesis undefined in this module, so this branch throws aReferenceErrorafter the POST succeeds and skips the success snackbar/return flow. PassformId,response.id, and the images array tosaveItemImagesinstead.🤖 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/actions/sponsor-forms-actions.js` around lines 1448 - 1465, The success branch in the sponsor form item save flow is calling an undefined saveImages helper, which causes a ReferenceError after the POST succeeds. Update the promise chain in the action that handles the POST response to use saveItemImages instead, and pass the formId, response.id, and the images array so the success snackbar and return flow complete normally.
🤖 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/actions/sponsor-forms-actions.js`:
- Around line 1627-1649: The removeSponsorCustomizedFormItemImages action is
using the whole-item deletion action for an image-only endpoint, which can cause
the item row to be removed instead of just the image. Update the deleteRequest
callback in removeSponsorCustomizedFormItemImages to dispatch a dedicated
image-delete action (or add one if missing) and ensure
sponsor-customized-form-items-list-reducer.js handles that image-specific action
separately from SPONSOR_CUSTOMIZED_FORM_ITEM_DELETED.
In `@src/pages/sponsors-global/form-templates/sponsor-inventory-popup.js`:
- Around line 87-90: The optional onImageDeleted callback is being invoked
unconditionally in handleIDeleteImage, which can throw when the prop is omitted.
Update handleIDeleteImage in sponsor-inventory-popup.js to guard the callback
before calling it, keeping the image id check but only invoking onImageDeleted
when it is defined.
In
`@src/pages/sponsors/sponsor-form-item-list-page/components/sponsor-form-item-form.js`:
- Around line 60-63: `handleDeleteImage` in `SponsorFormItemForm` should
defensively guard the `onImageDeleted` callback before calling it, since it may
be undefined like in the sibling `SponsorItemDialog`. Update the delete handler
to check that `onImageDeleted` exists alongside the `id` check, keeping the same
behavior when the callback is provided.
In
`@src/pages/sponsors/sponsor-form-item-list-page/components/sponsor-form-item-popup.js`:
- Around line 55-60: The dialog title in SponsorFormItemPopup includes leftover
debug text after the translated label, so remove the literal string from the
Typography block and keep only the T.translate result in the popup title. Use
the existing SponsorFormItemPopup component and its title rendering logic to
locate and clean up this stray text.
---
Outside diff comments:
In `@src/actions/sponsor-forms-actions.js`:
- Around line 1448-1465: The success branch in the sponsor form item save flow
is calling an undefined saveImages helper, which causes a ReferenceError after
the POST succeeds. Update the promise chain in the action that handles the POST
response to use saveItemImages instead, and pass the formId, response.id, and
the images array so the success snackbar and return flow complete normally.
🪄 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: 57ad779e-180d-41ad-9758-c88bd93dd6d4
📒 Files selected for processing (7)
src/actions/sponsor-forms-actions.jssrc/pages/sponsors-global/form-templates/form-template-item-list-page.jssrc/pages/sponsors-global/form-templates/sponsor-inventory-popup.jssrc/pages/sponsors-global/inventory/inventory-list-page.jssrc/pages/sponsors/sponsor-form-item-list-page/components/sponsor-form-item-form.jssrc/pages/sponsors/sponsor-form-item-list-page/components/sponsor-form-item-popup.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-forms-tab/components/manage-items/sponsor-forms-manage-items.js
|
|
||
| export const removeSponsorCustomizedFormItemImages = | ||
| (formId, formItemId, fileId) => async (dispatch, getState) => { | ||
| const { currentSummitState, currentSponsorState } = getState(); | ||
| const { currentSummit } = currentSummitState; | ||
| const { | ||
| entity: { id: sponsorId } | ||
| } = currentSponsorState; | ||
| const accessToken = await getAccessTokenSafely(); | ||
| const params = { access_token: accessToken }; | ||
|
|
||
| dispatch(startLoading()); | ||
|
|
||
| return deleteRequest( | ||
| null, | ||
| createAction(SPONSOR_CUSTOMIZED_FORM_ITEM_DELETED), | ||
| `${window.PURCHASES_API_URL}/api/v1/summits/${currentSummit.id}/sponsors/${sponsorId}/sponsor-forms/${formId}/items/${formItemId}/images/${fileId}`, | ||
| null, | ||
| snackbarErrorHandler | ||
| )(params)(dispatch).finally(() => { | ||
| dispatch(stopLoading()); | ||
| }); | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n 'SPONSOR_CUSTOMIZED_FORM_ITEM_DELETED' src -g '*.js'
rg -n 'SPONSOR_FORM_ITEM_FILE_DELETED' src -g '*.js'Repository: fntechgit/summit-admin
Length of output: 881
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,220p' src/reducers/sponsors/sponsor-customized-form-items-list-reducer.js
printf '\n---\n'
sed -n '1160,1215p' src/actions/sponsor-forms-actions.js
printf '\n---\n'
sed -n '1,180p' src/reducers/sponsors/sponsor-form-items-list-reducer.jsRepository: fntechgit/summit-admin
Length of output: 10737
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n 'function deleteRequest|const deleteRequest|export .*deleteRequest' src node_modules -g '*.js'
printf '\n---\n'
rg -n 'SPONSOR_CUSTOMIZED_FORM_ITEM_DELETED|SPONSOR_FORM_ITEM_FILE_DELETED' src/reducers src/actions -g '*.js' -A3 -B3
printf '\n---\n'
rg -n 'removeSponsorCustomizedFormItemImages|removeItemFile' src/actions/sponsor-forms-actions.js -A20 -B5Repository: fntechgit/summit-admin
Length of output: 6242
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n 'deleteRequest' src -g '*.js'
printf '\n---\n'
rg -n 'createAction\(SPONSOR_CUSTOMIZED_FORM_ITEM_DELETED\)|createAction\(SPONSOR_FORM_ITEM_FILE_DELETED\)' src/actions/sponsor-forms-actions.js -A6 -B6
printf '\n---\n'
rg -n 'itemId' src/actions src/reducers -g '*.js' -A2 -B2 | head -n 200Repository: fntechgit/summit-admin
Length of output: 31518
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n 'SPONSOR_FORM_ITEM_FILE_DELETED|SPONSOR_CUSTOMIZED_FORM_ITEM_DELETED' src/reducers src/actions -g '*.js' -A3 -B3
printf '\n---\n'
rg -n 'case .*FILE_DELETED|case .*CUSTOMIZED_FORM_ITEM_DELETED' src/reducers -g '*.js' -A4 -B4Repository: fntechgit/summit-admin
Length of output: 7406
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n 'export .*createAction|const createAction|function createAction' src node_modules openstack-uicore-foundation -g '*.js' -g '*.ts'
printf '\n---\n'
rg -n 'deleteRequest\(' src/actions -g '*.js' -A8 -B4 | head -n 120Repository: fntechgit/summit-admin
Length of output: 228
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n 'deleteRequest\(\s*null,\s*createAction\([A-Z0-9_]+\)(\s*\{|\s*\()|deleteRequest\(\s*null,\s*createAction\([A-Z0-9_]+\)\s*,' src/actions -g '*.js' -A4 -B2
printf '\n---\n'
rg -n 'createAction\([A-Z0-9_]+\)\s*\(\{[^}]*itemId[^}]*\}\)' src/actions -g '*.js' -A2 -B2Repository: fntechgit/summit-admin
Length of output: 160
🌐 Web query:
openstack-uicore-foundation deleteRequest createAction payload itemId
💡 Result:
In the openstack-uicore-foundation library, createAction and deleteRequest are components of a standard Redux-based pattern used for handling asynchronous API operations [1][2]. 1. createAction: This is a factory function used to create Redux action objects. It takes a type and returns a function that accepts a payload, resulting in an action object with the structure: { type, payload } [2]. javascript export const createAction = type => payload => ({ type, payload }); 2. deleteRequest: This is a helper function defined in src/utils/actions.js that facilitates HTTP DELETE requests [1][3]. It is typically used within an asynchronous thunk action to handle the API call lifecycle [2]. When implementing a delete action, the payload parameter often contains specific data required for the request, such as an itemId or other identifiers needed to construct the API endpoint or request body [1][2]. While the exact implementation of the payload depends on the specific consumer, it is typically passed through the thunk to the deleteRequest helper, which then executes the request and dispatches the corresponding Redux actions [1][2].
Citations:
- 1: https://unpkg.com/openstack-uicore-foundation@5.0.34/.claude/rules/openstack-uicore-foundation-project.md
- 2: https://unpkg.com/openstack-uicore-foundation@5.0.34/.claude/rules/openstack-uicore-foundation-components.md
- 3: https://app.unpkg.com/openstack-uicore-foundation@5.0.34/files/.claude/rules/openstack-uicore-foundation-project.md
🌐 Web query:
openstack-uicore-foundation deleteRequest implementation createAction payload action creator
💡 Result:
In the openstack-uicore-foundation library, the createAction factory and the deleteRequest helper are components of a standardized Redux pattern used for handling asynchronous API operations [1][2]. createAction Implementation The createAction function is a simple factory that generates a standard Redux action creator [2][3]. It takes an action type as its argument and returns a function that accepts a payload, producing an object with the structure { type, payload } [2][3]. Implementation: export const createAction = type => payload => ({ type, payload }); deleteRequest Implementation The deleteRequest function is a utility helper located in utils/actions.js [1]. It is designed to work within an asynchronous Redux thunk pattern to handle DELETE HTTP requests [1][2]. It typically takes the request action creator and the success/receive action creator (both created via createAction) as arguments, along with the API URL and an error handler [2]. Typical Usage Pattern: When used in a thunk, deleteRequest is invoked with the necessary action creators to dispatch the appropriate lifecycle events (e.g., initiating the request and handling the response) [1][2]. Example structure of an async thunk: export const myAction = (params) => async (dispatch, getState) => { dispatch(startLoading); return deleteRequest( createAction(REQUEST_TYPE), createAction(RECEIVE_TYPE), buildAPIBaseUrl('/api/v1/endpoint'), authErrorHandler)(params)(dispatch, getState); }; This pattern ensures consistent action dispatching and error handling, specifically integrating with authErrorHandler to manage authentication errors such as 401 redirects [1][2].
Citations:
- 1: https://unpkg.com/openstack-uicore-foundation@5.0.34/.claude/rules/openstack-uicore-foundation-project.md
- 2: https://unpkg.com/openstack-uicore-foundation@5.0.34/.claude/rules/openstack-uicore-foundation-components.md
- 3: https://app.unpkg.com/openstack-uicore-foundation@5.0.34/files/.claude/rules/openstack-uicore-foundation-components.md
🌐 Web query:
site:unpkg.com openstack-uicore-foundation deleteRequest src/utils/actions.js
💡 Result:
In the openstack-uicore-foundation library, deleteRequest is a utility function located in src/utils/actions.js [1]. It serves as an HTTP request helper designed to facilitate DELETE operations within the application's architecture [1]. This function is typically used as part of a set of API utilities, which include other standard HTTP request helpers like getRequest, putRequest, and postRequest [1]. These utilities are generally utilized in conjunction with Redux action creators to manage API interactions, often incorporating error handling mechanisms such as authErrorHandler to manage session states or authentication issues (e.g., handling 401 errors) [1].
Citations:
Use a dedicated image-delete action here. SPONSOR_CUSTOMIZED_FORM_ITEM_DELETED is handled by sponsor-customized-form-items-list-reducer.js as a whole-item removal, so using it for /images/${fileId} can drop the item row instead of just removing one image. src/actions/sponsor-forms-actions.js:1628
🤖 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/actions/sponsor-forms-actions.js` around lines 1627 - 1649, The
removeSponsorCustomizedFormItemImages action is using the whole-item deletion
action for an image-only endpoint, which can cause the item row to be removed
instead of just the image. Update the deleteRequest callback in
removeSponsorCustomizedFormItemImages to dispatch a dedicated image-delete
action (or add one if missing) and ensure
sponsor-customized-form-items-list-reducer.js handles that image-specific action
separately from SPONSOR_CUSTOMIZED_FORM_ITEM_DELETED.
✅ Addressed in commit 39221f4
There was a problem hiding this comment.
@tomrndom Reopening this — it was marked resolved but only half of the ask landed.
The thread asked for two things: dispatch a dedicated image-delete action, and have sponsor-customized-form-items-list-reducer.js handle it separately. On the current tip (bd8aa78):
- The dedicated constant
SPONSOR_CUSTOMIZED_FORM_ITEM_IMAGE_DELETEDexists, but it is dispatched unapplied —createAction(TYPE)instead ofcreateAction(TYPE)({ fileId })— so what reaches the store is{ type, payload: { response } }, not{ fileId }. git grep SPONSOR_CUSTOMIZED_FORM_ITEM_IMAGE_DELETED srcreturns only the constant declaration and that single dispatch. The reducer has zero references to it.
Net effect: the DELETE succeeds server-side and no state updates. Details, the user-visible symptom, and a suggested patch are in my comment on line 1793.
Worth noting the sibling thread on line 1285 makes the same "pass an applied creator carrying the payload" point — that one was applied to removeItemFile but not to removeSponsorCustomizedFormItemImages.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/pages/sponsors/sponsor-form-item-list-page/components/sponsor-form-item-popup.js (1)
74-81: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
removeItemFilemissing from propTypes.The prop is destructured and used at Line 29/46 but not declared in
propTypes, unlike the other injected action props.🐛 Proposed fix
SponsorFormItemPopup.propTypes = { open: PropTypes.bool.isRequired, onClose: PropTypes.func.isRequired, formId: PropTypes.string.isRequired, resetSponsorFormItem: PropTypes.func.isRequired, saveSponsorFormItem: PropTypes.func.isRequired, - updateSponsorFormItem: PropTypes.func.isRequired + updateSponsorFormItem: PropTypes.func.isRequired, + removeItemFile: PropTypes.func.isRequired };🤖 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/pages/sponsors/sponsor-form-item-list-page/components/sponsor-form-item-popup.js` around lines 74 - 81, The SponsorFormItemPopup component is using the removeItemFile prop but it is missing from its propTypes declaration. Update SponsorFormItemPopup.propTypes to include removeItemFile alongside the other injected action props so the component’s expected props match the destructuring and usage in SponsorFormItemPopup.
🤖 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/pages/sponsors/sponsor-form-item-list-page/components/sponsor-form-item-popup.js`:
- Around line 74-81: The SponsorFormItemPopup component is using the
removeItemFile prop but it is missing from its propTypes declaration. Update
SponsorFormItemPopup.propTypes to include removeItemFile alongside the other
injected action props so the component’s expected props match the destructuring
and usage in SponsorFormItemPopup.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9041e12b-b634-4ccf-855c-df050b4cb506
📒 Files selected for processing (1)
src/pages/sponsors/sponsor-form-item-list-page/components/sponsor-form-item-popup.js
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>
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
d27136e to
564071b
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/actions/sponsor-forms-actions.js`:
- Around line 1277-1285: Update the image-delete flow using
SPONSOR_FORM_ITEM_FILE_DELETED so deleteRequest receives an action payload
containing fileId rather than an unbound action creator. Add handling for this
action in both relevant reducers, removing the deleted image by fileId from
currentItem.images while preserving the remaining images.
In `@src/pages/sponsors/sponsor-form-item-list-page/index.js`:
- Around line 134-136: Update handleRemoveItemImage to pass the current form
item’s ID, rather than the currentItem object, as removeItemFile’s second
argument so the generated item URL contains the actual identifier.
🪄 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: b9f71fad-e979-4706-be14-026d85894e15
📒 Files selected for processing (8)
src/actions/sponsor-forms-actions.jssrc/pages/sponsors-global/form-templates/form-template-item-list-page.jssrc/pages/sponsors-global/form-templates/sponsor-inventory-popup.jssrc/pages/sponsors-global/inventory/inventory-list-page.jssrc/pages/sponsors/sponsor-form-item-list-page/components/sponsor-form-item-form.jssrc/pages/sponsors/sponsor-form-item-list-page/components/sponsor-form-item-popup.jssrc/pages/sponsors/sponsor-form-item-list-page/index.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-forms-tab/components/manage-items/sponsor-forms-manage-items.js
🚧 Files skipped from review as they are similar to previous changes (5)
- src/pages/sponsors-global/form-templates/form-template-item-list-page.js
- src/pages/sponsors/sponsor-form-item-list-page/components/sponsor-form-item-form.js
- src/pages/sponsors-global/form-templates/sponsor-inventory-popup.js
- src/pages/sponsors/sponsor-page/tabs/sponsor-forms-tab/components/manage-items/sponsor-forms-manage-items.js
- src/pages/sponsors-global/inventory/inventory-list-page.js
| return deleteRequest( | ||
| null, | ||
| createAction(SPONSOR_FORM_ITEM_FILE_DELETED), | ||
| `${window.PURCHASES_API_URL}/api/v1/summits/${currentSummit.id}/show-forms/${formId}/items/${formItemId}/images/${fileId}`, | ||
| null, | ||
| snackbarErrorHandler | ||
| )(params)(dispatch).finally(() => { | ||
| dispatch(stopLoading()); | ||
| }); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline src/reducers/sponsors/sponsor-form-items-list-reducer.js --items all
ast-grep outline src/reducers/sponsors/sponsor-customized-form-items-list-reducer.js --items all
rg -n -C 5 \
'SPONSOR_FORM_ITEM_FILE_DELETED|SPONSOR_CUSTOMIZED_FORM_ITEM_IMAGE_DELETED' \
src/reducers src/actions/__tests__ -g '*.js'Repository: fntechgit/summit-admin
Length of output: 1078
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- reducer sections ---'
sed -n '1,180p' src/reducers/sponsors/sponsor-form-items-list-reducer.js
sed -n '1,190p' src/reducers/sponsors/sponsor-customized-form-items-list-reducer.js
printf '%s\n' '--- action type definitions and usages ---'
rg -n -C 8 \
'SPONSOR_FORM_ITEM_FILE_DELETED|SPONSOR_CUSTOMIZED_FORM_ITEM_IMAGE_DELETED|removeItemFile|removeSponsorCustomizedFormItemImages' \
src -g '*.js'
printf '%s\n' '--- deleteRequest definition and call sites ---'
rg -n -C 12 'deleteRequest\s*=|function deleteRequest|const deleteRequest|deleteRequest\(' src -g '*.js'Repository: fntechgit/summit-admin
Length of output: 50379
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- standard form reducer ---'
rg -n -C 12 \
'SPONSOR_FORM_ITEM_FILE_DELETED|SPONSOR_FORM_ITEM_(CREATED|UPDATED|DELETED)|images|files' \
src/reducers/sponsors/sponsor-form-items-list-reducer.js
printf '%s\n' '--- customized form reducer ---'
rg -n -C 12 \
'SPONSOR_CUSTOMIZED_FORM_ITEM_IMAGE_DELETED|SPONSOR_CUSTOMIZED_FORM_ITEM_(CREATED|UPDATED|DELETED)|images|files' \
src/reducers/sponsors/sponsor-customized-form-items-list-reducer.js
printf '%s\n' '--- exact action declarations and tests ---'
rg -n -C 10 \
'SPONSOR_FORM_ITEM_FILE_DELETED|SPONSOR_CUSTOMIZED_FORM_ITEM_IMAGE_DELETED' \
src/actions src/reducers src -g '*.js' \
| head -n 240
printf '%s\n' '--- request helper locations ---'
rg -n -C 10 \
'deleteRequest' \
src node_modules/openstack-uicore-foundation 2>/dev/null \
| head -n 240Repository: fntechgit/summit-admin
Length of output: 50379
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- standard form reducer ---'
cat -n src/reducers/sponsors/sponsor-form-items-list-reducer.js
printf '%s\n' '--- customized form reducer ---'
cat -n src/reducers/sponsors/sponsor-customized-form-items-list-reducer.js
printf '%s\n' '--- targeted action constants and reducer cases ---'
rg -n -C 6 \
'SPONSOR_FORM_ITEM_FILE_DELETED|SPONSOR_CUSTOMIZED_FORM_ITEM_IMAGE_DELETED' \
src/actions/sponsor-forms-actions.js \
src/reducers/sponsors/sponsor-form-items-list-reducer.js \
src/reducers/sponsors/sponsor-customized-form-items-list-reducer.js
printf '%s\n' '--- request helper files ---'
rg --files -g '!node_modules/**' | rg '(^|/)(actions|utils)(/|.*)(actions|request).*\.js$|openstack'
rg -n -C 12 'export const deleteRequest|const deleteRequest|function deleteRequest' \
src node_modules/openstack-uicore-foundation/lib \
-g '*.js' -g '!*.map' -g '!*.min.js' \
| head -n 160Repository: fntechgit/summit-admin
Length of output: 21733
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- all repository references to image-delete action types ---'
rg -n -C 8 \
'SPONSOR_FORM_ITEM_FILE_DELETED|SPONSOR_CUSTOMIZED_FORM_ITEM_IMAGE_DELETED' \
. \
-g '!node_modules/**' \
-g '!build/**' \
-g '!dist/**' \
-g '!*.map'
printf '%s\n' '--- deleteRequest implementation candidates ---'
rg --files node_modules/openstack-uicore-foundation 2>/dev/null \
| rg 'actions(\.js|/)|utils|request' \
| head -n 80
rg -n -C 15 \
'deleteRequest' \
node_modules/openstack-uicore-foundation \
-g '*.js' -g '!*.map' -g '!*.min.js' \
| head -n 200
printf '%s\n' '--- image state consumers and image update handling ---'
rg -n -C 8 \
'currentItem\.images|images.*currentItem|SPONSOR_FORM_ITEM_IMAGES_UPDATED|removeItemFile|removeSponsorCustomizedFormItemImages' \
src \
-g '!*.map'Repository: fntechgit/summit-admin
Length of output: 43412
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
reducer_paths = [
Path("src/reducers/sponsors/sponsor-form-items-list-reducer.js"),
Path("src/reducers/sponsors/sponsor-customized-form-items-list-reducer.js"),
]
action_path = Path("src/actions/sponsor-forms-actions.js")
helper_path = Path("node_modules/openstack-uicore-foundation/lib/utils/actions.js")
action_text = action_path.read_text()
for name in (
"SPONSOR_FORM_ITEM_FILE_DELETED",
"SPONSOR_CUSTOMIZED_FORM_ITEM_IMAGE_DELETED",
):
matches = list(re.finditer(
rf"createAction\({name}\)(?P<args>\([^;]*\))?",
action_text,
flags=re.S,
))
print(name, "action-call-count=", len(matches))
for match in matches:
print(" call=", " ".join(match.group(0).split()))
for path in reducer_paths:
text = path.read_text()
print(path)
for name in (
"SPONSOR_FORM_ITEM_FILE_DELETED",
"SPONSOR_CUSTOMIZED_FORM_ITEM_IMAGE_DELETED",
):
print(" ", name, "referenced=", name in text)
print(" image-filter-operations=",
len(re.findall(r"\.filter\([^)]*images|images[^;\n]*\.filter", text)))
helper_text = helper_path.read_text()
match = re.search(r"X=\([^;]{0,1200}", helper_text)
print("deleteRequest-source=", " ".join(match.group(0).split()) if match else "not found")
PYRepository: fntechgit/summit-admin
Length of output: 874
Handle image-delete actions in both reducers. Neither reducer handles its image-delete action. Also, deleteRequest receives an unbound action creator, so the dispatched payload contains only the API response and not fileId. Pass { fileId } explicitly and filter that image from currentItem.images after deletion.
🤖 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/actions/sponsor-forms-actions.js` around lines 1277 - 1285, Update the
image-delete flow using SPONSOR_FORM_ITEM_FILE_DELETED so deleteRequest receives
an action payload containing fileId rather than an unbound action creator. Add
handling for this action in both relevant reducers, removing the deleted image
by fileId from currentItem.images while preserving the remaining images.
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/sponsor-form-items-list-reducer.js`:
- Around line 120-134: Update the SPONSOR_FORM_ITEM_FILE_DELETED flow and its
removeItemFile dispatch to include itemId: formItemId in the payload. In the
reducer, match state.items by itemId and only remove the image from currentItem
when currentItem.id matches itemId; otherwise preserve currentItem unchanged.
🪄 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: 361dafbf-da5f-4a22-b851-b8ca9427a8d1
📒 Files selected for processing (3)
src/actions/sponsor-forms-actions.jssrc/pages/sponsors/sponsor-form-item-list-page/index.jssrc/reducers/sponsors/sponsor-form-items-list-reducer.js
🚧 Files skipped from review as they are similar to previous changes (2)
- src/pages/sponsors/sponsor-form-item-list-page/index.js
- src/actions/sponsor-forms-actions.js
|
|
||
| return deleteRequest( | ||
| null, | ||
| createAction(SPONSOR_CUSTOMIZED_FORM_ITEM_IMAGE_DELETED), |
There was a problem hiding this comment.
@tomrndom SPONSOR_CUSTOMIZED_FORM_ITEM_IMAGE_DELETED is dispatched here as an unapplied action creator, and no reducer handles the type — so this delete updates no state.
Two problems compound:
createAction(TYPE)returnspayload => ({ type, payload }). WhendeleteRequestgets a function as its 2nd argument it calls it with the response envelope, so what reaches the store is{ type, payload: { response } }— not{ fileId }. CompareremoveItemFilein this same file, which passes the invoked creator:createAction(SPONSOR_FORM_ITEM_FILE_DELETED)({ fileId }). Every other delete thunk here (deleteSponsorFormItem,deleteSponsorFormManagedItem,deleteSponsorForm) does the same.sponsor-customized-form-items-list-reducer.jshas nocasefor this type at all —git grep SPONSOR_CUSTOMIZED_FORM_ITEM_IMAGE_DELETEDreturns only the constant and this dispatch.
Concrete failure: this PR also adds expand: "images" to getSponsorCustomizedFormItems, which is what populates row.images and makes the image icon column in the manage-items grid render for the first time. Delete an image from the item dialog and close it without saving — the request succeeds server-side and the dialog updates locally (MuiFormikUpload filters the file out of the formik value before calling onDelete), but the grid row keeps showing the icon, and its tooltip and click-through still point at the now-orphaned file_url. It stays wrong until an unrelated refetch (search, page change, archived toggle, or saving some item). The removeItemFile path doesn't have this problem because its reducer case updates both currentItem.images and the matching row in items.
This is the unfinished half of the earlier CodeRabbit thread — the dedicated action type was added, the dispatch and the reducer case weren't.
Suggested fix — invoke the creator:
createAction(SPONSOR_CUSTOMIZED_FORM_ITEM_IMAGE_DELETED)({ fileId }),and add the mirror case to sponsor-customized-form-items-list-reducer.js, matching SPONSOR_FORM_ITEM_FILE_DELETED in sponsor-form-items-list-reducer.js:
case SPONSOR_CUSTOMIZED_FORM_ITEM_IMAGE_DELETED: {
const { fileId } = payload;
const currentItem = {
...state.currentItem,
images: state.currentItem.images?.filter((img) => img.id !== fileId) ?? []
};
const items = state.items.map((item) =>
item.id === currentItem.id
? { ...item, images: item.images?.filter((img) => img.id !== fileId) }
: item
);
return { ...state, currentItem, items };
}
|
|
||
| return { ...state, items }; | ||
| } | ||
| case SPONSOR_FORM_ITEM_FILE_DELETED: { |
There was a problem hiding this comment.
@tomrndom This new reducer case ships without a test, and so does the rest of the PR — 9 source files changed, 0 test files.
Worth adding here specifically because the suite already exists: src/reducers/sponsors/__tests__/sponsor-form-items-list-reducer.test.js, so this is a few lines in an established file, not new scaffolding. A test that seeds currentItem.images and items[].images, dispatches SPONSOR_FORM_ITEM_FILE_DELETED with { fileId }, and asserts the image is gone from both would also have caught the customized-form-item twin (see my comment on sponsor-forms-actions.js), which dispatches an action type no reducer handles. A reducer test is the cheapest place to catch a dead action type.
.claude/rules/summit-admin-testing-patterns.md covers the action side too — src/actions/__tests__/sponsor-forms-actions.test.js exists with the redux-mock-store + getAccessTokenSafely spy setup, so removeItemFile / removeSponsorCustomizedFormItemImages can be asserted on dispatched payload and request URL.
Suggested: one reducer test for SPONSOR_FORM_ITEM_FILE_DELETED (plus the customized-item case once added), and one action test asserting the dispatched payload carries fileId.
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
… save Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
ref: https://app.clickup.com/t/9014802374/86bakf3mp
Signed-off-by: Tomás Castillo tcastilloboireau@gmail.com
Summary by CodeRabbit