From 81ce4ea02b76af4dcc859739985a41ff8c073b6e Mon Sep 17 00:00:00 2001
From: Xavier Morel <xmo@odoo.com>
Date: Wed, 5 Jul 2023 15:11:40 +0200
Subject: [PATCH] [IMP] rewrite /forwardport/outstanding

- add support for authorship (not just approval)
- make display counts directly
- fix `state` filter: postgres can't do negative index lookups
- add indexes for author and reviewed_by as we look them up
- ensure we handle the entire source filtering via a single subquery

Closes #778
---
 forwardport/controllers.py                 |  79 +++++++++++
 forwardport/data/views.xml                 | 152 +++++++++++++--------
 forwardport/models/project.py              |  25 ----
 runbot_merge/models/pull_requests.py       |   4 +-
 runbot_merge/static/scss/runbot_merge.scss |  11 ++
 5 files changed, 185 insertions(+), 86 deletions(-)

diff --git a/forwardport/controllers.py b/forwardport/controllers.py
index af464dcb..620eab96 100644
--- a/forwardport/controllers.py
+++ b/forwardport/controllers.py
@@ -1,7 +1,14 @@
+import collections
+import datetime
 import pathlib
 
+import werkzeug.urls
+
+from odoo.http import route, request
+from odoo.osv import expression
 from odoo.addons.runbot_merge.controllers.dashboard import MergebotDashboard
 
+DEFAULT_DELTA = datetime.timedelta(days=7)
 class Dashboard(MergebotDashboard):
     def _entries(self):
         changelog = pathlib.Path(__file__).parent / 'changelog'
@@ -13,3 +20,75 @@ class Dashboard(MergebotDashboard):
             for d in changelog.iterdir()
         ]
 
+
+    @route('/forwardport/outstanding', type='http', methods=['GET'], auth="user", website=True, sitemap=False)
+    def outstanding(self, partner=0, authors=True, reviewers=True, group=0):
+        Partners = request.env['res.partner']
+        PullRequests = request.env['runbot_merge.pull_requests']
+        partner = Partners.browse(int(partner))
+        group = Partners.browse(int(group))
+        authors = int(authors)
+        reviewers = int(reviewers)
+        link = lambda **kw: '?' + werkzeug.urls.url_encode({'partner': partner.id or 0, 'authors': authors, 'reviewers': reviewers, **kw, })
+        groups = Partners.search([('is_company', '=', True), ('child_ids', '!=', False)])
+        if not (authors or reviewers):
+            return request.render('forwardport.outstanding', {
+                'authors': 0,
+                'reviewers': 0,
+                'single': partner,
+                'culprits': partner,
+                'groups': groups,
+                'current_group': group,
+                'outstanding': [],
+                'outstanding_per_author': {partner: 0},
+                'outstanding_per_reviewer': {partner: 0},
+                'link': link,
+            })
+
+        source_filter = [('merge_date', '<', datetime.datetime.now() - DEFAULT_DELTA)]
+        partner_filter = []
+        if partner or group:
+            if partner:
+                suffix = ''
+                arg = partner.id
+            else:
+                suffix = '.commercial_partner_id'
+                arg = group.id
+
+            if authors:
+                partner_filter.append([(f'author{suffix}', '=', arg)])
+            if reviewers:
+                partner_filter.append([(f'reviewed_by{suffix}', '=', arg)])
+
+            source_filter.extend(expression.OR(partner_filter))
+
+        outstanding = PullRequests.search([
+            ('state', 'in', ['opened', 'validated', 'approved', 'ready', 'error']),
+            ('source_id', 'in', PullRequests._search(source_filter)),
+        ])
+        outstanding_per_author = collections.Counter()
+        outstanding_per_reviewer = collections.Counter()
+        outstandings = []
+        for source in outstanding.mapped('source_id').sorted('merge_date'):
+            outstandings.append({
+                'source': source,
+                'prs': source.forwardport_ids.filtered(lambda p: p.state not in ['merged', 'closed']),
+            })
+            if authors:
+                outstanding_per_author[source.author] += 1
+            if reviewers and source:
+                outstanding_per_reviewer[source.reviewed_by] += 1
+
+        culprits = Partners.browse(p.id for p, _ in (outstanding_per_reviewer + outstanding_per_author).most_common())
+        return request.render('forwardport.outstanding', {
+            'authors': authors,
+            'reviewers': reviewers,
+            'single': partner,
+            'culprits': culprits,
+            'groups': groups,
+            'current_group': group,
+            'outstanding_per_author': outstanding_per_author,
+            'outstanding_per_reviewer': outstanding_per_reviewer,
+            'outstanding': outstandings,
+            'link': link,
+        })
diff --git a/forwardport/data/views.xml b/forwardport/data/views.xml
index c25bb311..1b6069c2 100644
--- a/forwardport/data/views.xml
+++ b/forwardport/data/views.xml
@@ -30,67 +30,101 @@
         <t t-else="">bg-warning</t>
     </template>
 
