the comment box should be hidden if check_can_change_field() reports the user is unable to comment on the bug

RESOLVED FIXED in Bugzilla 4.4

Status

()

Bugzilla
Creating/Changing Bugs
--
minor
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: glob, Assigned: Simon Green)

Tracking

Bugzilla 4.4
Bug Flags:
approval +
approval4.4 +

Details

Attachments

(1 attachment, 1 obsolete attachment)

3.17 KB, patch
glob
: review+
Details | Diff | Splinter Review
(Reporter)

Description

6 years ago
if an extension uses the bug_check_can_change_field hook to indicate that a user is unable to comment on a bug, the comment ui is displayed, but submission results in:

> You tried to change the Comment field, but only a user with the required permissions may change that field.

we should hide the comment field if the user is unable to comment on a bug, and replace it with a message indicating that they are unable to comment.
(Assignee)

Comment 1

6 years ago
test
(Assignee)

Comment 2

6 years ago
Created attachment 703273 [details] [diff] [review]
v1 patch

Sorry about the last comment. Did test in the wrong window.

Regards,
Hugo
Attachment #703273 - Flags: review?
(Reporter)

Updated

6 years ago
QA Contact: default-qa → hugo.seabrook
(Assignee)

Comment 3

6 years ago
QA Contact: default-qa@bugzilla.bugshugo.seabrook@gmail.com

Hi Bryon,

Why was I made the QA contact of this bug? Can you please let me know what is involved in this?

Regards,
Hugo
(Reporter)

Comment 4

6 years ago
(In reply to Hugo Seabrook from comment #3)
> QA Contact: default-qa@bugzilla.bugshugo.seabrook@gmail.com
> 
> Hi Bryon,

byron :)

> Why was I made the QA contact of this bug? Can you please let me know what
> is involved in this?

oops, meant to make you the assignee.  fixed.
Assignee: general → hugo.seabrook
QA Contact: hugo.seabrook

Updated

6 years ago
QA Contact: default-qa
(Reporter)

Comment 5

6 years ago
Comment on attachment 703273 [details] [diff] [review]
v1 patch

r=glob

>+        [% ELSE %]
>+          You are unable to make an additional comment on this bug.
>+        [% END %]

this should be wrapped inside a <p> block to make it more readable.
can be fixed on commit.
Attachment #703273 - Flags: review? → review+
(Reporter)

Updated

6 years ago
Flags: approval?
(Reporter)

Comment 6

6 years ago
actually, the following looks better, and is consistent with the "you must be logged in" message.

[% ELSE %]
  <fieldset>
    <legend>Note</legend>
    You are unable to make an additional comment on this [% terms.bug %].
  </fieldset>
[% END %]

i can make this change on commit.
(Reporter)

Updated

6 years ago
Flags: approval?
(Reporter)

Comment 7

6 years ago
Comment on attachment 703273 [details] [diff] [review]
v1 patch

dkl pointed out that the [reply] link in existing comments also needs to be hidden if users are not able to add new comments.
Attachment #703273 - Flags: review+ → review-
(Assignee)

Comment 8

6 years ago
Created attachment 708520 [details] [diff] [review]
v2 patch

We also need to hide the private checkbox too
Attachment #703273 - Attachment is obsolete: true
Attachment #708520 - Flags: review?(glob)
(Reporter)

Comment 9

6 years ago
Comment on attachment 708520 [details] [diff] [review]
v2 patch

r=glob
Attachment #708520 - Flags: review?(glob) → review+
(Reporter)

Updated

6 years ago
Flags: approval?

Comment 10

6 years ago
Comment on attachment 708520 [details] [diff] [review]
v2 patch

>=== modified file 'template/en/default/bug/edit.html.tmpl'

>+        [% ELSE %]
>+          You are unable to make an additional comment on this bug.
>+        [% END %]

Hugo, make sure to run runtests.pl before uploading a patch. You must replace the hardcoded 'bug' by [% terms.bug %]. Also, I wonder if 'unable' shouldn't be replaced by 'not allowed'.

Please fix that on checkin.

Comment 11

6 years ago
@glob: I will let you do the fixes on checkin.
Severity: normal → minor
Status: NEW → ASSIGNED
Component: Bugzilla-General → Creating/Changing Bugs
Flags: approval?
Flags: approval4.4+
Flags: approval+
OS: Mac OS X → All
Hardware: x86 → All
Target Milestone: --- → Bugzilla 4.4
(Reporter)

Comment 12

6 years ago
Committing to: bzr+ssh://bjones%40mozilla.com@bzr.mozilla.org/bugzilla/trunk/
modified template/en/default/bug/comments.html.tmpl
modified template/en/default/bug/edit.html.tmpl
Committed revision 8574.

Committing to: bzr+ssh://bjones%40mozilla.com@bzr.mozilla.org/bugzilla/4.4/
modified template/en/default/bug/comments.html.tmpl
modified template/en/default/bug/edit.html.tmpl
Committed revision 8515.
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED

Comment 13

6 years ago
Added to relnotes for 4.4rc2.
Summary: the comment box should be hidden if bug_check_can_change_field reports the user is unable to comment on the bug → the comment box should be hidden if check_can_change_field() reports the user is unable to comment on the bug
You need to log in before you can comment on or make changes to this bug.