#179 update ticket title, improve review votes display and other changes
Closed 2 years ago by kparal. Opened 2 years ago by kparal.

file modified
+1 -1
@@ -58,7 +58,7 @@ 

      PAGURE_BOT_USERNAME = 'blockerbot'

      PAGURE_BOT_ENABLED = True

      PAGURE_BOT_LOOP_THRESHOLD = 2

-     PAGURE_DISCUSSION_TITLE = "rhbz#$bugid $summary"

+     PAGURE_DISCUSSION_TITLE = "[$component] $summary | rhbz#$bugid"

      PAGURE_DISCUSSION_CONTENT = '''\

  Bug details: ** $bug_url **

  Information from [BlockerBugs App]({}):

@@ -58,6 +58,6 @@ 

      if app.config['FAS_ADMIN_GROUP'] in g.fas_user.groups:

          return None

  

-     app.logger.info('Failed admin access to %s by %s' % (

-         request.url, g.fas_user.username))

+     app.logger.info('Failed admin access to {url} by {user} (not in group "{admin}")'.format(

+         url=request.url, user=g.fas_user.username, admin=app.config['FAS_ADMIN_GROUP']))

      abort(401)

@@ -51,12 +51,13 @@ 

  

  

  {% block body %}

- 

+ {% set buglist_names = ['Proposed Blockers','Accepted Blockers', 'Accepted 0-day Blockers', 'Accepted Previous Release Blockers',

+                         'Proposed Freeze Exceptions', 'Accepted Freeze Exceptions', 'Prioritized Bugs'] %}

  

  <div class="row">

      <div class="col-md-12" id="blockertables">

-         {% for buglist in ['Proposed Blockers','Accepted Blockers', 'Accepted 0-day Blockers', 'Accepted Previous Release Blockers', 'Proposed Freeze Exceptions', 'Accepted Freeze Exceptions', 'Prioritized Bugs'] %}

-         {% if buglists[buglist] %}<h2>{{ buglists[buglist] | length }} {% if (buglists[buglist] | length) == 1 %}{{ buglist[:-1]}}{% else %}{{ buglist }}{% endif %}</h2>

+         {% for buglist in buglist_names if buglists[buglist] %}  {# for non-empty buglists #}

+         <h2>{{ buglists[buglist] | length }} {% if (buglists[buglist] | length) == 1 %}{{ buglist[:-1]}}{% else %}{{ buglist }}{% endif %}</h2>

          <table id="{{ buglist | tagify }}" cellspacing="1" class="table table-striped">

              <thead class="thead-light">

                  <tr>
@@ -72,32 +73,38 @@ 

              <tbody>

                  {% for bug in buglists[buglist] %}

                      <tr>

-                         <td>{% if bug.bugid in recent %}<span class="foundicon-idea yellow" title="modified recently"></span>{% endif %} {% if bug.needinfo %}<span class="needinfo" title="NEEDINFO{% if bug.needinfo_requestee %} from {{ bug.needinfo_requestee }}{% else %}{% endif %}">?</span>{% endif %}{% if bug.bugid in wb_change %}<span class="foundicon-refresh green" title="blocker/fe status changed recently"></span>{% endif %}</td>

+                         <td class="text-nowrap">

+                             {% if bug.bugid in recent %}<span class="foundicon-idea yellow" title="modified recently"></span>{% endif -%}

+                             {% if bug.needinfo %}<span class="needinfo" title="NEEDINFO{% if bug.needinfo_requestee %} from {{ bug.needinfo_requestee }}{% else %}{% endif %}">?</span>{% endif -%}

+                             {% if bug.bugid in wb_change %}<span class="foundicon-refresh green" title="blocker/fe status changed recently"></span>{% endif %}

+                         </td>

                          <td class='bugid'><a href='{{ bug.url }}' target="_blank" rel="noopener noreferrer">{{ bug.bugid }}</a></td>

                          <td>{{ bug.component }}</td>

                          <td>{{ bug.status }}</td>

                          <td>{{ bug.summary }}</td>

                          <td class="text-center">

-                             {% if vote_info[bug.bugid][buglist] %}

-                             <span class="{{'text-success' if  vote_info[bug.bugid][buglist][0] > 0 else ''}}">+{{ vote_info[bug.bugid][buglist][0] }}</span>,

-                             <span>{{ vote_info[bug.bugid][buglist][1] }}</span>,

-                             <span class="{{'text-danger' if  vote_info[bug.bugid][buglist][2] > 0 else ''}}">-{{ vote_info[bug.bugid][buglist][2] }}</span>

-                             <br />

+                             {% if buglist.startswith('Proposed') and vote_info[bug.bugid][buglist] %}

+                                 <span class="text-nowrap">

+                                     <span class="{{'text-success' if  vote_info[bug.bugid][buglist][0] > 0 else ''}}">+{{ vote_info[bug.bugid][buglist][0] }}</span>,

+                                     <span>{{ vote_info[bug.bugid][buglist][1] }}</span>,

+                                     <span class="{{'text-danger' if  vote_info[bug.bugid][buglist][2] > 0 else ''}}">-{{ vote_info[bug.bugid][buglist][2] }}</span>

+                                 </span>

+                                 <br />

                              {% endif %}

                              {% if bug.discussion_link %}

-                             <a href='{{ bug.discussion_link }}' target="_blank" rel="noopener noreferrer">

-                                 {% if buglist.startswith('Accepted') %}

-                                     Discuss

-                                 {% elif vote_info[bug.bugid]['user_voted'] %}

-                                     <span class="text-success">Voted</span>

-                                 {% else %}

-                                     Vote!

-                                 {% endif %}

-                             </a>

+                                 <a href='{{ bug.discussion_link }}' target="_blank" rel="noopener noreferrer">

+                                     {% if buglist.startswith('Accepted') %}

+                                         Discuss

+                                     {% elif vote_info[bug.bugid]['user_voted'] %}

+                                         <span class="text-success">Voted</span>

+                                     {% else %}

+                                         Vote!

+                                     {% endif %}

+                                 </a>

                              {% elif buglist.startswith('Prioritized') %}

-                             <!-- Nothing here, go on...-->

+                                 {# Nothing here, go on... #}

                              {% else %}

-                             TBD

+                                 TBD

                              {% endif %}

                          </td>

                          <td class="popupification">
@@ -107,7 +114,7 @@ 

                                  {{ bug | updatelabel | safe}}</a>

                          </td>

                      </tr>

-                     {% else %}

+                 {% else %}

                      <tr>

                          <td></td>

                          <td></td>
@@ -117,13 +124,13 @@ 

                          <td></td>

                          <td></td>

                      </tr>

-                     {% endfor %}

+                 {% endfor %}

              </tbody>

-         </table>{% endif %}

+         </table>

          {% endfor %}

-         {% for buglist in ['Proposed Blockers','Accepted Blockers', 'Accepted 0-day Blockers', 'Accepted Previous Release Blockers', 'Proposed Freeze Exceptions', 'Accepted Freeze Exceptions', 'Prioritized Bugs'] %}

-             {% if not buglists[buglist] %}<h4>No {{ buglist }}</h4>{% endif %}

+         {% for buglist in buglist_names if not buglists[buglist] %}  {# for empty buglists #}

+             <h4>No {{ buglist }}</h4>

          {% endfor %}

-     </div> <!-- end of 12 columns -->

- </div> <!-- end of row -->

+     </div>  {# end of 12 columns #}

+ </div>  {# end of row #}

  {% endblock %}

@@ -21,7 +21,9 @@ 

  #link {{ bug.discussion_link }}

  {% endif -%}

  #info {{ buglist | replace("Blockers", "Blocker") }}, {{ bug.component }}, {{ bug.status }}

+ {% if buglist.startswith('Proposed') -%}

  {{ vote_info[bug.bugid] }}

+ {% endif -%}

  

  

  {% endfor -%} {# end of bug iteration #}

file modified
+3 -3
@@ -46,9 +46,9 @@ 

  

  

  class FakeFAS(object):

-     fake_user_groups = ['qa-admin']

-     fake_user = munch.Munch(username='developer',

-                             groups=fake_user_groups)

+     fake_user = munch.Munch(

+         username=app.config['FAS_USER'] or 'developer',

+         groups=[app.config['FAS_ADMIN_GROUP']])

  

      def __init__(self, app):

          app.before_request(self._set_fas_user)

@@ -10,16 +10,8 @@ 

  

  

  def create_bug_discussion(bug):

-     title = Template(app.config['PAGURE_DISCUSSION_TITLE']).safe_substitute(

-         bugid=bug.bugid,

-         summary=bug.summary)

-     content = Template(app.config['PAGURE_DISCUSSION_CONTENT']).safe_substitute(

-         bugid=bug.bugid,

-         bug_img=app.config['BLOCKERBUGS_API'] + "bugimg/%s" % bug.bugid,

-         bug_url=bug.url,

-         vote_summary='Nobody voted yet.',

-         pagure_url=app.config['PAGURE_URL'],

-         pagure_repo=app.config['PAGURE_REPO'],)

+     title = render_discussion_template(app.config['PAGURE_DISCUSSION_TITLE'], bug)

+     content = render_discussion_template(app.config['PAGURE_DISCUSSION_CONTENT'], bug)

  

      return create_issue(title, content)

  
@@ -33,17 +25,9 @@ 

      update_issue_url = app.config['PAGURE_API'] + app.config['PAGURE_REPO'] + '/issue/%s' % issue_id

      bug = issue_to_bug(issue_id)

  

-     bug_url = app.config['BUGZILLA_URL'] + "show_bug.cgi?id=%s" % bug.bugid

-     title = Template(app.config['PAGURE_DISCUSSION_TITLE']).safe_substitute(

-         bugid=bug.bugid,

-         summary=bug.summary)

-     content = Template(app.config['PAGURE_DISCUSSION_CONTENT']).safe_substitute(

-         bugid=bug.bugid,

-         bug_img=app.config['BLOCKERBUGS_API'] + "bugimg/%s" % bug.bugid,

-         bug_url=bug_url,

-         vote_summary=vote_summary,

-         pagure_url=app.config['PAGURE_URL'],

-         pagure_repo=app.config['PAGURE_REPO'],)

+     title = render_discussion_template(app.config['PAGURE_DISCUSSION_TITLE'], bug)

+     content = render_discussion_template(app.config['PAGURE_DISCUSSION_CONTENT'],

+                                          bug, vote_summary)

      data = {

          'title': title,

          'issue_content': content,
@@ -159,3 +143,17 @@ 

              'response text:\n%s' % (resp.status_code, resp.text))

  

      return resp.json()

+ 

+ 

+ def render_discussion_template(template, bug, vote_summary='Nobody voted yet.'):

+     rendered = Template(template).substitute(

+         bug_img=f"{app.config['BLOCKERBUGS_API']}bugimg/{bug.bugid}",

+         bug_url=bug.url,

+         bugid=bug.bugid,

+         component=bug.component,

+         pagure_url=app.config['PAGURE_URL'],

+         pagure_repo=app.config['PAGURE_REPO'],

+         summary=bug.summary,

+         vote_summary=vote_summary,

+     )

+     return rendered

file modified
+1
@@ -5,3 +5,4 @@ 

  testpaths = testing

  filterwarnings =

      ignore:the imp module is deprecated in favour of importlib:DeprecationWarning:koji

+     ignore:defusedxml.cElementTree is deprecated, import from defusedxml.ElementTree instead:DeprecationWarning:openid

This contains multiple different changes, please see the diff per commit, thank you. Commits included:

commit b3151b8fd466f97211bc47632a07b5cd7700cfe3 (HEAD -> changes, origin/changes)
Author: Kamil Páral <kparal@redhat.com>
Date:   Tue May 4 10:33:52 2021 +0200

    blocker_list.html: make it more readable, don't wrap status icons

    Make the Jinja template a bit more readable by aligning sections, defining a
    variable, wrapping long lines, dropping unnecessary `if` statements.

    Also make status icons non-wrappable, which looked bad.


commit 12c5cb7b5a9c314f85b83277f0f677d97639d7a5
Author: Kamil Páral <kparal@redhat.com>
Date:   Mon May 3 16:53:39 2021 +0200

    pytest: ignore a DeprecationWarning from the openid package


commit b799b8a4b68b841ae300e488f77c8d0a2ac02fa2
Author: Kamil Páral <kparal@redhat.com>
Date:   Mon May 3 16:35:25 2021 +0200

    show review vote counts only for Proposed bugs

    For Proposed blockers, it makes sense to display vote counts (e.g. +3,0,-1). But
    for Accepted blockers, it doesn't really make sense, because it doesn't include
    the votes from the meeting (where it could've been accepted), and therefore it
    often seems confusing (e.g. an accepted blocker which is +0,0,-0 or even
    +0,0,-1).

    Fixes: https://pagure.io/fedora-qa/blockerbugs/issue/177


commit 26e77ebbea11fa8c3bc63d553eed990d44c19de4
Author: Kamil Páral <kparal@redhat.com>
Date:   Mon May 3 16:09:32 2021 +0200

    discussions: add component to the ticket title, reshuffle

    This makes ticket titles more readable, which helps in the Pagure's Issues view
    and also for email notifications.

    Fixes: https://pagure.io/fedora-qa/blockerbugs/issue/178


commit c886b7e5951b94e322e89ce6c25e37e2a52e5a56
Author: Kamil Páral <kparal@redhat.com>
Date:   Mon May 3 14:22:11 2021 +0200

    blocker_list: prevent wrapping votes in the middle


commit b5c85668a012eb2ccd9e49d65ed042a58fae70bf
Author: Kamil Páral <kparal@redhat.com>
Date:   Mon May 3 14:07:03 2021 +0200

    allow the dev to log in as any user, always in admin group

    Allowing the developer to log in as any user is important to work on web UI
    which shows the "Review" column differently, if you're logged in as a user who
    already voted.

    Always placing the user into the admin group is a quality of life improvement.
    The default config is created with FAS_ADMIN_GROUP='', which prevents any admin
    access until you correctly populate it (with "qa-admin"). That serves little
    purpose. Instead, assign the admin group to the FakeFAS user automatically,
    whichever its value is (even an empty string).

Build succeeded.

1 new commit added

  • pytest: ignore a DeprecationWarning from the openid package
2 years ago

Build succeeded.

@lbrabec Please review, thank you.

Metadata Update from @kparal:
- Request assigned

2 years ago

1 new commit added

  • blocker_list.html: make it more readable, don't wrap status icons
2 years ago

Build succeeded.

fake_user_groups was a list, but app.config['FAS_ADMIN_GROUP'] is a string. Is this okay?

fake_user_groups was a list, but app.config['FAS_ADMIN_GROUP'] is a string. Is this okay?

That's probably not ok, thanks for spotting that! :-) I'll fix it, test it, and then commit the whole PR, thanks for the review.

rebased onto 2686047

2 years ago

Build succeeded.

Pull-Request has been closed by kparal

2 years ago