-    <record id="forwardport.outstanding_fp" model="website.page">
-        <field name="name">Outstanding forward ports</field>
-        <field name="type">qweb</field>
-        <field name="url">/forwardport/outstanding</field>
-        <field name="website_indexed" eval="False"/>
-        <field name="is_published">True</field>
-        <field name="key">forwardport.outstanding_fp</field>
-        <field name="arch" type="xml">
-            <t name="Outstanding forward ports" t-name="forwardport.outstanding_fp">
-                <t t-call="website.layout">
-                    <t t-set="hof" t-value="env['runbot_merge.pull_requests']._hall_of_shame()"/>
-                    <div id="wrap" class="oe_structure oe_empty"><div class="container-fluid">
-                        <ul class="alert bg-light list-inline">
-                            <span t-foreach="hof.reviewers" t-as="count" class="list-inline-item">
-                                <a t-attf-href="?reviewer={{count[0].id}}"
-                                   t-field="count[0].display_name"
-                                   t-att-title="count[1]"
-                                />
-                            </span>
-                        </ul>
-                        <h1>List of pull requests with outstanding forward ports</h1>
-                        <t t-set="reviewer" t-value="env['res.partner'].browse(int(request.params.get('reviewer') or 0))"/>
-                        <form method="get" action="" id="reset-filter"/>
-                        <h2 t-if="reviewer" class="text-muted">
-                            merged by <span t-field="reviewer.display_name" t-attf-title="@{{reviewer.github_login}}"/>
-                            <button form="reset-filter" type="submit"
-                                    name="reviewer" value=""
-                                    title="See All" class="btn fa fa-times"/>
-                        </h2>
-                        <dl><t t-foreach="hof.outstanding" t-as="x">
-                            <t t-set="source" t-value="x[0]"/>
-                            <t t-if="not reviewer or source.reviewed_by == reviewer">
-                                <dt>
-                                    <a t-att-href="source.url"><span t-field="source.display_name"/></a>
-                                    by <span t-field="source.author.display_name"
-                                             t-attf-title="@{{source.author.github_login}}"/>
-                                    merged <span t-field="source.merge_date"
-                                                 t-options="{'widget': 'relative'}"
-                                                 t-att-title="source.merge_date"/>
-                                    <t t-if="not reviewer">
-                                        by <span t-field="source.reviewed_by.display_name"
-                                                 t-attf-title="@{{source.reviewed_by.github_login}}"/>
-                                    </t>
-                                </dt>
-                                <dd>
-                                    Outstanding forward-ports:
-                                    <ul>
-                                        <li t-foreach="x.prs" t-as="p">
-                                            <a t-att-href="p.url"><span t-field="p.display_name"/></a>
-                                            (<span t-field="p.state"/>)
-                                            targeting <span t-field="p.target.name"/>
-                                        </li>
-                                    </ul>
-                                </dd>
+    <template id="outstanding" name="Outstanding forward ports">
+        <t t-call="website.layout">
+            <div id="wrap" class="oe_structure oe_empty"><div class="container-fluid">
+                <div class="alert bg-light outstanding-partners">
+                    <t t-foreach="groups" t-as="group">
+                        <span>
+                            <t t-if="group == current_group">
+                                <span class="bg-primary" t-out="group.display_name"/>
+                                <a t-att-href="link()" class="btn fa fa-times p-0"/>
                             </t>
