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

[4.0] Updating or uninstalling one library no longer removes another when both belong to same vendor. #34954

Merged
merged 4 commits into from
Aug 2, 2021

Conversation

BrainforgeUK
Copy link
Contributor

Pull Request for Issue # .
#34910

Summary of Changes

Check if there are any other children before removing vendor folder.

Testing Instructions

As described in #34910

Actual result BEFORE applying this Pull Request

Updating or uninstalling one library removes another when both belong to same vendor.

Expected result AFTER applying this Pull Request

Updating or uninstalling one library no longer removes another when both belong to same vendor.

Documentation Changes Required

Check if using library names like these is documented.

mycompany/mylibrary1

mycompany/mylibrary2

…emoves another when both belong to same vendor.

### Summary of Changes
Check if there are any other children before removing vendor folder.

### Testing instructions
As described in joomla#34910
@joomdonation
Copy link
Contributor

@BrainforgeUK Could you please give two sample libraries so that we can use it for testing?

@BrainforgeUK
Copy link
Contributor Author

lib_bftest1.zip
lib_bftest2.zip
lib_bftest3.zip

With J4 RC5
Install the 3 extensions 1,2,3 - installed OK.
Folders .../libraries/brainforgeuk/test[123] exist OK.
Install lib_bftest2 again (do not remove it - pretend its and update).
Admin Extensions Manage page shows all 3 installed.
Folder .../libraries/brainforgeuk/test2 exists, the others have been removed!

With Change
Install the 3 extensions 1,2,3 - installed OK.
Folders .../libraries/brainforgeuk/test[123] exist OK.
Install lib_bftest2 again (do not remove it - pretend its and update).
Admin Extensions Manage page shows all 3 installed.
Folder .../libraries/brainforgeuk/test[123] exist OK.

In admin Extensions Manage page unininstall lib_bftest3 OK, folders for test1 and test2 still exist.
In admin Extensions Manage page unininstall lib_bftest1 OK, folder for test2 still exists.
In admin Extensions Manage page unininstall lib_bftest2 OK, folder brainforgeuk removed.

@maikol-ortigueira
Copy link

I have tested this item ✅ successfully on 6b2c94b

I have tested this issue successfully, and for this I have made use of the files you provide.

PHP version : 7.4
Joomla Version: 4.0.0-rc5
OS: Ubuntu 20.04.2 LTS

Thank you!!


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

@pabloarias
Copy link

Tested with Devilbox and library packages provided.

PHP 7.4.20
Joomla! 4.0.0-rc6-dev

Thank very much!


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

@pabloarias
Copy link

I have tested this item ✅ successfully on 6b2c94b


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

@alikon
Copy link
Contributor

alikon commented Jul 30, 2021

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jul 30, 2021
@alikon
Copy link
Contributor

alikon commented Jul 30, 2021

happy to see yet another one Devilbox https://github.com/cytopia/devilbox/ user

@richard67
Copy link
Member

@BrainforgeUK Could you implement @joomdonation ' suggestion above - see #34954 (comment) - and remove the empty arrays for the last 2 parameters so the defaults are used? I will then help to get testers again. Thanks in advance.

@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jul 30, 2021
@alikon
Copy link
Contributor

alikon commented Jul 30, 2021

back to pending


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

@richard67 richard67 added the Updates Requested Indicates that this pull request needs an update from the author and should not be tested. label Jul 30, 2021
…() so the defaults are used

As suggestion made in pull comment.
@BrainforgeUK
Copy link
Contributor Author

Removed the empty arrays, as requested,

@richard67 richard67 removed the Updates Requested Indicates that this pull request needs an update from the author and should not be tested. label Jul 30, 2021
@richard67
Copy link
Member

Thanks

@richard67
Copy link
Member

@maikol-ortigueira @pabloarias Could you test again? There has been made a change. Thanks in advance.

@richard67
Copy link
Member

@BrainforgeUK Sorry I have to bother you again. My mistake, I promose it's the last time. Could you also remove the optional two boolean parameters which now are the last two, so only $folders = Folder::folders(JPATH_PLATFORM . '/' . $elementParts[0], '.'); remains? Thanks in advance.

@joomdonation is right, we don't need to use absolute path here, so the last 2 could be "false, false" and so equal to the defaults instead of "false, true".

@richard67
Copy link
Member

@BrainforgeUK Thanks a lot, very appreciated. Will prepare to test now.

@BrainforgeUK
Copy link
Contributor Author

Also removed the filter parameter as the default is the same.
Ok challenging me on this ... spend to much time on DIY!
Maybe might be able to usefully contribute again another time.

@richard67
Copy link
Member

I have tested this item ✅ successfully on 19ba82c


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

1 similar comment
@joomdonation
Copy link
Contributor

I have tested this item ✅ successfully on 19ba82c


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

@richard67
Copy link
Member

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jul 31, 2021
@richard67 richard67 added this to the Joomla 4.0 milestone Jul 31, 2021
@richard67
Copy link
Member

@wilsonge Because it's a fix for the installer, I'd like to have that in 4.0, so I've set the 4.0 milestone. Feel free to change if not ok.

@wilsonge wilsonge merged commit 4d992e2 into joomla:4.0-dev Aug 2, 2021
@wilsonge
Copy link
Contributor

wilsonge commented Aug 2, 2021

Thanks!

@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Aug 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants