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

[UX] Updates via the UI: Do not take the site out of maintenance mode if there are db updates pending #4320

Open
klonos opened this issue Feb 26, 2020 · 5 comments

Comments

@klonos
Copy link
Member

klonos commented Feb 26, 2020

Reported by @bradbulger in the forum: https://forum.backdropcms.org/forum/module-updates-and-maintenance-mode

...I noticed that when you use the admin UI to update contributed modules, it defaults to putting the site into maintenance mode, downloads and installs the code, and then takes the site out of maintenance mode - before running any pending database updates. Shouldn't it wait until after the database updates have been run to leave maintenance mode?

We seem to have missed this scenario in #3550.

@laryn
Copy link
Contributor

laryn commented Feb 26, 2020

#3550 was simply getting it functioning the way it does in Drupal 7 -- but this seems like a good idea, I think.

@yorkshire-pudding
Copy link
Member

yorkshire-pudding commented Apr 15, 2022

I'd like to see this happen. I mentioned it in #3245 ; here's what I said:

It advises you to put into maintenance mode to install the module/update and gives you a handy tick box to do it. Then it takes you out of maintenance mode. It then (let's assume there are database updates) says to do the database updates and recommends they're done in maintenance mode, however there is no option to re-enable it; you would need to come out re-enable maintenance mode then go back and activate the database updates.

What it should do, taking into account the above:

  1. Download modules/updates
  2. Recommend to put into maintenance mode and keep the option
  3. If there are database updates, keep it in maintenance mode
  4. Once database updates are done take out of maintenance mode

From what I can tell, then lines that take it out of maintenance mode:
https://github.com/backdrop/backdrop/blob/2726808b51b49108b6b37a9a8e362989d72eb3f4/core/modules/installer/installer.authorize.inc#L204-L211

Need to be moved to between lines 248-251 in @laryn 's PR for #3245

 elseif ($success) { //L248
    backdrop_set_message($page_message['message'], $page_message['type']);
    backdrop_goto('admin/config/system/updates');
  } //L251

And also somewhere in /core/update.php, perhaps in function update_results_page() ?

https://github.com/backdrop/backdrop/blob/2726808b51b49108b6b37a9a8e362989d72eb3f4/core/update.php#L205-L283

Happy to have a go at this, but would welcome a steer on the update.php bit @laryn @klonos

I think this would be good to go alongside #3245 if it can be done in time.

@rbargerhuff
Copy link

rbargerhuff commented Jun 23, 2023

I was just wondering what the status is on this because this issue has been sitting for a year without any update.

I think the community is in agreement that this behavior is not ideal. Also, this issue introduces the user to potential site errors if a module is installed that requires some kind of database update and the site is visited before the updates are carried out. For example, a hook is implemented that is executed on every page.

@laryn
Copy link
Contributor

laryn commented Jun 23, 2023

@yorkshire-pudding I agree this would pair nicely with #3245 -- if there are database updates it could go straight to those while still in maintenance mode (and then take you out of maintenance mode after updates are complete); if there are no database updates it could skip that part and take you out of maintenance mode automatically.

I'm unassigning myself on #3245 since I won't have time to get back to it for some time. I know @argiepiano took a crack at that one too so may have some input here for you, if you're still interested in working on these @yorkshire-pudding.

@rbargerhuff I think the status is needs work!

@izmeez
Copy link

izmeez commented Jan 24, 2024

I'm going to bump this issue up because it is important even though it may be nestled with other issues, which may be a good thing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants