Allow setting default_handler as a session preference

[Ryan J Ollos]

The proposed feature will make it possible to set the default_handler from the Preferences page or using a TracAdmin command (trac-admin session set). The feature was previously mentioned in comment:17:ticket:11519.

Together with an enhancement to be proposed to the ​th:AccountManagerPlugin, this will support convenient configuration of a use-case for which I've previously used the ​th:GroupBasedRedirectionPlugin - having members of groups/teams arrive at different pages when logging in to Trac.

[Ryan J Ollos]

The core functionality has been implemented in log:rjollos.git:t11597. However, I'll add tests before committing.

[Jun Omae]

I don't think the selected handler is always valid for default handler. I think we should check whether the handler is valid.

trac/web/main.py

@@ -276 +276 @@
         # Use default_handler from the Session if it is a valid value.
         name = req.session.get('default_handler')
         handler = self._request_handlers.get(name)
+        if handler and not is_valid_default_handler(handler):
+            handler = None
 
         if not handler:
             # Use default_handler from project configuration.

[Ryan J Ollos]

Good catch. I will include it and make sure we have test coverage.

[Ryan J Ollos]

log:rjollos.git:t11597.2 contains some refactoring of the proposed changes. Functional tests still need to be added.

[Ryan J Ollos]

Proposed changes in log:rjollos.git:t11597.3, with functional and unit tests.

[Ryan J Ollos]

Committed to trunk in [12922].

[Jun Omae]

I noticed missing i18n:msg attribute in trunk/trac/prefs/templates/prefs_general.html@12922#L51​ while checking [12931].

-            <option value="">Default ($project_default_handler)</option>
+            <option value="" i18n:msg="handler">Default ($project_default_handler)</option>

[Ryan J Ollos]

Thanks for catching that. Fixed in [12949].

[Jun Omae]

I think we should have the same extracted message in both trunk/trac/admin/templates/admin_basics.html@13020:57#L55​ and trunk/trac/prefs/templates/prefs_general.html@13020:62#L60​. I noticed that while translating catalog.

trac/admin/templates/admin_basics.html

@@ -53 +53 @@
             </select>
           </label>
           <span py:if="default_handler not in valid_default_handlers"
-                class="hint" i18n:msg="default_handler">
+                class="hint" i18n:msg="handler">
             $default_handler is not a valid IRequestHandler or is not enabled.
           </span>
         </div>

trac/prefs/templates/prefs_general.html

@@ -58 +58 @@
                     value="$handler">$handler</option>
           </select>
           <span py:if="default_handler not in valid_default_handlers"
-                class="hint" i18n:msg="session_default_handler">
+                class="hint" i18n:msg="handler">
             $default_handler is not a valid IRequestHandler or is not enabled.
           </span>
         </td>

[Jun Omae]

At trunk/trac/web/session.py@13020:453,457,464#L451​, it's inconsistent that those messages use single quote and double quote characters. It would be consistent to use only single quote.

-                raise AdminCommandError(_('Invalid default_handler "%(val)s"',
+                raise AdminCommandError(_("Invalid default_handler '%(val)s'",

[Ryan J Ollos]

Replying to jomae:

At trunk/trac/web/session.py@13020:453,457,464#L451​, it's inconsistent that those messages use single quote and double quote characters. It would be consistent to use only single quote.

Fixed in [13026], along with issue from comment:11. New extraction on trunk in [13027]. Thanks for catching the issues.

At some point it would be good to establish a convention on the use of single vs double quotes in messages since there is inconsistency in the codebase. Here is one of many cases where the identifier is wrapped in double-quotes trunk/trac/wiki/web_ui.py@:119-120#L114​. Overall that's not a big deal though. Also I was thinking we might try to use markup to put emphasis on identifiers rather than single or double quotes, and then adjust the styling through CSS:

raise AdminCommandError(tag_("Invalid default_handler %(val)s", val=tag.em(val)))

[Ryan J Ollos]

The effects of the changes in this ticket on a trac plugin have been reported in ​th:#11905. ​th:TracDeveloperPlugin was abusing its internal knowledge of the request handling mechanism a bit, so I don't feel too bad about the consequences of this change.

[Ryan J Ollos]

#11813 will consider moving the Default handler preference to another tab.

[figaro]

Integrating the ​​th:GroupBasedRedirectionPlugin into the ​​th:AccountManagerPlugin was discussed in ​th:#9943. The GroupBasedRedirectionPlugin is deprecated.

[Ryan J Ollos]

Revised default_handler documentation updated in r15824.