[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
This commit is contained in:
Xavier Morel 2023-07-05 15:11:40 +02:00
parent aefbdaf974
commit 81ce4ea02b
5 changed files with 185 additions and 86 deletions

View File

@ -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,
})

View File

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

View File

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

View File

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

View File

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