-                        </t></dl>
-                    </div></div>
+                            <t t-else="">
+                                <a t-att-href="link(group=group.id, partner=0)" t-out="group.display_name"/>
+                            </t>
+                        </span>
+                    </t>
+                </div>
+                <div class="alert bg-light outstanding-partners">
+                    <t t-foreach="culprits" t-as="culprit">
+                        <t t-set="approved" t-value="outstanding_per_reviewer[culprit]"/>
+                        <t t-set="created" t-value="outstanding_per_author[culprit]"/>
+                        <a t-att-href="link(partner=culprit.id)"
+                           t-attf-title="approved {{approved}}, created {{created}}"
+                        ><t t-out="culprit.display_name"/>:
+                            <t t-if="approved" t-out="approved"/>
+                            <t t-if="approved and created"> + </t>
+                            <t t-if="created" t-out="created"/>
+                        </a>
+                    </t>
+                </div>
+                <t t-if="not single">
+                    by
+                    <span class="btn-group btn-group-toggle">
+                        <a t-att-href="link(authors=1, reviewers=1)"
+                           t-attf-class="btn btn-sm btn-secondary {{'active' if authors and reviewers else ''}}">
+                            both
+                        </a>
+                        <a t-att-href="link(authors=1, reviewers=0)"
+                           t-attf-class="btn btn-sm btn-secondary {{'active' if authors and not reviewers else ''}}">
+                            creators
+                        </a>
+                        <a t-att-href="link(reviewers=1, authors=0)"
+                           t-attf-class="btn btn-sm btn-secondary {{'active' if reviewers and not authors else ''}}">
+                            reviewers
+                        </a>
+                    </span>
                 </t>
-            </t>
-        </field>
-    </record>
+                <h1>List of pull requests with outstanding forward ports</h1>
+                <h2 t-if="single">
+                    for <span t-field="single.display_name" t-attf-title="@{{single.github_login}}"/>
+                    <a t-att-href="link(partner=0)" title="All Users" class="btn fa fa-times"/>
+
+                    <span class="btn-group btn-group-toggle">
+                        <a t-att-href="link(authors=1, reviewers=1)"
+                           t-attf-class="btn btn-sm btn-secondary {{'active' if authors and reviewers else ''}}">
+                            both
+                        </a>
+                        <a t-att-href="link(authors=1, reviewers=0)"
+                           t-attf-class="btn btn-sm btn-secondary {{'active' if authors and not reviewers else ''}}">
+                            created
+                        </a>
+                        <a t-att-href="link(reviewers=1, authors=0)"
+                           t-attf-class="btn btn-sm btn-secondary {{'active' if reviewers and not authors else ''}}">
+                            reviewed
+                        </a>
+                    </span>
+                </h2>
+
+                <dl><t t-foreach="outstanding" t-as="x">
+                    <t t-set="source" t-value="x['source']"/>
+                    <t t-if="not single or source.reviewed_by == single or source.author == single">
+                        <dt>
+                            <a t-att-href="source.url"><span t-field="source.display_name"/></a>
+                            created by <span t-field="source.author.display_name"
+                                             t-attf-title="@{{source.author.github_login}}"/>
+                            merged <span t-field="source.merge_date"
+                                         t-options="{'widget': 'relative'}"
+                                         t-att-title="source.merge_date"/>
+                            by <span t-field="source.reviewed_by.display_name"
+                                     t-attf-title="@{{source.reviewed_by.github_login}}"/>
+                        </dt>
+                        <dd>
+                            Outstanding forward-ports:
+                            <ul>
+                                <li t-foreach="x['prs']" t-as="p">
+                                    <a t-att-href="p.url"><span t-field="p.display_name"/></a>
+                                    (<span t-field="p.state"/>)
+                                    targeting <span t-field="p.target.name"/>
+                                </li>
+                            </ul>
+                        </dd>
+                    </t>
+                </t></dl>
+            </div></div>
+        </t>
+    </template>
 
     <template id="view_pull_request" inherit_id="runbot_merge.view_pull_request">
         <xpath expr="//dl[hasclass('runbot-merge-fields')]" position="inside">
