Add note to log when site put into and taken out of Offline mode

Hi
I just crossed the perfect example on why this issue should be taken much seriously: a admin user saw the "outdated message", so thinking doing the right thing he follows the link and follow the update process putting the site in maintenance mode... The update failed but he didn't notice was still in maintenance mode because he was already logged in.
He do it twice over several months...
It take a while to debug it since there is no drupal log, and webserver log didn't show any visit to maintenance page.

Ok, the fault is mainly on the dev team side (update module enable on production site and bad permissions) but still I think this issue shoul get a higher priority (or the first patch should be accepted)

See also: https://github.com/drush-ops/drush/issues/5365

FWIW the last patch still applies cleanly against 9.5.x.

Thanks, I just add it to my `composer.json`.

So the patch number 17 could be marked as RTBTC

The patch still applies cleanly. I don't see any outstanding requests for changes. Switching this to 'needs review.'

The issue summary is pretty bare.

Also could use some kind of test coverage. Even simply coverage.

I am running core version 9.5.11 I tested the patch from #21. I was not able to get anything in logs when setting the maintenance mode through the UI. I tried clearing caches with Drush a few times but it didn't have any effect.

Patch from #17 also applies cleanly and now I got log messages when it was set. But I think the solution in #21 would be better if it worked because we also set maintenance mode with Drush and would be good to catch those also.

As maintenance mode is stored in state, it is good to log message in Drupal\Core\State::set() method. So, moving the fix given in #21 to State.php. This way, there will be a log for both cases - drush and form save. Verified the attached patch with Drupal 10.1.4

@sumit_saini Thanks for the patch - tests are still needed, so moving back to Needs Work.

Aditi Saraf → made their first commit to this issue’s fork.

The last patch is still missing the test(s).

And it would be great to update to update the issue summary (see #42). We should also include the summary of possible solutions (State::set() vs CacheCollector:set() vs SiteMaintenanceModeForm ..), as there are multiple patches with different approaches, so better to have this summarized. Thanks!

Rerolled.

For the issue summary update and tests.

We encountered an error when using this patch in core with Redirect 404 submodule of Redirect.

After upgrading to Drupal 10.2, I get the following error when trying to disable the maintenance mode in en/admin/config/development/maintenance:

The website encountered an unexpected error. Try again later.

TypeError: Drupal\system\Form\SiteMaintenanceModeForm::__construct(): Argument #1 ($logger_factory) must be of type Drupal\Core\Logger\LoggerChannelFactory, Drupal\redirect_404\Render\Redirect404LogSuppressor given, called in /var/www/html/web/core/modules/system/src/Form/SiteMaintenanceModeForm.php on line 74 in Drupal\system\Form\SiteMaintenanceModeForm->__construct() (line 58 of core/modules/system/src/Form/SiteMaintenanceModeForm.php).

Drupal\system\Form\SiteMaintenanceModeForm::create(Object) (Line: 28)
Drupal\Core\DependencyInjection\ClassResolver->getInstanceFromDefinition('\Drupal\system\Form\SiteMaintenanceModeForm') (Line: 48)
Drupal\Core\Controller\HtmlFormController->getFormObject(Object, '\Drupal\system\Form\SiteMaintenanceModeForm') (Line: 58)
Drupal\Core\Controller\FormController->getContentResult(Object, Object)
call_user_func_array(Array, Array) (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 627)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 124)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) (Line: 97)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 181)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 76)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 58)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 28)
Drupal\Core\StackMiddleware\ContentLength->handle(Object, 1, 1) (Line: 106)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 85)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 36)
Drupal\Core\StackMiddleware\AjaxPageState->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\StackedHttpKernel->handle(Object, 1, 1) (Line: 704)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

I’m not convinced by the solution proposed in #21. While it’s nice that the message is logged when the site is put into maintenance mode using Drush, I don’t believe the State class should be responsible for that. Perhaps we could handle it in Drupal\Core\EventSubscriber\MaintenanceModeSubscriber by introducing a second state variable to store the last logged action, which would help prevent log flooding.

In the meantime, I’ve refined the SiteMaintenanceModeForm solution slightly and added a kernel test.

Do you think we should invest effort into covering the Drush case?

Left some small comments on the MR.

Thanks for the review @smustgrave :)

All threads resolved!