[#48320] - [5.4] Prevent duplicate item selection causing 404 delete errors in media manager
- Ready to Commit
- Medium
- Build: 5.4-dev
- # 48320
- Diff
- hiteshm0:fix/multi-delete-media-manager
User tests: Successful: Unsuccessful:
Pull Request resolves #48295
- I read the Generative AI policy and my contribution is either not created with the help of AI or is compatible with the policy and GNU/GPL 2 or later.
Summary of Changes
Added an Array.some() check inside the SELECT_BROWSER_ITEM mutation (mutations.es6.js) to verify an item isn't already selected before pushing it to the state.
WHY?
When a user performs a 'top-to-down' shift-click selection, the array-slicing logic in utils.es6.js included the 'anchor' (i.e the item selected first) item in the loop. Because the mutation blindly pushed incoming items without validating them, the anchor item was duplicated in the selectedItems array.
By making the mutation more robust, we protect against duplicate DELETE requests regardless of how the data is sliced.
Testing Instructions
- Navigate to the Joomla Media Manager.
- Open a folder containing at least 3 or 4 files/images.
- Click to select the first file in the grid.
- Hold the Shift key and click the third file in the grid (selecting files 1, 2, and 3).
- Click the Delete button.
Actual result BEFORE applying this Pull Request
- The user is immediately sees a red error stating: "Error deleting the item / File or folder not found".
NOTE: It is also possible that the duplicate DELETE request succeeds ( i.e The user will observe an extra success message)
Expected result AFTER applying this Pull Request
- The files are deleted without any inconsistent messages.
Link to documentations
Please select:
- Documentation link for guide.joomla.org:
- No documentation changes for guide.joomla.org needed
- Pull Request link for manual.joomla.org:
- No documentation changes for manual.joomla.org needed
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | JavaScript Administration com_media NPM Change |
| Labels |
Added:
NPM Resource Changed
PR-5.4-dev
|
||
I have tested this item ✅ successfully on cd08532
The patch works as described for multiple deletes.
I used the pre-built package 5.4.9-dev+pr.48320 to set up a fresh site. Navigated to the Media Manager.
Then I tried to upload seven files to the /images folder. Doing so, I got the following error messages:
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/48320.
I have tested this item ✅ successfully on cd08532
The patch works as described for multiple deletes.
I used the pre-built package 5.4.9-dev+pr.48320 to set up a fresh site. Navigated to the Media Manager.
Then I tried to upload seven files to the /images folder. Doing so, I got the following error messages:
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/48320.
I have tested this item ✅ successfully on cd08532
The patch works as described for multiple deletes.
I used the pre-built package "5.4.9-dev+pr.48320" to set up a fresh site. Navigated to the Media Manager.
Then I tried to upload seven files to the /images folder. Doing so, I got the following error messages:
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/48320.
I have tested this item ✅ successfully on cd08532
The patch works as described for multiple deletes.
I used the pre-built package "5.4.9-dev+pr.48320" to set up a fresh site. Navigated to the Media Manager.
Then I tried to upload seven files to the /images folder. Doing so, I got the following error messages:
I propose to check whether this PR has introduced those error messages.
Subsequent upload worked without those errors.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/48320.
@richard67 My test was successful regarding the intended functionality. But I'm in doubt that the PR introduced a new issue. See my comment above.
@richard67 My test was successful regarding the intended functionality. But I'm in doubt that the PR introduced a new issue. See my comment above.
@dautrich I've just triggered a branch update for this PR, so new up to date patched packages have been built meanwhile.
Could you check if you can reproduce your issue with that, and if so, if you also can reproduce it with e.g. the latest 5.4-dev nightly build to se if it also happens without this PR?
doubt the failing check is due to the changes made in this pr
The failure of the NPM installation step in the CI checks was caused by node.js providing a wrong version number, This has been fixed meanwhile in their current version, see https://github.com/nodejs/node/releases/tag/v26.8.1 .
@dautrich I've just triggered a branch update for this PR, so new up to date patched packages have been built meanwhile.
Could you check if you can reproduce your issue with that, and if so, if you also can reproduce it with e.g. the latest 5.4-dev nightly build to se if it also happens without this PR?
@richard67
I repeated the test with your newly built patched packages. The error shown in my previous screenshot showed up again. I tried to upload seven files, and got five error messages and two uploaded files. I deleted the two files and repeated the upload of the seven files. This time, all seven files were uploaded correctly.
The delete after multiple select worked fine.
Afterward, I downloaded and a 5.4.9 nightly build. On upload of seven files, I got the identical behavior: five error messages, two uploaded files.
My conclusion: The errors on file upload are unrelated to the PR here. Therefore, my test should be rated as successful.
@dautrich I've just triggered a branch update for this PR, so new up to date patched packages have been built meanwhile.
Could you check if you can reproduce your issue with that, and if so, if you also can reproduce it with e.g. the latest 5.4-dev nightly build to se if it also happens without this PR?
@richard67
I repeated the test with your newly built patched package. The error shown in my previous screenshot showed up again. I tried to upload seven files, and got five error messages and two uploaded files. I deleted the two files and repeated the upload of the seven files. This time, all seven files were uploaded correctly.
The delete after multiple select worked fine.
Afterward, I downloaded and installed a 5.4.9 nightly build. On upload of seven files, I got the identical behavior: five error messages, two uploaded files. Again, subsequent uploads worked fine.
My conclusion: The errors on file upload are unrelated to the PR here. Therefore, my test should be rated as successful.
Should I create an issue for the file upload errors?
My conclusion: The errors on file upload are unrelated to the PR here. Therefore, my test should be rated as successful.
@dautrich It is already recorded as successful test.
Should I create an issue for the file upload errors?
Yes please. A quick search for keywords "media manager file upload" has not shown any existing issue which could fit. So it will very likely not be a duplicate report.
Thanks for checking.
I have tested this item ✅ successfully on cd2077f
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/48320.
I have tested this item ✅ successfully on cd2077f
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/48320.
| Status | Pending | ⇒ | Ready to Commit |
RTC
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/48320.
RTC
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/48320.
| Labels |
Added:
RTC
|
||
doubt the failing check is due to the changes made in this pr