Currently the configuration form exists as a separate from from the core maintenance mode form. Logically, it should be either appended to the core maintenance mode as a form alter or at least a local task (tab) in the maintenance mode form.

CommentFileSizeAuthor
#10 i3074238-9.patch3.43 KBattiks
#7 i3074238-7.patch3.42 KBattiks
#5 i3074238-3.patch650 bytesbarrett
#2 i3074238.patch1.04 KBattiks

Comments

Barrett created an issue. See original summary.

attiks’s picture

attiks commented
Status: Active » Needs review
StatusFileSize
new i3074238.patch1.04 KB

Title is also added, see https://www.drupal.org/project/maintenance200/issues/3092992

Status: Needs review » Needs work

The last submitted patch, 2: i3074238.patch, failed testing. View results

attiks’s picture

attiks commented
Parent issue: » #3134141: Create a stable release
barrett’s picture

barrett commented
StatusFileSize
new i3074238-3.patch650 bytes

Rerolling the previous patch since the change to routing was already made in a previous patch.

It would be great to have tests in the patch to verify the configuration tab appears in the expected location.

attiks’s picture

attiks commented
Assigned: Unassigned » attiks
attiks’s picture

attiks commented
Status: Needs work » Needs review
StatusFileSize
new i3074238-7.patch3.42 KB

Test added

barrett’s picture

barrett commented
Status: Needs review » Needs work

Thanks, @attiks. This is a good start but it needs a couple adjustments.

First, the test itself passes even when the change to the placement of the form isn't applied (i.e., the test passes whether the form is standalone or appears as a tab on the maintenance page). Essentially the test as written tests that there is a form at the module specified route and that it generally works.

Second, and much smaller, the test needs a description which distinguishes it from the tests in Maintenance200Test. Currently both sets of tests have a description of, "
Test maintenance 200."

attiks’s picture

attiks commented

I'll update the comment, but not sure if a test is needed for the task, since it's enforced by the config?

attiks’s picture

attiks commented
Status: Needs work » Needs review
StatusFileSize
new i3074238-9.patch3.43 KB

Patch for comment

It seems there's no easy way to see if it's a tab or stand-alone page, I might be able to see if there's another tab present, but not sure if it's worth it

  • Barrett committed 3f12513 on 8.x-1.x authored by attiks 
    Issue #3074238 by attiks, Barrett: Move config form to be part of...

  • Barrett committed 7cb84c7 on 8.x-1.x 
    Issue #3074238 by attiks, Barrett: Move config form to be part of...
barrett’s picture

barrett commented
Status: Needs review » Fixed

I merged your patch and just pushed a separate commit that adds the test for the tab on the maintenance page.

attiks’s picture

attiks commented

Nice!

Status: Fixed » Closed (fixed)

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