diff --git a/forwardport/models/project.py b/forwardport/models/project.py
index 720000e7..51974020 100644
--- a/forwardport/models/project.py
+++ b/forwardport/models/project.py
@@ -1114,31 +1114,6 @@ stderr:
             ('source_id.merge_date', '<', cutoff),
         ], order='source_id, id'), lambda p: p.source_id)
 
-    def _hall_of_shame(self):
-        """Provides data for the HOS view
-
-        * outstanding forward ports per reviewer
-        * pull requests with outstanding forward ports, oldest-merged first
-        """
-        cutoff_dt = datetime.datetime.now() - DEFAULT_DELTA
-        outstanding = self.env['runbot_merge.pull_requests'].search([
-            ('source_id', '!=', False),
-            ('state', 'not in', ['merged', 'closed']),
-            ('source_id.merge_date', '<', cutoff_dt),
-        ], order=None)
-        # only keep merged because apparently some PRs are in a weird spot
-        # where they're sources but closed?
-        sources = outstanding.mapped('source_id').filtered('merge_date').sorted('merge_date')
-        outstandings = []
-        reviewers = collections.Counter()
-        for source in sources:
-            outstandings.append(Outstanding(source=source, prs=source.forwardport_ids & outstanding))
-            reviewers[source.reviewed_by] += 1
-        return HallOfShame(
-            reviewers=reviewers.most_common(),
-            outstanding=outstandings,
-        )
-
     def _reminder(self):
         cutoff = self.env.context.get('forwardport_updated_before') \
               or fields.Datetime.to_string(datetime.datetime.now() - DEFAULT_DELTA)
diff --git a/runbot_merge/models/pull_requests.py b/runbot_merge/models/pull_requests.py
index 19c63aef..d48ad505 100644
--- a/runbot_merge/models/pull_requests.py
+++ b/runbot_merge/models/pull_requests.py
@@ -527,7 +527,7 @@ class PullRequests(models.Model):
     ], default='opened', index=True)
 
     number = fields.Integer(required=True, index=True, group_operator=None)
-    author = fields.Many2one('res.partner')
+    author = fields.Many2one('res.partner', index=True)
     head = fields.Char(required=True)
     label = fields.Char(
         required=True, index=True,
@@ -545,7 +545,7 @@ class PullRequests(models.Model):
     ], default=False)
     method_warned = fields.Boolean(default=False)
 
-    reviewed_by = fields.Many2one('res.partner')
+    reviewed_by = fields.Many2one('res.partner', index=True)
     delegates = fields.Many2many('res.partner', help="Delegate reviewers, not intrinsically reviewers but can review this PR")
     priority = fields.Integer(default=2, index=True, group_operator=None)
 
diff --git a/runbot_merge/static/scss/runbot_merge.scss b/runbot_merge/static/scss/runbot_merge.scss
index 2b98cf32..2e6577b3 100644
--- a/runbot_merge/static/scss/runbot_merge.scss
+++ b/runbot_merge/static/scss/runbot_merge.scss
@@ -110,3 +110,14 @@ dl.runbot-merge-fields {
 .staging-statuses {
   cursor: wait;
 }
+
+/* forwardport */
+.outstanding-partners > * {
+  @extend .pt-1;
+  // because there's a trailing space which is annoying to remove, which plays
+  // the role of padding-right
+  @extend .pl-1;
+  @extend .text-nowrap;
+  // works better for the left edge of the *box*
+  @extend .border-left;
+}