#871 Remove reply button for non-authn users
Merged 2 years ago by pingou. Opened 2 years ago by pfrields.
pfrields/pagure issue839  into  master

@@ -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

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

for the record: this was previously PR #870

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

Same question for repo_admin here as below

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?

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.

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

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.

Pull-Request has been rebased

2 years ago

Pull-Request has been updated

2 years ago

Pull-Request has been updated

2 years ago

I also removed other unnecessary "if repo_admin" statements in this PR in a separate commit.

We should rework the length of some of these lines but that's for another time.

Thanks for fixing this @pfrields! Merging :)

Pull-Request has been merged by pingou

2 years ago
Metadata