Configure maintenance and lockout message and visibility

Feel free to submit a patch, zhangtaihao. There's similar behavior in my old Admin Only Mode module (hosted at github at the moment) that you may be able to get implementation ideas from.

It's a neat implementation, but I'm having trouble understanding why we want this level of per-page configurability.

When "restrict logins" is set, this module:

1. Logs out users not having the "login during restricted logins" permission, and
2. Allows other users to login/remain logged in.

I see three obvious maintenance message locations one might want, in order of likeliness:

1. Show message for still-authenticated users only, not for anonymous users (e.g., those just logged out)
2. Show only on login screen
3. Show everywhere for everyone

I propose that in all cases, the message should be shown when logging someone out and the login screen -- that's just a courtesy.

It seems to me like it's more interesting *who* sees the message, than *where* the message is shown.

Do your needs align with the above, zhangtaihao, or am I off base here?

Would it work to just show the maintenance message right after the "Sorry, you do not have the permission to login at this time." message?

I suggest the config page could simply be:

• [x] Restrict Logins
• Maintenance Message (leave empty for none)
• [x] Show Maintenance Message to users without "login during restricted logins" privilege

We would always show the non-empty maintenance message when logging someone out and on the user login page (This would satisfy all three cases from comment #7 above).

zhangtaihao, I guess I want to hear what you're trying to accomplish with this module. Features for the sake of features don't make sense to me (complicates both configuration and code), but if you're trying to do something specific that others will want to do too, then we should add such configurability. Do you have a set of sites where something like the above simple configuration would be insufficient?

Alright, if we change #11's "Show message for anonymous" to 'Show message for users without "login during restricted logins" permission' then +1 from me, assuming that the "Show maintenance message on specific pages" either defaults to /user or /user is included regardless of this setting.

Thanks for working through this, zhangtaihao, and especially for the patches!

Your code looks very good, zhangtaihao -- many good general code improvements in there.

I want to test it, but your patch doesn't apply to 6.x-2.x-dev, and your git repository is broken (can't be cloned or the tree viewed online). Can you re-roll against 6.2-2.x-dev?

Also, the new maintenance_helper.install file should use UNIX line-endings (\n) instead of DOS ones (\r\n).

Lastly, let's add Drush support for these new features as part of this same patch. I can work on that if you don't want to.

Amazing work, zhangtaihao.

I don't have time for a full review at the moment, but I want to write these few things down while I see them.

1. At admin/settings/maintenance_helper I'm getting
   warning: Invalid argument supplied for foreach() in /inet/www/drupal-dev/htdocs/includes/form.inc on line 1212.

2. @@ -73,12 +84,31 @@ function maintenance_helper_admin_settings() {
   +  /*
   

   Code should be removed rather than commented out.

3. +    $pages = variable_get('maintenance_helper_message_pages', '');
   

   This default value should match the value set in maintenance_helper_install() (occurs several places in the patch)

4. +php = "4.0.6"
   \ No newline at end of file
   

   Needs newline.

You were right, uninstall/reinstall fixed this by setting the default values (which is what the foreach at line 1212 was looping through). I'm fine with not creating an update, but we aught to document in the README and/or CHANGELOG to uninstall/reinstall (or perhaps just save the settings page once?) to remove the error message if upgrading.

More review:

1. $ drush mh-restrict-logins 1
   --show-message-on can only be set to 0 or 1.
   

   show-message-on should be optional (also, --show-message-everywhere should work without other options set -- it currently gives this same error)

2. $ drush mh-restrict-logins 1 --show-message-on=1
   --show-message-to can only be set to 0, 1, or 2.
   

   show-message-to should be optional

3. drush @engg-dev mh-restrict-logins 1 --show-message-on=1 --show-message-to=2
   Show message to users with the 'login during restricted logins' permission.
   Show message on every page except: .
   restricted logins enabled.
   

   A few things here

   1. --show-message-pages should be optional. Settings --show-message-on without setting --show-message-pages should result in the message showing on previously-configured pages. Without this, you lose your carefully crafted standard set of pages setup on the module setttings page.
   2. The drush help says --show-message-on=0 means "all pages except", and =1 means "only these pages", but the current actual behavior is the reverse.
   3. "restricted logins enabled." -- Restricted should be capitalized.
   4. Should we add a --show-message-nowhere flag to match the --show-message-everywhere one? I wanted to try having the maintenance message show up nowhere, and it took me a while to figure out how (answer: set --show-message-on to "only" and set --show-message-pages to "").
4. Comment above "maintenance_helper_set_message_users()" says:
   + * Sets the maintenance message -- used by drush

5. Ensure when using "variable_get()" that the default values match those set in maintenance_helper_install() -- I think this is all of them:

   In maintenance_helper_init():
       $visibility = variable_get('maintenance_helper_message_show', 0);
     $restrict_logins = variable_get('maintenance_helper_restrict_logins',FALSE);
   In maintenance_helper_admin_settings()
       '#default_value' => variable_get('maintenance_helper_message_users', 1),
       '#default_value' => variable_get('maintenance_helper_message_show', 0),
   

Thanks for the fast and great work, zhangtaihao!

Oh yeah, I forgot to mention this non-standard encoding in the patch of maintenance_helper.install:

+<?php

A note on the project page sounds fine to me! Yes, all of the parameters should be optional.

Looking good!

+      '--show-message-everywhere' => dt("Show message on all pages. Specifying this will override any option set in --show-message-on. This option has the effect of setting --show-message-on to 0 and --show-message-pages to empty.")

This needs updated to use the new syntax. I hope to install this and test more this week. Thanks!

Review of #30 -- working very well except for some drush problems:

1. The maintenance message defaults to being shown to nobody. The default should be everybody; otherwise no maintenance message shows anywhere when the module is first enabled, which is confusing.

2. $drush @cecs-dev mh-restrict-logins 1 --show-message-to=all --show-message-everywhere
   Show message to all users.
   WD php: explode() expects parameter 2 to be string, array given in                       [error]
   /home/claar/mh/6.x-2.x/maintenance_helper.drush.inc on line 112.
   WD php: array_filter() expects parameter 1 to be array, null given in                    [error]
   /home/claar/mh/6.x-2.x/maintenance_helper.drush.inc on line 112.
   

3. $drush @cecs-dev mh-restrict-logins 1 --show-message-to=all --show-message-on=only --show-message-pages=
   Show message to all users.
   WD php: implode(): Invalid arguments passed in                                           [error]
   /home/claar/mh/6.x-2.x/maintenance_helper.drush.inc on line 117.
   Show message on only: .
   Restricted logins enabled.
   

4.     if (isset($show_message_pages)) {
         $pages = array_filter(explode(':', $show_message_pages));
       }
       $pages = maintenance_helper_set_show_message($show_message_on, $pages);
   

   $pages might not be defined here

5. --show-message-on=only doesn't seem to work

Thanks for the fast responses and improvements! Latest review:

1. My point #5 in Comment #30 was me being an idiot -- I needed to CTRL-refresh my browser to make the form update
2. Maintenance message shouldn't be shown if restrict logins is disabled. Showing a maintenance message without restricting logins would be a new feature -- create an issue for that if desired.
3. Let's choose opposing words for show-message-to. Either use "all" and "none", "everyone" and "noone", or "everybody" and "nobody". Your choice.

Drush is working perfectly now -- thanks for all the tweaks.

Needs D7 port.