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

Execute sql uninstall query (if exists) on extension install abort #13376

Merged
merged 4 commits into from
Dec 29, 2016

Conversation

andrepereiradasilva
Copy link
Contributor

@andrepereiradasilva andrepereiradasilva commented Dec 26, 2016

Pull Request for New Issue.

Summary of Changes

Currently when a extension install is aborted and the install sql query already run we get a broken install.
The extension is uninstalled but the database tables will stay in joomla.

This PR proposes to change that by adding a step to rollback the sql (if an uninstall sql exists).

Testing Instructions

  1. Code review
  2. Install this broken com_test.zip extension that install with a db table called #__test. Notice you get the extension install rollback but you stay with the #__test table in the database
  3. Manually delete the #__test table
  4. Apply patch
  5. Repeat step 2 and notice now on failure the #__test table is also removed.

Documentation Changes Required

None.

@frankmayer
Copy link
Contributor

I haven't tested this yet, but one question comes to mind:
What happens if an extension is installed on top of the same? I am thinking of updating an extension by installing it on top of itself.
If that "update" fails, wouldn't it destroy for example the data of the previously working extension?

@andrepereiradasilva
Copy link
Contributor Author

andrepereiradasilva commented Dec 27, 2016

that's why i putted the if ($route === 'install'
https://github.com/joomla/joomla-cms/pull/13376/files#diff-b88334a2e44587a12ed5fed7beb8bd1eR478

But please test that scenario also

@frankmayer
Copy link
Contributor

frankmayer commented Dec 27, 2016

I have tested this item ✅ successfully on f504164

Done code review.

Note: I also tested installing the extension completely (by removing the exception) and then re-installing the broken one (with the addition of an exception in the update method). The existing table stayed intact, as expected.


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

@ggppdk
Copy link
Contributor

ggppdk commented Dec 27, 2016

That's why i putted the if ($route === 'install'
https://github.com/joomla/joomla-cms/pull/13376/files#diff-b88334a2e44587a12ed5fed7beb8bd1eR478
But please test that scenario also

Exactly my fear it would be destructive, i will test too

I mean i ll test to "update" an already installed extension with a broken package

@alikon
Copy link
Contributor

alikon commented Dec 27, 2016

I have tested this item ✅ successfully on f504164


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

@jeckodevelopment
Copy link
Member

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Dec 28, 2016
@jeckodevelopment jeckodevelopment added this to the Joomla 3.7.0 milestone Dec 28, 2016
@rdeutz rdeutz merged commit 3f400e6 into joomla:staging Dec 29, 2016
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Dec 29, 2016
@andrepereiradasilva andrepereiradasilva deleted the patch-19 branch December 29, 2016 12:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants