Edgewall Software
Modify

Opened 5 years ago

Closed 4 years ago

Last modified 20 months ago

#11597 closed enhancement (fixed)

Allow setting default_handler as a session preference

Reported by: Ryan J Ollos Owned by: Ryan J Ollos
Priority: normal Milestone: 1.1.2
Component: general Version:
Severity: normal Keywords: session, preferences, default_handler
Cc:
Release Notes:

The default_handler can be set as a session preference.

API Changes:

Description

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.

Attachments (0)

Change History (17)

comment:1 Changed 5 years ago by Ryan J Ollos

Status: newassigned

comment:2 Changed 5 years ago by Ryan J Ollos

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

comment:3 Changed 5 years ago by 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

    diff --git a/trac/web/main.py b/trac/web/main.py
    index 3303337..6574b66 100644
    a b class RequestDispatcher(Component):  
    276276        # Use default_handler from the Session if it is a valid value.
    277277        name = req.session.get('default_handler')
    278278        handler = self._request_handlers.get(name)
     279        if handler and not is_valid_default_handler(handler):
     280            handler = None
    279281
    280282        if not handler:
    281283            # Use default_handler from project configuration.

comment:4 Changed 5 years ago by Ryan J Ollos

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

comment:5 Changed 4 years ago by Ryan J Ollos

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

comment:6 Changed 4 years ago by Ryan J Ollos

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

comment:7 Changed 4 years ago by Ryan J Ollos

Release Notes: modified (diff)
Resolution: fixed
Status: assignedclosed

Committed to trunk in [12922].

comment:8 Changed 4 years ago by 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>

comment:9 Changed 4 years ago by Ryan J Ollos

Thanks for catching that. Fixed in [12949].

comment:10 Changed 4 years ago by 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

    diff --git a/trac/admin/templates/admin_basics.html b/trac/admin/templates/admin_basics.html
    index 310eb58..472d9fc 100644
    a b ${project.descr}</textarea>  
    5353            </select>
    5454          </label>
    5555          <span py:if="default_handler not in valid_default_handlers"
    56                 class="hint" i18n:msg="default_handler">
     56                class="hint" i18n:msg="handler">
    5757            $default_handler is not a valid IRequestHandler or is not enabled.
    5858          </span>
    5959        </div>
  • trac/prefs/templates/prefs_general.html

    diff --git a/trac/prefs/templates/prefs_general.html b/trac/prefs/templates/prefs_general.html
    index 44af41c..c40a27d 100644
    a b  
    5858                    value="$handler">$handler</option>
    5959          </select>
    6060          <span py:if="default_handler not in valid_default_handlers"
    61                 class="hint" i18n:msg="session_default_handler">
     61                class="hint" i18n:msg="handler">
    6262            $default_handler is not a valid IRequestHandler or is not enabled.
    6363          </span>
    6464        </td>

comment:11 Changed 4 years ago by 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'",

comment:12 in reply to:  11 Changed 4 years ago by 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)))

comment:13 Changed 4 years ago by Ryan J Ollos

Keywords:

comment:14 Changed 4 years ago by 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.

Last edited 4 years ago by Ryan J Ollos (previous) (diff)

comment:15 Changed 4 years ago by Ryan J Ollos

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

comment:16 Changed 2 years ago by figaro

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

comment:17 Changed 20 months ago by Ryan J Ollos

Revised default_handler documentation updated in r15824.

Last edited 20 months ago by Ryan J Ollos (previous) (diff)

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Ryan J Ollos.
The resolution will be deleted.
to The owner will be changed from Ryan J Ollos to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.