Skip to content

Conversation

@3idey
Copy link

@3idey 3idey commented Mar 19, 2025

Summary

This PR improves error handling in renameItem() in the Joomla Media Manager. It replaces raw error throws with Joomla notifications, ensuring better user feedback and using Joomla's Notification System Instead of Throwing Errors

Changes Made

  • Updated renameItem() to use notifications.error() instead of throw new Error(error);
  • Ensured that errors are properly logged in Joomla’s notification system.
  • Improve File Exists Handling (Duplicate Files).

Testing Instructions

  • Open Joomla Media Manager.
  • Try renaming a file to an invalid name (e.g., same name as an existing file).
  • You should see an error notification instead of the app crashing.

Checklist

  • Code follows Joomla's coding standards.
  • Fix does not introduce new bugs.
  • Properly tested within Joomla.

@joomla-cms-bot joomla-cms-bot added NPM Resource Changed This Pull Request can't be tested by Patchtester PR-5.2-dev labels Mar 19, 2025
@3idey
Copy link
Author

3idey commented Mar 19, 2025

@richard67 @laoneo can you tell me is there anything that needs to be fixed.

@bembelimen bembelimen added the bug label Apr 15, 2025
@HLeithner HLeithner changed the base branch from 5.2-dev to 5.3-dev April 15, 2025 16:36
@HLeithner
Copy link
Member

This pull request has been automatically rebased to 5.3-dev.

@HLeithner HLeithner changed the title Fix error handling rename item [5.3] Fix error handling rename item Apr 15, 2025
@3idey
Copy link
Author

3idey commented Apr 21, 2025

Will it be merged soon @HLeithner

@HLeithner
Copy link
Member

@3idey thanks for the PR, please revert the package-lock.json which seems to be unrelated.

@Fedik @joomdonation can you have a look?

@Fedik
Copy link
Member

Fedik commented Apr 22, 2025

Looks good.

@3idey
Copy link
Author

3idey commented Apr 22, 2025

Done. @HLeithner

@richard67
Copy link
Member

Done. @HLeithner

@3idey Not right. Check the changed files of your PR on GitHub: https://github.com/joomla/joomla-cms/pull/45164/files

Now you PR deletes the package-lock-json file.

You should have reverted the changes in that file, not delete that file.

@3idey
Copy link
Author

3idey commented Apr 22, 2025

@HLeithner @richard67 what about now ? I revert the version to "5.2.1"

@richard67
Copy link
Member

@HLeithner @richard67 what about now ? I revert the version to "5.2.1"

@3idey Yes, but at the wrong place. Again: Check the changed files of your PR on GitHub here: https://github.com/joomla/joomla-cms/pull/45164/files . Then you will see that.

@3idey
Copy link
Author

3idey commented Apr 22, 2025

@richard67 I'm so sorry for the confusion that I made. now it should be good

@richard67
Copy link
Member

@richard67 I'm so sorry for the confusion that I made. now it should be good

@3idey Yes, now it looks alright. I've allowed myself to update the branch to the base (5.3-dev), so now the PR shows only 2 changed files and no obviously unrelated changes. Please check again on https://github.com/joomla/joomla-cms/pull/45164/files if something is missing or if it looks complete to you. Thanks in advance.

@3idey
Copy link
Author

3idey commented Apr 22, 2025

@richard67 Everything looks great. Thanks

@brianteeman
Copy link
Contributor

Should this be removed now?

// @todo error handling

@3idey
Copy link
Author

3idey commented Apr 22, 2025

@HLeithner Done.

@brianteeman
Copy link
Contributor

I tried to rename an existing image to the same name as another existing image

Before This PR

image

With this PR

image

@3idey 3idey closed this by deleting the head repository May 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug NPM Resource Changed This Pull Request can't be tested by Patchtester

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants