Edgewall Software
Modify

Opened 6 years ago

Closed 6 years ago

#11663 closed enhancement (fixed)

Milestone edit page should more clearly show when milestone doesn't exist

Reported by: Ryan J Ollos Owned by: Ryan J Ollos
Priority: normal Milestone: 1.0.2
Component: roadmap Version:
Severity: normal Keywords:
Cc: Branch:
Release Notes:

On navigating to a non-existent milestone and being redirected to the Milestone edit page, a notice is displayed to clearly indicate that the milestone does not yet exist.

API Changes:
Internal Changes:

Description

When navigating from the Add new milestone button on the Roadmap page the path will be /milestone?action=new and the user should have a good idea that the milestone does not yet exist (although it might be less clear when &name=milestoneX is appended to the path resulting in the milestone name input having text).

In the case that a link is followed to a non-existing milestone the path will be /milestone/milestoneX and it might be less clear that the milestone does not yet exist. We could give a clear indicator in this case:

The patch would be simple, and would not add a notice for the case that was first described, when the user follows the Add new milestone link.

  • trac/ticket/roadmap.py

    commit 97d3d7d9c4be58309aa696474bf45dd5a749a605
    Author: rjollos <ryan.j.ollos@gmail.com>
    Date:   Tue Jul 1 10:23:37 2014 -0700
    
        Add notice if milestone doesn't exist.
    
    diff --git a/trac/ticket/roadmap.py b/trac/ticket/roadmap.py
    index ab4c41d..bb53ea5 100644
    a b class MilestoneModule(Component):  
    860860            data['milestone_groups'] = group_milestones(milestones,
    861861                'TICKET_ADMIN' in req.perm)
    862862        else:
     863            if milestone.name:
     864                add_notice(req, _("Milestone %(name)s does not exist. You can"
     865                                  " create it here.", name=milestone.name))
    863866            req.perm(milestone.resource).require('MILESTONE_CREATE')
    864867
    865868        chrome = Chrome(self.env)

Attachments (1)

NonExistingMilestone.png (33.0 KB ) - added by Ryan J Ollos 6 years ago.

Download all attachments as: .zip

Change History (7)

by Ryan J Ollos, 6 years ago

Attachment: NonExistingMilestone.png added

comment:1 by Ryan J Ollos, 6 years ago

One side note: I noticed a while back at a few places in the codebase some multiline strings that always placed the whitespace on the left hand side (like shown in my patch above):

"This is a multi-line string"
" with the whitespace on the"
" left on each continuation line"
" which seems to make it less likely to omit"
" a whitespace and inadvertently join two words."

as opposed to,

"This is a multi-line string "
"with the whitespace on the "
"left on each continuation line "
"which seems to make it less likely to omit "
"a whitespace and inadvertently join two words."

Maybe that is a good practice to add to TracDev/CodingStyle#Strings? Any opinions?

comment:2 by Jun Omae, 6 years ago

I usually use the latter. It is commonly-used in the codebase.

$ grep -rE "^ *u?['\"] " --include='*.py' trac tracopt | wc -l
47
$ grep -rE " ['\"] *\\\\?$" --include='*.py' trac tracopt | wc -l
483

comment:3 by Ryan J Ollos, 6 years ago

I have usually used the later as well. That doesn't form an argument for why it's a better style though.

comment:4 by Jun Omae, 6 years ago

Adding the notice sounds good to me. But I guess that we should add it after checking priviledges. Otherwise, it would be shown in PermissionError page.

             req.perm(milestone.resource).require('MILESTONE_CREATE')
+            if milestone.name:
+                add_notice(req, _("Milestone %(name)s does not exist. You can"
+                                  " create it here.", name=milestone.name))

comment:5 by Ryan J Ollos, 6 years ago

Milestone: next-stable-1.0.x1.0.2
Owner: set to Ryan J Ollos
Status: newassigned

Thanks for spotting that issue. Proposed changes in log:rjollos.git:t11663. I'm anticipating that RequestHandlerPermissionsTestCaseBase will be reused in numerous test case modules as we write more unit tests and depend less on functional tests.

comment:6 by Ryan J Ollos, 6 years ago

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

Committed to 1.0-stable in [12910:12911], merged to trunk in [12912:12913].

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Ryan J Ollos.
The resolution will be deleted. Next status will be 'reopened'.
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.