Edgewall Software
Modify

Opened 10 years ago

Closed 9 years ago

Last modified 7 years ago

#8548 closed defect (fixed)

TICKET_EDIT_DESCRIPTION requires TICKET_MODIFY rights

Reported by: Michael <michael.lecomte@…> Owned by: Christian Boos
Priority: normal Milestone: 0.11.6
Component: ticket system Version: 0.13dev
Severity: normal Keywords: permission description needinfo
Cc: ryan.j.ollos@… Branch:
Release Notes:
API Changes:

Description

Someone (public) may create a new ticket and need to edit the description later due to typos, etc. The public can select the priority, etc. when the create the ticket.

Once the ticket is entered, the developer can change the priority or milestone to something the developer chooses for scheduling purposes.

Currently it appears that you must grant a user modification rights (TICKET_MODIFY) in order for that person to be able do modify the description field (TICKET_EDIT_DESCRIPTION). I don't see why these are tied together as dependencies, they should be completely separate permissions.

What happens:

TICKET_MODIFY does not grant permission to 'description' field. TICKET_EDIT_DESCRIPTION allows permission to 'description' field only if has TICKET_MODIFY enabled.

What was expected:

TICKET_EDIT_DESCRIPTION allows permission to 'description' field independent of setting of TICKET_MODIFY.

Attachments (3)

t8548-TICKET_EDIT_DESCRIPTION-r8424.diff (7.8 KB ) - added by Christian Boos 10 years ago.
make TICKET_EDIT_PERMISSION independent from TICKET_MODIFY
t8548-TICKET_EDIT_DESCRIPTION-r8424.2.diff (8.6 KB ) - added by Christian Boos 10 years ago.
updated patch, which additionally disables the Action field when it's not possilbe to change them
t8548-TICKET_EDIT_DESCRIPTION-r8424.3.diff (9.3 KB ) - added by Christian Boos 10 years ago.
another variation on the base patch, don't show the Action fieldset when the user has only TICKET_EDIT_DESCRIPTION

Download all attachments as: .zip

Change History (19)

by Christian Boos, 10 years ago

make TICKET_EDIT_PERMISSION independent from TICKET_MODIFY

comment:1 by Christian Boos, 10 years ago

Yes, this has been requested before.

Please try the attachment:t8548-TICKET_EDIT_DESCRIPTION-r8424.diff.

One thing which can eventually be improved is to hide the Action part of the form but keep the default action value (if omitting it entirely, submitting a description change will fail with "invalid action 'view'"). Attempt to change the status will of course still fail if TICKET_CHGPROP is missing, so it's not a big deal and it's maybe even informative to see the "leave" action?

by Christian Boos, 10 years ago

updated patch, which additionally disables the Action field when it's not possilbe to change them

comment:2 by Christian Boos, 10 years ago

Keywords: permission description review added
Owner: set to Christian Boos
Status: newassigned

I would be glad to get some testing feedback on this one…

comment:3 by Remy Blank, 10 years ago

Works well here. The only comment I have is that I would not show the "leave" radio button as disabled, as it makes the text badly readable. Rather mark it readonly, or even don't bother, as it is the only option shown. Then you also won't need to have the hidden field for the selected radio button anymore.

in reply to:  3 ; comment:4 by Christian Boos, 10 years ago

Replying to rblank:

Works well here. The only comment I have is that I would not show the "leave" radio button as disabled, as it makes the text badly readable.

Yeah, agree, this doesn't look as good as it could.

Rather mark it readonly,

Same rendering.

or even don't bother, as it is the only option shown. Then you also won't need to have the hidden field for the selected radio button anymore.

It's not always the only option shown - at least in my testing. Maybe don't show the Action fieldset at all? See next patch.

by Christian Boos, 10 years ago

another variation on the base patch, don't show the Action fieldset when the user has only TICKET_EDIT_DESCRIPTION

in reply to:  4 comment:5 by Remy Blank, 10 years ago

Replying to cboos:

Rather mark it readonly,

Same rendering.

Not quite. If I only replace disabled with readonly, the rendering stays the same. However, if I also remove the hidden field (and you have to, as a readonly field will be included in the form data AFAIK), then only the button looks disabled, but the label is normal. This is on FF3.

What other option than "leave" could be shown if the user has no permission to "exercise" the workflow?

comment:6 by Remy Blank, 10 years ago

The patch t8548-TICKET_EDIT_DESCRIPTION-r8424.3.diff doesn't work properly if the user has e.g. TICKET_APPEND, as you've still got the readonly= and the hidden field using can_modify_workflow.

But yeah, not showing the "Action" fieldset would be an option, as it simplifies the layout, which could be useful for non-technical people.

comment:7 by Christian Boos, 10 years ago

Ok, you're right with the readonly, the rendering is fine. But it seems that this attribute doesn't prevent the user to change the selection (Chrome and Firefox), despite the XHTML being:

<fieldset id="action">
              <legend>Action</legend>
              <div>
                  <input type="radio" id="action_leave" name="action" value="leave" readonly="readonly" />
                  <label for="action_leave">leave</label>

                  as new
                  <span class="hint"></span>
              </div><div>
                  <input type="radio" id="action_force_status" name="action" value="force_status" checked="checked" readonly="readonly" />
                  <label for="action_force_status">force_status</label>
                  <span class="hint">Next status will be 'started'</span>
              </div>
            </fieldset>

Btw, as you can see, the extra action I have is because I still had the TICKET_STATUSFIX permission from the StatusFixer sample plugin… not a common situation.

So I think I'll go with the (fixed) patch 3.

comment:8 by Christian Boos, 10 years ago

Resolution: fixed
Status: assignedclosed

Done in r8455.

in reply to:  8 ; comment:9 by Carsten Klein <carsten.klein@…>, 9 years ago

Milestone: 0.11.6
Resolution: fixed
Status: closedreopened
Version: 0.11.50.13dev

Replying to cboos:

Done in r8455.

In 0.13dev, when allowing the anonymous user group to TICKET_APPEND, they get the fieldset "action" under ticket modify, although it presents them with only one option → leave as new.

So, in line

380	            <fieldset py:when="can_append or can_modify" id="action"> 

of the presented patch, it should rather read

380	            <fieldset py:when="can_modify" id="action"> 

in order to remove the 'action' fieldset completely from the rendered view.

Rationale here: I do not expect someone with only the ability to add new comments to a ticket or add new attachments to also be able to view/modify the existing workflow state under "Modify Ticket".

Removing milestone and changing version to 0.13dev.

in reply to:  9 ; comment:10 by Christian Boos, 9 years ago

Replying to Carsten Klein <carsten.klein@…>:

I do not expect someone with only the ability to add new comments to a ticket or add new attachments to also be able to view/modify the existing workflow state under "Modify Ticket".

I don't remember all the subtleties of the TracWorkflow, but I suppose you could perfectly define some transitions to require TICKET_APPEND? (e.g. need feedback → feedback given).

comment:11 by Christian Boos, 9 years ago

Keywords: needinfo added; review removed

in reply to:  10 ; comment:12 by Carsten Klein <carsten.klein@…>, 9 years ago

Replying to cboos:

Replying to Carsten Klein <carsten.klein@…>:

I do not expect someone with only the ability to add new comments to a ticket or add new attachments to also be able to view/modify the existing workflow state under "Modify Ticket".

I don't remember all the subtleties of the TracWorkflow, but I suppose you could perfectly define some transitions to require TICKET_APPEND? (e.g. need feedback → feedback given).

I don't, either. Perhaps close this as fixed and we will reopen when the issue turns up…

in reply to:  12 comment:13 by Carsten Klein <carsten.klein@…>, 9 years ago

Resolution: fixed
Status: reopenedclosed

Replying to Carsten Klein <carsten.klein@…>:

I don't, either. Perhaps close this as fixed and we will reopen when the issue turns up…

Besides that, I eventually happened to post this on behalf of a totally different issue.

And, as for the original author's request, this is definitely fixed, so I will close this.

comment:14 by Remy Blank, 9 years ago

Milestone: 0.11.6

Setting milestone back to original value.

in reply to:  1 comment:15 by Ryan J Ollos <ryan.j.ollos@…>, 7 years ago

Replying to cboos:

One thing which can eventually be improved is to hide the Action part of the form but keep the default action value …

Maybe we could just hide the Action part of the form with JavaScript if leave is the only allowed action. This would also address the concern in comment:9. I don't find the form particularly bothersome, but it serves no purpose to the user when the only action is leave, so I think it should be hidden.

comment:16 by Ryan J Ollos <ryan.j.ollos@…>, 7 years ago

Cc: ryan.j.ollos@… added

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Christian Boos.
The resolution will be deleted. Next status will be 'reopened'.
to as closed The owner will be changed from Christian Boos 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.