Open klonos opened 9 years ago
I like the intention here, but just hiding the 3rd item may cause confusion as to why it disappeared. Overall, this page really needs some work. "Backup your code" and "Install your new files" are generally non-sense by the time you've gotten to this page. The only suggestions that really make sense are "Backup your database" and "Put your site in maintenance mode". We can detect the maintenance mode option but not if a backup has been made.
How about we turn these bullets into questions, something like:
I personally like the way they are listed now as tasks. What we could do is detect if the site is in maintenance mode and tick ✔️ point 3 off 😉
...or use bullet points instead of numbers, so that there is no "missing" step 😄
I will take another look at this page sometime next week and think a bit deeper because as @quicksketch said, some points/tasks might not make sense by the time the user has reached that page.
...in the meantime, we have updated the text to say the following:
- Create backups. This update utility will alter your database and config files. In case of an emergency you may need to revert to a recent backup; make sure you have one.
- Database: Create a database dump of the 'my_backdrop_db' database.
- Config files: Back up the entire directory at './files/config_random_hash_here/active'.
- Put your site into maintenance mode.
- Install your new files into the appropriate location, as described in the handbook.
I was thinking either close this issue as outdated, or make these number/bullet points checkboxes instead, and have the "Continue" button be disabled via #states
until these checkboxes have been checked. We can detect maintenance mode and pre-tick the respective checkbox.
Thoughts:
Alternatively, I was wondering if it would be better UX to implement something like #3957, where the "Backup your site/db" thing is a warning, and the maintenance mode is a checkbox that does this for you.
Oh, it looks like I just opened a duplicate of this issue: https://github.com/backdrop/backdrop-issues/issues/3960 or maybe two? https://github.com/backdrop/backdrop-issues/issues/3959
I had the opposite reaction though: Rather than requiring the user to do things that are unnecessary, we should remove the extra bloat from this page and make it easier to use.
I do like https://github.com/backdrop/backdrop-issues/issues/3957 though, making it a warning message might do a better job of getting people to actually do backups :)
...making it a warning message might do a better job of getting people to actually do backups :)
Yeah, it's been always bothering me how in Drupal we've been using bold text to warn people about things, when we have actual warnings for that job.
I've updated the issue summary with a screenshot of the current text; will draft a mockup of how things could be improved, and lets discuss.
...If you are unsure of what these terms mean, contact your hosting provider.
^^ What do we refer to here by "terms"? ...is it the term "database" or "backup"? I honestly do not see the helpfulness/value of this piece of text. I think we should remove it altogether, and simply point people to online documentation on b.org or wikipedia.
I have started a PR: https://github.com/backdrop/backdrop/pull/2806 which includes the changes recommended in #3959 and #3960.
...things do not work as expected yet, so setting to NW. Currently on vacation, so just wanted to dump my WIP in a place where it won't get lost 😅
...update_info_page()
was previously "manually" setting the POST action for the "Continue" button, which was also rendered "manually" ...same as the "Cancel" link. In my PR, I use a proper form, and proper submit/cancel Form API elements.
I need to sort some other problems, but the tricky bit currently is that if I set #action
in the form, then the submit handler does not get called (so we won't be able to set the maintenance mode); but using $form_state['redirect']
in the submit handler does not work either 😓 ...I have googled around a bit, and came across articles like https://drupal.stackexchange.com/questions/96237/form-api-action-is-in-conflict-with-submit and https://www.drupal.org/forum/support/post-installation/2010-08-31/form-action-and-submit-in-the-same-form ...still need to test things and make the code behave.
...OK, I have managed to get things working with $form_state['redirect']
in the submit handler. I now need to work on the logic that sets/unsets the maintenance mode.
If the site is already in maintenance mode before running update.php, I think that we should we be disabling the checkbox in this step, otherwise it may seem that the checkbox allows people to take the site out of maintenance mode:
...and a general question: why do we even show this page/step if there are no updates to begin with? I think that we should be running update_get_update_list()
after the "Verify requirements" step, and then skip to the "Review updates" step, where the No pending updates.
message is shown.
...in fact, I think that the No pending updates.
message does not belong in the "Review updates" step to begin with. If I am at a step called "Review updates", I expect to see updates listed there for my review; if there are no updates, I expect that to be communicated to me in the "Overview" step. I've updated the PR to account for that case. Here's what happens now if a user tries to run update.php w/o any pending updates:
...I am not 100% sure if the logic that shows the same message in update_script_selection_form()
is still required; if not, we should remove it.
I should return to this. Seeing the various inconsistencies here while working on #2858. Also wanted to note that the "Cancel" link is always taking people to the front page, regardless of where they originated from. The most common use case is from the site status report page, when the "Out of date (Run database updates)" error is shown. We should be using $_SERVER['HTTP_REFERER']
to grab the origin page, and then save that and retain it for the other steps in this wizard (either using custom headers or adding it to the $query
parameter in backdrop_current_script_url()
).
Edit: I've created #6289 for that ^^
There are some wires crossed between this issue here and #3245. I'll try to check and "untangle" and unblock these (or close one over the other).
If the PR here is still useful, I'll need to update things after the change from "database updates" to "site updates" landed with #5630.
We should also run a check to see if any updates are pending to begin with (edit: this is being handled separately in #3245). If no updates, then communicate that to the user, instead of asking them to back things up, set the site to maintenance mode, and continue.
Nitpicking, I know, but these are the suggested steps as it is now:
Related:
Status (Aug 2019)
The user gets the following, whether there are pending updates or not:
Proposed change
If there are no pending updates:
If there are updates pending:
If there are pending updates, but the site has been set to maintenance mode before running update.php:
PR by @klonos: https://github.com/backdrop/backdrop/pull/2806