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

Improve Joomla! Update error state #38002

Merged
merged 4 commits into from
Jun 8, 2022
Merged

Improve Joomla! Update error state #38002

merged 4 commits into from
Jun 8, 2022

Conversation

nikosdion
Copy link
Contributor

Pull Request for Issue #37870 .

Ping @brianteeman @richard67 @jeckodevelopment

Summary of Changes

  • Updated Joomla Update's extract.php to set a long PHP timeout (3600s) and a very large memory limit (1GiB) to minimise easily negotiable server limits from interfering with the update.
  • Converted the error state from a modal to a card which replaces the progress. The card retains the information previously shown in the modal dialog and adds the following buttons:
    • Resume Update. Retries the previous extraction step. Useful when the update error occurred because of the server erroneously identifying the repeated requests to extract.php as a Denial of Service attack.
    • Restart Update. Starts the extraction from scratch. Useful if you had a “permissions hell” or disk space outage situation and want to restart the update after fixing it.
    • Cancel. Takes you back to Joomla Update's main page. Whether this will work depends on how far the extraction went.

WE DESPERATELY NEED THE HELP PAGE IN THE JOOMLA DOCUMENTATION SITE!. People, please remember that I have already written the content for you and it's in PR #35388

PLEASE tell me how to put this documentation in the wiki so that the More Information button in the error state of Joomla! Update actually gives the user useful, reliable information instead of leading to a Wiki error page!!!

Testing Instructions

Go to Joomla Update and start an update with an uploaded file. Use the sabotaged ZIP file below: sabotage.zip

Actual result BEFORE applying this Pull Request

You get an error dialog. Closing the dialog... you have no idea what to do.

Expected result AFTER applying this Pull Request

You get an error pane. You can retry the extraction or give up and go back to Joomla Update. Please test these buttons work.

If you want to test the retry/restart buttons, open your site's tmp directory. You will see a file named something like juiXFAzI and its size will be 6815744 bytes. That's our sabotage.zip file. Replace it with the following, fixed file:
mirage.zip

(Bonus points if you guessed the song these filenames refer to)

Upon replacing the file and hitting retry or restart the update will run to completion. You will get a folder named _deleteme under your site's root with the following files. I also give you their MD5 sums:

test00.dat  84d0968ea471621c2158373496023322
test01.dat  6a7ccf26116ab9629c4e47c36a27ba12
test02.dat  70ae439f0447e150b61f943f0c94bbea
test03.dat  2209f6fa847d36bbeb9714a79e5ff7db
test04.dat  3061e9faba464bb478f7e145233fe035
test05.dat  44e468ad78f174afaf07399c0d717cef
test06.dat  5efa22bdddb81ba4f84206eaf4cd01b2
test07.dat  8cc552218cd1db89fd4ea843a6cfea51
test08.dat  81a4ba890d83fe0686f1152fff304661
test09.dat  1016eb0a7a38e77162dc81d00b46464d

All files are exactly 2097152 bytes long.

Documentation Changes Required

There are no changes to the documentation I already gave you and has not been copied over to the documentation wiki.

@nikosdion nikosdion requested review from rdeutz and zero-24 as code owners June 7, 2022 12:40
@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.2-dev labels Jun 7, 2022
@Kostelano
Copy link
Contributor

It works. A bit unexpected for me is the location of the block with an error :). While I do not send the test, there may be some discussion.

Screenshot_1

@nikosdion
Copy link
Contributor Author

@Kostelano I kept the same margins and padding as the progress bar. It looks funny to me too. Maybe I could change the my-5 to my-2 to pull it a bit further up. You can play with the classes when seeing this page using the browser's dev tools to see what I mean. Ideas welcome.

@Kostelano
Copy link
Contributor

As far as indentation goes, I'd rather remove the py-5 my-5 classes, push the block all the way to the top of the page.

Screenshot_1

As for the help page, I've reopened issue #35668, which I signaled earlier. But for version 4.0, it was left without attention. At one point I was unable to reproduce the problem and closed it, but now I see that it is still not resolved.

@Kostelano
Copy link
Contributor

I have tested this item ✅ successfully on 127274f


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

@nikosdion
Copy link
Contributor Author

@Kostelano Thank you!

@viocassel
Copy link
Contributor

I have tested this item ✅ successfully on 127274f


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

@richard67
Copy link
Member

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jun 8, 2022
@roland-d roland-d merged commit 85e3760 into joomla:4.2-dev Jun 8, 2022
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jun 8, 2022
@roland-d
Copy link
Contributor

roland-d commented Jun 8, 2022

Thanks everybody.

@nikosdion Mike Brandner is looking at the 4.2 links for the wiki

@nikosdion
Copy link
Contributor Author

Thank you very much, Roland! Much appreciated, mate!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Language Change This is for Translators 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.

7 participants