Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[5.2] Error handling while create folder in media manager new pr #39878

Merged
merged 5 commits into from
Jan 18, 2025

Conversation

Kaushik1216
Copy link
Contributor

@Kaushik1216 Kaushik1216 commented Feb 17, 2023

Pull Request for Issue #39263
Alternate pr of pr #39793

Summary of Changes

Handling error while create folder by checking enter name by user

Testing Instructions

create folder and try create a folder by input invalid name

Actual result BEFORE applying this Pull Request

php error and create folder modal is open

Expected result AFTER applying this Pull Request

afoldername.mp4

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

@joomla-cms-bot joomla-cms-bot added Language Change This is for Translators NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.3-dev labels Feb 17, 2023
@Kaushik1216
Copy link
Contributor Author

@dgrammatiko please hint me how this error handling posisble for online storage solution

@dgrammatiko
Copy link
Contributor

You need to ask @laoneo for this, I'm not well educated on this

@laoneo
Copy link
Member

laoneo commented Feb 17, 2023

We should not have duplicate folder name validation on the server and client side. So all the client should do is to handle the error properly which comes from the server.

@Kaushik1216
Copy link
Contributor Author

@laoneo but at current stage backend show msg error occured for all validation failure

cases that can handle by this pr

  1. If user already have a folder name let's say ds if user try to make folder name d@s you get folder created success msg but actually you folder not created because php side validation remove @ in folder name and now folder name ds already exist so no folder created thus resticting user to enter only alphanumaric ,undersocore ,hyphen and period which accepted by php side.
  2. when user give same file name which already exist only error occured but with this pr user get notification like folder already exist .
  3. relative path error
    thus user not have to put it's effort to find error

@laoneo
Copy link
Member

laoneo commented Feb 17, 2023

Then you need to fix the server side to return proper errors.

@Kaushik1216
Copy link
Contributor Author

Then you need to fix the server side to return proper errors.

yes that can also done but this will increase number of clicks by user .Best way that I got it to show error in modal by @dgrammatiko and from issue #39263 .

@Hackwar Hackwar added the bug label Apr 7, 2023
@HLeithner HLeithner changed the base branch from 4.3-dev to 4.4-dev September 30, 2023 22:44
@HLeithner
Copy link
Member

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

@HLeithner HLeithner changed the title [4.3] Error handling while create folder in media manager new pr [4.4] Error handling while create folder in media manager new pr Apr 24, 2024
@HLeithner HLeithner changed the base branch from 4.4-dev to 5.2-dev November 15, 2024 13:21
@HLeithner
Copy link
Member

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

@HLeithner HLeithner changed the title [4.4] Error handling while create folder in media manager new pr [5.2] Error handling while create folder in media manager new pr Nov 15, 2024
@fgsw
Copy link

fgsw commented Jan 16, 2025

I have tested this item ✅ successfully on b1e5105

Test by using Prebuilt update package: Tried to create an existing folder; create a folder having more as one dot.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/39878.

@pe7er
Copy link
Contributor

pe7er commented Jan 18, 2025

I have tested this item ✅ successfully on b1e5105

Nice improvement! Works fine. Thanks also for the clear video demo which made it easy to test!


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/39878.

@pe7er
Copy link
Contributor

pe7er commented Jan 18, 2025

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/39878.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jan 18, 2025
@Hackwar Hackwar enabled auto-merge (squash) January 18, 2025 12:12
@Hackwar Hackwar merged commit 7c11cd0 into joomla:5.2-dev Jan 18, 2025
3 checks passed
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jan 18, 2025
@richard67 richard67 added this to the Joomla! 5.2.4 milestone Jan 18, 2025
@Hackwar
Copy link
Member

Hackwar commented Jan 18, 2025

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Language Change This is for Translators NPM Resource Changed This Pull Request can't be tested by Patchtester PR-5.2-dev
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants