Edgewall Software
Modify

Ticket #8548 (closed defect: fixed)

Opened 3 years ago

Last modified 18 months ago

TICKET_EDIT_DESCRIPTION requires TICKET_MODIFY rights

Reported by: Michael <michael.lecomte@…> Owned by: cboos
Priority: normal Milestone: 0.11.6
Component: ticket system Version: 0.13dev
Severity: normal Keywords: permission description needinfo
Cc:
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

t8548-TICKET_EDIT_DESCRIPTION-r8424.diff (7.8 KB) - added by cboos 3 years ago.
make TICKET_EDIT_PERMISSION independent from TICKET_MODIFY
t8548-TICKET_EDIT_DESCRIPTION-r8424.2.diff (8.6 KB) - added by cboos 3 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 cboos 3 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

Changed 3 years ago by cboos

make TICKET_EDIT_PERMISSION independent from TICKET_MODIFY

comment:1 Changed 3 years ago by cboos

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?

Changed 3 years ago by cboos

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

comment:2 Changed 3 years ago by cboos

  • Keywords permission description review added
  • Owner set to cboos
  • Status changed from new to assigned

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

comment:3 follow-up: Changed 3 years ago by 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. 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.

comment:4 in reply to: ↑ 3 ; follow-up: Changed 3 years ago by cboos

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.

Changed 3 years ago by cboos

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

comment:5 in reply to: ↑ 4 Changed 3 years ago by rblank

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 Changed 3 years ago by rblank

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 Changed 3 years ago by cboos

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 follow-up: Changed 3 years ago by cboos

  • Resolution set to fixed
  • Status changed from assigned to closed

Done in r8455.

comment:9 in reply to: ↑ 8 ; follow-up: Changed 21 months ago by Carsten Klein <carsten.klein@…>

  • Milestone 0.11.6 deleted
  • Resolution fixed deleted
  • Status changed from closed to reopened
  • Version changed from 0.11.5 to 0.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.

comment:10 in reply to: ↑ 9 ; follow-up: Changed 21 months ago by 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).

comment:11 Changed 20 months ago by cboos

  • Keywords needinfo added; review removed

comment:12 in reply to: ↑ 10 ; follow-up: Changed 18 months ago by Carsten Klein <carsten.klein@…>

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…

comment:13 in reply to: ↑ 12 Changed 18 months ago by Carsten Klein <carsten.klein@…>

  • Resolution set to fixed
  • Status changed from reopened to closed

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 Changed 18 months ago by rblank

  • Milestone set to 0.11.6

Setting milestone back to original value.

View

Add a comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
The resolution will be deleted. Next status will be 'reopened'
to The owner will be changed from cboos. Next status will be 'closed'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.