- π§πͺBelgium stefdewa
On D10.2.2 I got
Symfony\Component\DependencyInjection\Exception\ServiceCircularReferenceException: Circular reference detected for service "maintenance_mode_subscriber"
because of the overriding maintenance_mode service.Added a new patch (and interdiff) for a fix.
It is probably better to use a service decorator here instead of overriding the service. Leaving that suggestion here for another day/person.
- π§πͺBelgium stefdewa
Bumping version so I can create a merge request for the active development branch.
- Merge request !68Override maintenance mode service with domain specific config. β (Open) created by stefdewa
- π§πͺBelgium stefdewa
Stefdewa β changed the visibility of the branch 2857402-domain-access-for to hidden.
- First commit to issue fork.
- π«π·France jykin
Hi,
I have rebased the fork on the 2.0.x branch. After testing, I can confirm that now I can put a single site into maintenance mode without any errors.
- π«π·France Asterovim Paris
Hello. I can confirm that the rebase of Jykin is OK ! Thanks !
- π³π±Netherlands boazpoolman
Created a patch file of the MR at time of writing this comment.
Thanks for all the hard work!
- π³π±Netherlands boazpoolman
Patch #21 - and in extend MR 68 - seemed to work fine at first, but I did encounter an issue.
When the patch is applied, and I try to put a site in to maintenance mode through drush, it does not work.
The command I ran:drush state:set system.maintenance_mode 1 && drush cr
This caused our deployment pipelines to fail because we rely on this command to put all the sites in maintenance during the deployment.
- πΊπΈUnited States kevinquillen
This seems to work only some of the time - as in #22 it fails to apply when All Domains is set and an actual Domain config does not exist for maintenance mode. Also, sometimes it looks like the form appears to 'stick' for domains where toggling it on also toggles All Domains value.
- πΊπΈUnited States kevinquillen
With this patch, are the configuration settings saved into the correct Domain Config record? It just looks like one value is updated. If I tick off maintenance mode for one site, but not another, both wind up in maintenance mode.
Additionally, I think DomainMaintenanceMode service needs additional logic:
/** * {@inheritdoc} */ public function applies(RouteMatchInterface $route_match) { if (!$this->state->get('system.maintenance_mode')) { return FALSE; } if (!$this->config->get('domain_config_ui.domain_settings')->get('maintenance_mode') && !$this->state->get('system.maintenance_mode')) { return FALSE; } if ($route = $route_match->getRouteObject()) { if ($route->getOption('_maintenance_access')) { return FALSE; } } return TRUE; }
Meaning:
- MM is enabled, if the global is enabled (supports core behavior) - here the Domain specific setting should not matter
- MM is enabled, if the domain has it enabled and the global is OFF
Domains should be able to have this enabled independently.
- Merge request !145Issue #2857402: Rework patch to be based on state values and not config. β (Open) created by kevinquillen
- π«π·France mably
Looks interesting to me, but I feel like the feature should probably be added in its own submodule, what do you think?
The MR needs to be rebased, it's 32 commits behind.
- πΊπΈUnited States kevinquillen
I don't follow all the posts in this thread, but there seems to be some confusion around the maintenance mode. As #8 says, maintenance mode is a state value, not a config value.
I reworked some of the ideas in this thread into a new branch and MR:
https://git.drupalcode.org/project/domain/-/merge_requests/145.diff
This puts the value on state, not in config. When saving the maintenance mode form, 'All Domains' value will use the core state setting, where individual domains will use a new state key of 'domain.' domain_id . 'system.maintenance_mode'.
In my testing, this made the following scenarios work:
- the maintenance mode form to respect domain overrides
- the form must use state value, not config value
- the state must save on change
- the state must also be able to be reflected with drush and set by drush (for global or site)
- the controller needs to handle all of those cases and show, or not show, the sites based on their respective state value
If all domains is set and maintenance mode is on, its assumed to be on for all sites. If it is off, but individual sites have it on, it should disable those sites for users without permission to access.
I am not sure it will work in every single Domain case here, but it worked for our needs (turn specific sites on or off with maintenance mode, without disabling the Domain).
- πΊπΈUnited States kevinquillen
Test needs updating to include the negotiator service call for DomainMaintenanceMode, but will likely pass once mocked and test scenario updated.