Problem/Motivation

We need a way to put a Drupal site in and out of maintenance mode in a fully decoupled admin UI.

Proposed resolution

Option 1:
Add a single "maintenance_mode.enabled" method with a "enabled" boolean parameter to the JSON RPC methods.

Option 2:
Add 2 seperate methods without parameters:
- "maintenance_mode.enable"
- "maintenance_mode.disable"

Remaining tasks

- Decide which option is the most desirable
- Add maintenance_mode method(s) to jsonrpc_core module methods
- Write tests
- Document request and response structure
- Review
- Commmit

User interface changes

None.

API changes

- The discovery will report an aditional method, "maintenance_mode"
- The /jsonrpc endpoint will accept requests for the "maintenance_mode" method

Data model changes

None.

CommentFileSizeAuthor
#8 interdiff.txt2.87 KBdinesh18
#8 2983907-8.patch6.69 KBdinesh18
#6 maintenance_mode_enabled_2983907_5.patch6.75 KBrogierbom

Comments

rogierbom created an issue. See original summary.

rogierbom’s picture

rogierbom commented
Issue summary: View changes
gabesullice’s picture

gabesullice
Primary language English
commented
Issue tags: +API-First Initiative

Hi @rogierbom! Great idea and great issue!

I think my preference would be for option #1, but changing "state" to "enabled" so it's a little clearer whether it's on or off. We should be sure that a client can send TRUE, even if it is already TRUE in order to keep the method idempotent.

One larger question: perhaps we should generalize this even more and just make a State API service rather than specific methods for each state variable. What do you think?

e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
commented

+1 to this issue.

One larger question: perhaps we should generalize this even more and just make a State API service rather than specific methods for each state variable. What do you think?

I'm very tempted by this suggestion. The only inconvenience will be that we won't be able to have a consistent schema for the RPC method, which is unfortunate. This is because the values in the keyval store have different (and unanticipated) shapes. I don't think this is a blocker, but maybe we can make it work.

rogierbom’s picture

rogierbom commented
Issue summary: View changes
rogierbom’s picture

rogierbom commented
Status: Active » Needs review
StatusFileSize
new maintenance_mode_enabled_2983907_5.patch6.75 KB

Here is an initial patch that implements option 1, with the remarks of @gabesullice. The test breaks on a validation issue in the JsonRpcMethodManager, I wil file a separate issue for this.

I don't know how I feel about just exposing the State API and using that endpoint to perform actions like this one. From the perspective of a decoupled admin UI it makes sense. But on the other hand it would require other (non-Drupal) consumers to have knowledge of the internal structure of the State API to perform these actions. I myself would prefer a dedicated and clear method for this.

e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
commented
Status: Needs review » Needs work

Thanks for the patch @rogierbom! This looks really good. Some minor details:

  1. +++ b/modules/jsonrpc_core/src/Plugin/jsonrpc/Method/MaintenanceModeEnabled.php
@@ -0,0 +1,92 @@
+ *   id = "maintenance_mode.enabled",

    Maybe "maintenance_mode.isEnabled" 

  2. +++ b/modules/jsonrpc_core/src/Plugin/jsonrpc/Method/MaintenanceModeEnabled.php
@@ -0,0 +1,92 @@
+    $result = 'enabled';
+    if (!$enabled) {
+      $result = 'disabled';
+    }
+
+    return sprintf('The maintenance page has been %s.', $result);

    Let's either return 204 or the actual status of the state:

    $this->state->get('system.maintenance_mode');

  3. +++ b/modules/jsonrpc_core/tests/src/Functional/MaintenanceModeEnabledTest.php
@@ -0,0 +1,68 @@
+    'jsonrpc_discovery',

    How is this dependency used? 

  4. +++ b/modules/jsonrpc_core/tests/src/Functional/MaintenanceModeEnabledTest.php
@@ -0,0 +1,68 @@
+    $this->drupalLogin($account);
...
+    // Retry request with basic auth.

    Do we need to login if we are going to use basic auth? 

  5. +++ b/modules/jsonrpc_core/tests/src/Functional/MaintenanceModeEnabledTest.php
@@ -0,0 +1,68 @@
+

    DCS Nit: extra line break. 

  6. +++ b/tests/src/Functional/JsonRpcTestBase.php
@@ -0,0 +1,49 @@
+   *   The user to be used for authentication.

    The user to be used for Basic Auth authentication. 

  7. +++ b/tests/src/Functional/JsonRpcTestBase.php
@@ -0,0 +1,49 @@
+  }
+}

    DCS Nit: missing line break.

I wonder if we should at have a base class that contains the code that would be reused in other State API related RPC mehtods.

dinesh18’s picture

dinesh18 commented
Status: Needs work » Needs review
StatusFileSize
new 2983907-8.patch6.69 KB
new interdiff.txt2.87 KB

Fixed as per the comment #7.
Attached updated patch and interdiff

  • e0ipso committed eff6c53 on 8.x-1.x authored by Dinesh18 
    Issue #2983907 by Dinesh18, rogierbom, e0ipso, gabesullice: Add method...
e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
commented
Status: Needs review » Fixed

Merged, tagged and released. Thanks for you awesome contribution!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.