PR#871: Remove reply button for non-authn users - pagure

pagure

#871 Remove reply button for non-authn users

Merged 7 years ago by pingou. Opened 7 years ago by pfrields.

pfrields/pagure issue839 into master

Remove other unnecessary repo_admin

Paul W. Frields • 7 years ago

741e7eb

repo_admin not needed due to g.fas_user

Paul W. Frields • 7 years ago

f3a30c5

Remove reply button for non-authn users

Paul W. Frields • 7 years ago

c30730f

pagure/templates/_formhelper.html

file modified

+13 -11

		`@@ -138,12 +138,13 @@`
		`{% endif %}`
		`<div class="issue_action icon pull-xs-right p-b-1">`
		`<div class="btn-group" role="group" aria-label="Basic example">`
		`- <a class="reply btn btn-secondary btn-sm" data-toggle="tooltip" title="Reply to this comment - loose formating">`
		`- <span class="oi" data-glyph="share-boxed"></span>`
		`- </a>`
		`+ {% if id != 0 and g.fas_user and comment.parent.status in [True, 'Open'] %}`
		`+ <a class="reply btn btn-secondary btn-sm" data-toggle="tooltip" title="Reply to this comment - loose formating">`
		`+ <span class="oi" data-glyph="share-boxed"></span>`
		`+ </a>`
		`+ {% endif %}`
		`{% if id != 0 and g.fas_user and (`
		`- (comment.parent.status in [True, 'Open'] and g.fas_user.username == comment.user.username)`
		`- or repo_admin) %}`
		`+ comment.parent.status in [True, 'Open'] and g.fas_user.username == comment.user.username) %}`
		`<a class="btn btn-secondary btn-sm edit_btn" href="{{`
		`'%s/comment/%s/edit' % (request.base_url, comment.id) }}"`
		`data-comment="{{ comment.id }}" data-objid="{{ issueid }}">`
		`@@ -151,8 +152,7 @@`
		`</a>`
		`{% endif %}`
		`{% if id != 0 and g.fas_user and (`
		`- (comment.parent.status in [True, 'Open'] and g.fas_user.username == comment.user.username)`
		`- or repo_admin) %}`
		`+ comment.parent.status in [True, 'Open'] and g.fas_user.username == comment.user.username) %}`
		`<button class="btn btn-secondary btn-sm" type="submit" name="drop_comment" value="{{ comment.id }}"`
		`onclick="return confirm('Do you really want to remove this comment?');"`
		`title="Remove comment">`
		`@@ -176,10 +176,12 @@`
		`</section>`
		`<div class="issue_action icon pull-xs-right">`
		`<div class="btn-group" role="group">`
		`- <a class="btn btn-secondary btn-sm reply"`
		`- title="Reply to the initial comment - loose formatting">`
		`- <span class="oi" data-glyph="share-boxed"></span>`
		`- </a>`
		`+ {% if g.fas_user %}`
		`+ <a class="btn btn-secondary btn-sm reply"`
		`+ title="Reply to the initial comment - loose formatting">`
		`+ <span class="oi" data-glyph="share-boxed"></span>`
		`+ </a>`
		`+ {% endif %}`
		`{% if repo_admin or (`
		`g.fas_user and g.fas_user.username == comment.user.username) %}`
		`<a class="btn btn-primary btn-sm" href="{{`

no initial comment

pfrields commented 7 years ago

Fixes https://pagure.io/pagure/issue/839 -- now with indentation (and rebased)

puiterwijk commented 7 years ago

:thumbsup:
Please note you can update the PR in place by force-pushing to the PR branch, and the web interface will automatically update.

puiterwijk commented 7 years ago

for the record: this was previously PR #870

pingou commented on line 24 of pagure/templates/_formhelper.html 7 years ago

One sudden thought, do we need repo_admin here? Since it can be true only if g.fas_user is not None, no?

pingou commented on line 8 of pagure/templates/_formhelper.html 7 years ago

Same question for repo_admin here as below

pfrields commented 7 years ago

Good questions on repo_admin, pingou... I didn't know whether pagure allowed some out-of-band users that would make g.fas_user == None. It sounds like that's not the case?

pingou commented 7 years ago

We always check for g.fas_user != None to check if an user is authenticated or not, and there isn't really any special-case users that would be using the UI.

cverna commented 7 years ago

Would g.fas_user be used (ie. different of None) by instance of pagure not using FAS for the user authentication ???

pingou commented 7 years ago

Would g.fas_user be used (ie. different of None) by instance of pagure not using FAS for the user authentication ???

Yes, however unfortunate the variable name is in this case, if you use openid or the local auth in pagure, in both cases the user's info are stored in g.fas_user.