feature: migrate media upload types grid to MUI#1005
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (12)
💤 Files with no reviewable changes (2)
🚧 Files skipped from review as they are similar to previous changes (10)
📝 WalkthroughWalkthroughThis PR replaces the media upload edit route/page with an in-page MUI dialog, updates media upload actions and reducers, rewrites the list page flow, and adds new test coverage for the dialog and list interactions. ChangesMedia uploads dialog rewrite
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant MediaUploadListPage
participant getMediaUpload
participant MediaUploadDialog
participant saveMediaUpload
participant getMediaUploads
MediaUploadListPage->>getMediaUpload: load row entity
getMediaUpload-->>MediaUploadListPage: currentMediaUpload
MediaUploadListPage->>MediaUploadDialog: render dialog
MediaUploadDialog->>saveMediaUpload: submit form
saveMediaUpload-->>MediaUploadDialog: resolve
MediaUploadListPage->>getMediaUploads: refresh listing
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Tools execution failed with the following error: Failed to run tools: 13 INTERNAL: Received RST_STREAM with code 2 (Internal server error) 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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/actions/media-upload-actions.js (1)
238-255: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winWrap
deleteMediaUploadin loading state
deleteMediaUploadis the only action here that skipsstartLoading()/stopLoading(). The list page also doesn’t disable delete elsewhere, so a second delete can be fired while the first request is still in flight.🤖 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/media-upload-actions.js` around lines 238 - 255, Wrap deleteMediaUpload in the same loading lifecycle used by the other media-upload actions: start loading before the delete request begins and stop loading in a finally path after it completes or fails. Update the deleteMediaUpload thunk in media-upload-actions.js to dispatch startLoading()/stopLoading() around deleteRequest so the UI is blocked while the request is in flight and duplicate deletes can’t be triggered.
🤖 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/media-upload-actions.js`:
- Around line 147-194: The saveMediaUpload thunk in media-upload-actions.js can
reject after snackbarErrorHandler, so callers using
saveMediaUpload(...).then(...) need to handle failures explicitly. Update the
caller(s) of saveMediaUpload to add a .catch() (or equivalent rejection
handling) alongside the existing .then() success flow, so rejected
putRequest/postRequest promises do not become unhandled rejections. Use the
saveMediaUpload symbol to find the async action and preserve the current success
snackbar behavior.
In `@src/pages/media_uploads/components/__tests__/media-upload-dialog.test.js`:
- Around line 208-223: The media upload dialog validation is still rejecting a
cleared max_size field because positiveNumberValidation() only allows integers
and min(0), so onSubmit never reaches onSave when the input is empty. Update the
validation/schema used by the media-upload-dialog flow so the max_size field can
accept an empty string and normalize it to 0 (or the intended empty-state value)
before submission, and keep the test in media-upload-dialog.test.js aligned with
the behavior by asserting the submitted value from the dialog’s save path.
---
Outside diff comments:
In `@src/actions/media-upload-actions.js`:
- Around line 238-255: Wrap deleteMediaUpload in the same loading lifecycle used
by the other media-upload actions: start loading before the delete request
begins and stop loading in a finally path after it completes or fails. Update
the deleteMediaUpload thunk in media-upload-actions.js to dispatch
startLoading()/stopLoading() around deleteRequest so the UI is blocked while the
request is in flight and duplicate deletes can’t be triggered.
🪄 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: 66af4e14-fb5e-4488-a5b7-cd7d79d61583
📒 Files selected for processing (11)
src/actions/media-upload-actions.jssrc/components/forms/media-upload-form.jssrc/i18n/en.jsonsrc/layouts/media-upload-layout.jssrc/pages/media_uploads/__tests__/media-upload-list-page.test.jssrc/pages/media_uploads/components/__tests__/media-upload-dialog.test.jssrc/pages/media_uploads/components/media-upload-dialog.jssrc/pages/media_uploads/edit-media-upload-page.jssrc/pages/media_uploads/media-upload-list-page.jssrc/reducers/media_uploads/media-upload-reducer.jssrc/reducers/media_uploads/media-uploads-list-reducer.js
💤 Files with no reviewable changes (2)
- src/pages/media_uploads/edit-media-upload-page.js
- src/components/forms/media-upload-form.js
| export const saveMediaUpload = (entity) => async (dispatch, getState) => { | ||
| const { currentSummitState } = getState(); | ||
| const accessToken = await getAccessTokenSafely(); | ||
| const { currentSummit } = currentSummitState; | ||
|
|
||
| dispatch(startLoading()); | ||
| dispatch(startLoading()); | ||
|
|
||
| const normalizedEntity = normalizeEntity(entity); | ||
| const params = { access_token: accessToken }; | ||
| const normalizedEntity = normalizeEntity(entity); | ||
| const params = { access_token: accessToken }; | ||
|
|
||
| if (entity.id) { | ||
| putRequest( | ||
| createAction(UPDATE_MEDIA_UPLOAD), | ||
| createAction(MEDIA_UPLOAD_UPDATED), | ||
| `${window.API_BASE_URL}/api/v1/summits/${currentSummit.id}/media-upload-types/${entity.id}`, | ||
| normalizedEntity, | ||
| authErrorHandler, | ||
| entity | ||
| )(params)(dispatch).then(() => { | ||
| if (!noAlert) | ||
| dispatch(showSuccessMessage(T.translate("media_upload.saved"))); | ||
| else dispatch(stopLoading()); | ||
| }); | ||
| } else { | ||
| const successMessage = { | ||
| title: T.translate("general.done"), | ||
| html: T.translate("media_upload.created"), | ||
| type: "success" | ||
| }; | ||
|
|
||
| postRequest( | ||
| createAction(UPDATE_MEDIA_UPLOAD), | ||
| createAction(MEDIA_UPLOAD_ADDED), | ||
| `${window.API_BASE_URL}/api/v1/summits/${currentSummit.id}/media-upload-types`, | ||
| normalizedEntity, | ||
| authErrorHandler, | ||
| entity | ||
| )(params)(dispatch).then((payload) => { | ||
| if (entity.id) { | ||
| return putRequest( | ||
| createAction(UPDATE_MEDIA_UPLOAD), | ||
| createAction(MEDIA_UPLOAD_UPDATED), | ||
| `${window.API_BASE_URL}/api/v1/summits/${currentSummit.id}/media-upload-types/${entity.id}`, | ||
| normalizedEntity, | ||
| snackbarErrorHandler, | ||
| entity | ||
| )(params)(dispatch) | ||
| .then(() => { | ||
| dispatch( | ||
| showMessage(successMessage, () => { | ||
| history.push( | ||
| `/app/summits/${currentSummit.id}/media-uploads/${payload.response.id}` | ||
| ); | ||
| snackbarSuccessHandler({ | ||
| title: T.translate("general.success"), | ||
| html: T.translate("media_upload.saved") | ||
| }) | ||
| ); | ||
| }); | ||
| } | ||
| }; | ||
| }) | ||
| .finally(() => dispatch(stopLoading())); | ||
| } | ||
|
|
||
| return postRequest( | ||
| createAction(UPDATE_MEDIA_UPLOAD), | ||
| createAction(MEDIA_UPLOAD_ADDED), | ||
| `${window.API_BASE_URL}/api/v1/summits/${currentSummit.id}/media-upload-types`, | ||
| normalizedEntity, | ||
| snackbarErrorHandler, | ||
| entity | ||
| )(params)(dispatch) | ||
| .then(() => { | ||
| dispatch( | ||
| snackbarSuccessHandler({ | ||
| title: T.translate("general.success"), | ||
| html: T.translate("media_upload.created") | ||
| }) | ||
| ); | ||
| }) | ||
| .finally(() => dispatch(stopLoading())); | ||
| }; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect the library's request helpers to see if the promise chain rejects after errorHandler runs
fd -a -t d 'openstack-uicore-foundation' node_modules 2>/dev/null | head -1
fd -a 'actions.js' node_modules/openstack-uicore-foundation 2>/dev/null
rg -n "export (const|function) (putRequest|postRequest|deleteRequest|getRequest)" -A 30 node_modules/openstack-uicore-foundation/lib/utils/actions.js 2>/dev/nullRepository: fntechgit/summit-admin
Length of output: 737
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== actions.js outline =="
wc -l /home/jailuser/git/node_modules/openstack-uicore-foundation/lib/utils/actions.js
sed -n '1,260p' /home/jailuser/git/node_modules/openstack-uicore-foundation/lib/utils/actions.js
echo
echo "== locate media upload caller =="
fd -a 'media-upload-list-page.js' /home/jailuser/git/src /home/jailuser/git 2>/dev/null || true
rg -n "saveMediaUpload\\(|handleSave|then\\(" /home/jailuser/git/src -g 'media-upload-list-page.js' -g '*.js' 2>/dev/null | head -200Repository: fntechgit/summit-admin
Length of output: 31358
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== actions.js outline =="
wc -l /home/jailuser/git/node_modules/openstack-uicore-foundation/lib/utils/actions.js
sed -n '1,260p' /home/jailuser/git/node_modules/openstack-uicore-foundation/lib/utils/actions.js
echo
echo "== media upload caller =="
fd -a 'media-upload-list-page.js' /home/jailuser/git/src /home/jailuser/git 2>/dev/null || true
rg -n "saveMediaUpload\\(|handleSave|then\\(" /home/jailuser/git/src -g 'media-upload-list-page.js' -g '*.js' 2>/dev/null | head -200Repository: fntechgit/summit-admin
Length of output: 37151
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,220p' /home/jailuser/git/node_modules/openstack-uicore-foundation/lib/utils/actions.js | nl -ba | sed -n '1,220p'Repository: fntechgit/summit-admin
Length of output: 200
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== openstack-uicore-foundation/lib/utils/actions.js =="
nl -ba /home/jailuser/git/node_modules/openstack-uicore-foundation/lib/utils/actions.js | sed -n '1,260p'
echo
echo "== media-upload caller references =="
rg -n "saveMediaUpload\\(|handleSave|\\.then\\(|\\.catch\\(" /home/jailuser/git/src -g '*.js' 2>/dev/null | sed -n '1,220p'Repository: fntechgit/summit-admin
Length of output: 255
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== src/actions/media-upload-actions.js =="
nl -ba /home/jailuser/git/src/actions/media-upload-actions.js | sed -n '120,230p'
echo
echo "== caller chain =="
rg -n "saveMediaUpload\\(" /home/jailuser/git/src -g '*.js' -A 6 -B 6Repository: fntechgit/summit-admin
Length of output: 242
🌐 Web query:
openstack-uicore-foundation lib/utils/actions.js putRequest postRequest snackbarErrorHandler reject catch
💡 Result:
In the openstack-uicore-foundation library, lib/utils/actions.js serves as a central module for managing API interactions within the application's Redux-based architecture [1]. The library implements a standardized pattern for handling asynchronous HTTP requests through helper functions such as getRequest, postRequest, and putRequest [1][2]. These helpers are designed to be used in conjunction with Redux thunks to dispatch actions for the request lifecycle (starting, success, and error) [1][2]. Regarding the specific components you mentioned: 1. postRequest and putRequest: These are API utility functions located in lib/utils/actions.js [1]. They typically accept action creators for request and success states, the API endpoint, and an error handler, returning a function that executes the request when invoked with dispatch and getState [2]. 2. snackbarErrorHandler: This is a specialized error-handling function used within this utility pattern [1]. When a request fails, it is often passed to these API helpers to intercept the error—frequently handling authentication issues (such as a 401 redirect) or triggering user-facing notifications like a snackbar [1]. 3. reject and catch: These are standard components of the promise chains used within these utility functions [2]. When an API call fails, the catch block intercepts the error, passes it to the error handler (e.g., snackbarErrorHandler), and typically triggers a reject action or promise rejection to ensure the calling thunk can handle the error state appropriately [1][2]. In practice, this pattern ensures that API requests, loading states, and error messaging are handled consistently across the application [1]. Developers typically call these helpers inside their action creators, providing the necessary dispatch and state parameters [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
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
paths = [
Path('/home/jailuser/git/src/actions/media-upload-actions.js'),
Path('/home/jailuser/git/src/pages/media_uploads/media-upload-list-page.js'),
]
for path in paths:
print(f'== {path} ==')
lines = path.read_text().splitlines()
for i, line in enumerate(lines, 1):
if path.name == 'media-upload-actions.js' and 147 <= i <= 194:
print(f'{i:4}: {line}')
elif path.name == 'media-upload-list-page.js' and 70 <= i <= 90:
print(f'{i:4}: {line}')
print()
PYRepository: fntechgit/summit-admin
Length of output: 2823
Handle rejected save promises in the media upload flow. putRequest/postRequest reject after snackbarErrorHandler, so saveMediaUpload(...).then(...) without a .catch() can surface unhandled promise rejections on save errors. Add a catch on the caller.
🤖 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/media-upload-actions.js` around lines 147 - 194, The
saveMediaUpload thunk in media-upload-actions.js can reject after
snackbarErrorHandler, so callers using saveMediaUpload(...).then(...) need to
handle failures explicitly. Update the caller(s) of saveMediaUpload to add a
.catch() (or equivalent rejection handling) alongside the existing .then()
success flow, so rejected putRequest/postRequest promises do not become
unhandled rejections. Use the saveMediaUpload symbol to find the async action
and preserve the current success snackbar behavior.
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 `@package.json`:
- Line 96: The package.json dependency for openstack-uicore-foundation is pinned
to a beta version instead of a stable release. Update the version in
package.json to the stable release currently in use unless this PR explicitly
requires the beta, and keep the dependency aligned with the stable tag for this
package.
🪄 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: 074439ba-9f84-476f-881b-dc0ce401054b
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (4)
package.jsonsrc/pages/media_uploads/components/__tests__/media-upload-dialog.test.jssrc/pages/media_uploads/components/media-upload-dialog.jssrc/pages/media_uploads/media-upload-list-page.js
🚧 Files skipped from review as they are similar to previous changes (3)
- src/pages/media_uploads/components/media-upload-dialog.js
- src/pages/media_uploads/components/tests/media-upload-dialog.test.js
- src/pages/media_uploads/media-upload-list-page.js
There was a problem hiding this comment.
Pull request overview
This PR migrates the “Media Upload Types” list + edit/create flow to the newer MUI-based UI Core components, replacing the dedicated edit page with an in-page dialog while also improving list paging/total tracking and snackbar-based feedback.
Changes:
- Replaced the legacy list/table/search/pagination UI with MUI equivalents and introduced an in-page
MediaUploadDialogfor create/edit. - Updated media upload actions/reducers to support totals/paging metadata and snackbar success/error handling.
- Added Jest/RTL coverage for the new dialog and the updated list page behavior.
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| yarn.lock | Updates lockfile for the newer openstack-uicore-foundation dependency. |
| package.json | Bumps openstack-uicore-foundation to 5.0.41-beta.0. |
| src/actions/media-upload-actions.js | Migrates error/success handling to snackbar handlers; adjusts list request payload; updates save/copy flows. |
| src/reducers/media_uploads/media-uploads-list-reducer.js | Tracks totalMediaUploads and threads perPage through request state. |
| src/reducers/media_uploads/media-upload-reducer.js | Clears errors on receive and reset paths for cleaner dialog behavior. |
| src/pages/media_uploads/media-upload-list-page.js | Migrates the list page to MUI table/search; adds dialog-based create/edit/delete behavior. |
| src/pages/media_uploads/edit-media-upload-page.js | Removes the dedicated edit page (replaced by dialog flow). |
| src/pages/media_uploads/components/media-upload-dialog.js | Adds the new MUI/Formik/Yup dialog for creating/editing media upload types. |
| src/pages/media_uploads/components/tests/media-upload-dialog.test.js | Adds unit tests for dialog behaviors (validation, saving, TTL toggle, etc.). |
| src/pages/media_uploads/tests/media-upload-list-page.test.js | Adds unit tests for list page wiring (fetch on mount, paging/sort/search, dialog open/save/delete/copy). |
| src/layouts/media-upload-layout.js | Removes now-obsolete routes and simplifies the layout to render only the list page. |
| src/i18n/en.json | Adds new strings used by snackbar and dialog UI (e.g., copied success, “minutes”). |
| src/components/forms/media-upload-form.js | Removes the legacy bootstrap-based form (replaced by dialog). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| deleteDialogBody={(name) => | ||
| `${T.translate("media_upload.delete_warning")}${name}` | ||
| } |
| initialValues: entity, | ||
| validationSchema: yup.object().shape({ | ||
| name: yup.string().required(T.translate("validation.required")), | ||
| max_size: positiveNumberValidation(), |
santipalenque
force-pushed
the
feature/migrate-media-upload-types
branch
from
2aa7e2f to
d25c54d
Compare
https://app.clickup.com/t/9014802374/86bapy7m1
Summary by CodeRabbit
New Features
Bug Fixes
Localization