From 2204c0410a449a4dc1b021354f9e88c7ed33fb12 Mon Sep 17 00:00:00 2001
From: Xavier Morel <xmo@odoo.com>
Date: Tue, 7 Jun 2022 15:49:52 +0200
Subject: [PATCH] [IMP] mergebot, forwardbot: various UI bits

- code in the various menus added over time through the UI (queues,
  configuration, ...)
- update / improve PR layout a tick
- fix "outstanding forward ports" count on the dashboard
- improve hover title / help on dashboard
  - add date of last modification (usually date of success / failure)
  - make casing more coherent (everything lowercase)
  - add explicit note that UTC date on staged at label is staged at datetime
  - rediscover yet again that the staging information is when hovering
    on the staging *except the staged at label*
- improve `PullRequest.unstage` to always insert the PR at the start of the
  reason when cancelling the staging, for clarity / traceability

Closes #560, closes #609
---
 forwardport/__manifest__.py          |  1 +
 forwardport/data/queues.xml          | 51 +++++++++++++++
 forwardport/data/views.xml           |  4 ++
 runbot_merge/__manifest__.py         |  2 +
 runbot_merge/changelog/2022-06/ui.md |  6 ++
 runbot_merge/controllers/__init__.py |  6 +-
 runbot_merge/models/pull_requests.py | 10 +--
 runbot_merge/views/configuration.xml | 43 ++++++++++++
 runbot_merge/views/mergebot.xml      | 79 +++++++---------------
 runbot_merge/views/queues.xml        | 97 ++++++++++++++++++++++++++++
 runbot_merge/views/templates.xml     | 14 ++--
 11 files changed, 241 insertions(+), 72 deletions(-)
 create mode 100644 forwardport/data/queues.xml
 create mode 100644 runbot_merge/changelog/2022-06/ui.md
 create mode 100644 runbot_merge/views/configuration.xml
 create mode 100644 runbot_merge/views/queues.xml

diff --git a/forwardport/__manifest__.py b/forwardport/__manifest__.py
index 185f8256..b86c425e 100644
--- a/forwardport/__manifest__.py
+++ b/forwardport/__manifest__.py
@@ -8,6 +8,7 @@
         'data/security.xml',
         'data/crons.xml',
         'data/views.xml',
+        'data/queues.xml',
     ],
     'license': 'LGPL-3',
 }
diff --git a/forwardport/data/queues.xml b/forwardport/data/queues.xml
new file mode 100644
index 00000000..ba011344
--- /dev/null
+++ b/forwardport/data/queues.xml
@@ -0,0 +1,51 @@
+<odoo>
+    <record id="action_forward_port" model="ir.actions.act_window">
+        <field name="name">Forward port batches</field>
+        <field name="res_model">forwardport.batches</field>
+        <field name="context">{'active_test': False}</field>
+    </record>
+    <record id="tree_forward_port" model="ir.ui.view">
+        <field name="name">Forward port batches</field>
+        <field name="model">forwardport.batches</field>
+        <field name="arch" type="xml">
+            <tree>
+                <field name="source"/>
+                <field name="batch_id"/>
+            </tree>
+        </field>
+    </record>
+    <record id="form_forward_port" model="ir.ui.view">
+        <field name="name">Forward port batch</field>
+        <field name="model">forwardport.batches</field>
+        <field name="arch" type="xml">
+            <form>
+                <group>
+                    <group><field name="source"/></group>
+                    <group><field name="batch_id"/></group>
+                </group>
+            </form>
+        </field>
+    </record>
+
+    <record id="action_followup_updates" model="ir.actions.act_window">
+        <field name="name">Followup Updates</field>
+        <field name="res_model">forwardport.updates</field>
+    </record>
+    <record id="tree_followup_updates" model="ir.ui.view">
+        <field name="name">Followup Updates</field>
+        <field name="model">forwardport.updates</field>
+        <field name="arch" type="xml">
+            <tree editable="bottom">
+                <field name="original_root"/>
+                <field name="new_root"/>
+            </tree>
+        </field>
+    </record>
+
+    <menuitem name="Forward Port Batches" id="menu_forward_port"
+              parent="runbot_merge.menu_queues"
+              action="action_forward_port"/>
+    <menuitem name="Followup Updates" id="menu_followup"
+              parent="runbot_merge.menu_queues"
+              action="action_followup_updates"/>
+</odoo>
diff --git a/forwardport/data/views.xml b/forwardport/data/views.xml
index ff7f66c9..b7000892 100644
--- a/forwardport/data/views.xml
+++ b/forwardport/data/views.xml
@@ -12,6 +12,7 @@
                 <t t-set="outstanding" t-value="env['runbot_merge.pull_requests'].search_count([
                     ('source_id', '!=', False),
                     ('state', 'not in', ['merged', 'closed']),
+                    ('source_id.merge_date', '&lt;', datetime.datetime.now() - relativedelta(days=3)),
                 ])"/>
                 <div t-if="outstanding != 0" class="alert col-md-12 alert-warning mb-0">
                     <a href="/forwardport/outstanding">
@@ -175,6 +176,9 @@
         <field name="inherit_id" ref="runbot_merge.runbot_merge_form_prs"/>
         <field name="model">runbot_merge.pull_requests</field>
         <field name="arch" type="xml">
+            <xpath expr="//field[@name='state']" position="after">
+                <field name="merge_date" attrs="{'invisible': [('state', '!=', 'merged')]}"/>
+            </xpath>
             <xpath expr="//sheet/group[2]" position="after">
                 <separator string="Forward Port" attrs="{'invisible': [('source_id', '=', False)]}"/>
                 <group attrs="{'invisible': [('source_id', '!=', False)]}">
diff --git a/runbot_merge/__manifest__.py b/runbot_merge/__manifest__.py
index 8375a3d8..9f44e106 100644
--- a/runbot_merge/__manifest__.py
+++ b/runbot_merge/__manifest__.py
@@ -10,6 +10,8 @@
         'views/res_partner.xml',
         'views/runbot_merge_project.xml',
         'views/mergebot.xml',
+        'views/queues.xml',
+        'views/configuration.xml',
         'views/templates.xml',
         'models/project_freeze/views.xml',
     ],
diff --git a/runbot_merge/changelog/2022-06/ui.md b/runbot_merge/changelog/2022-06/ui.md
new file mode 100644
index 00000000..33a1667c
--- /dev/null
+++ b/runbot_merge/changelog/2022-06/ui.md
@@ -0,0 +1,6 @@
+IMP: various UI items
+
+- more clearly differentiate between "pending" and "unknown" statuses on stagings
+- fix "outstanding forward ports" count
+- add date of staging last modification (= success / failure instant)
+- correctly retrieve and include fast-forward and unstaging reasons
diff --git a/runbot_merge/controllers/__init__.py b/runbot_merge/controllers/__init__.py
index 199b4801..d96d703d 100644
--- a/runbot_merge/controllers/__init__.py
+++ b/runbot_merge/controllers/__init__.py
@@ -171,11 +171,7 @@ def handle_pr(env, event):
             return "It's my understanding that closed/merged PRs don't get sync'd"
 
         if pr_obj.state == 'ready':
-            pr_obj.unstage(
-                "PR %s updated by %s",
-                pr_obj.display_name,
-                event['sender']['login']
-            )
+            pr_obj.unstage("updated by %s", event['sender']['login'])
 
         _logger.info(
             "PR %s updated to %s by %s, resetting to 'open' and squash=%s",
diff --git a/runbot_merge/models/pull_requests.py b/runbot_merge/models/pull_requests.py
index 63a7e3ef..76407ebf 100644
--- a/runbot_merge/models/pull_requests.py
+++ b/runbot_merge/models/pull_requests.py
@@ -846,7 +846,7 @@ class PullRequests(models.Model):
                                 'pull_request': self.number,
                                 'message': "PR priority reset to 1, as pull requests with priority 0 ignore review state.",
                             })
-                        self.unstage("unreview (r-) by %s", author.github_login)
+                        self.unstage("unreviewed (r-) by %s", author.github_login)
                         ok = True
                     else:
                         msg = "r- makes no sense in the current PR state."
@@ -1349,7 +1349,7 @@ class PullRequests(models.Model):
                 # else remove this batch from the split
                 b.split_id = False
 
-        self.staging_id.cancel(reason, *args)
+        self.staging_id.cancel('%s ' + reason, self.display_name, *args)
 
     def _try_closing(self, by):
         # ignore if the PR is already being updated in a separate transaction
@@ -1369,11 +1369,7 @@ class PullRequests(models.Model):
         ''', [self.id])
         self.env.cr.commit()
         self.modified(['state'])
-        self.unstage(
-            "PR %s closed by %s",
-            self.display_name,
-            by
-        )
+        self.unstage("closed by %s", by)
         return True
 
 # state changes on reviews
diff --git a/runbot_merge/views/configuration.xml b/runbot_merge/views/configuration.xml
new file mode 100644
index 00000000..70e8d710
--- /dev/null
+++ b/runbot_merge/views/configuration.xml
@@ -0,0 +1,43 @@
+<odoo>
+    <record id="action_overrides" model="ir.actions.act_window">
+        <field name="name">CI / statuses overrides</field>
+        <field name="res_model">res.partner.override</field>
+    </record>
+    <record id="tree_overrides" model="ir.ui.view">
+        <field name="name">Overrides List</field>
+        <field name="model">res.partner.override</field>
+        <field name="arch" type="xml">
+            <tree editable="bottom">
+                <field name="context"/>
+                <field name="repository_id"/>
+                <field name="partner_ids" widget="many2many_tags"/>
+            </tree>
+        </field>
+    </record>
+
+    <record id="action_review" model="ir.actions.act_window">
+        <field name="name">Review Rights</field>
+        <field name="res_model">res.partner.review</field>
+        <field name="context">{'search_default_group_by_repository': True}</field>
+    </record>
+    <record id="tree_review" model="ir.ui.view">
+        <field name="name">Review Rights</field>
+        <field name="model">res.partner.review</field>
+        <field name="arch" type="xml">
+            <tree editable="bottom">
+                <field name="repository_id"/>
+                <field name="partner_id"/>
+                <field name="review"/>
+                <field name="self_review"/>
+            </tree>
+        </field>
+    </record>
+
+    <menuitem name="Configuration" id="menu_configuration" parent="runbot_merge_menu"/>
+        <menuitem name="CI Overrides" id="menu_configuration_overrides"
+                  parent="menu_configuration"
+                  action="action_overrides"/>
+        <menuitem name="Review Rights" id="menu_configuration_review"
+                  parent="menu_configuration"
+                  action="action_review"/>
+</odoo>
diff --git a/runbot_merge/views/mergebot.xml b/runbot_merge/views/mergebot.xml
index f8922130..34472a5a 100644
--- a/runbot_merge/views/mergebot.xml
+++ b/runbot_merge/views/mergebot.xml
@@ -97,10 +97,10 @@
                             <field name="target"/>
                             <field name="state"/>
                             <field name="author"/>
-                            <field name="priority"/>
                         </group>
                         <group>
                             <field name="label"/>
+                            <field name="priority"/>
                             <field name="squash"/>
                         </group>
                     </group>
@@ -108,6 +108,8 @@
                         <group colspan="4">
                             <field name="head"/>
                             <field name="statuses"/>
+                        </group>
+                        <group colspan="4">
                             <field name="overrides"/>
                         </group>
                     </group>
@@ -205,66 +207,33 @@
         </field>
     </record>
 
-    <record id="runbot_merge_action_fetches" model="ir.actions.act_window">
-        <field name="name">PRs to fetch</field>
-        <field name="res_model">runbot_merge.fetch_job</field>
-        <field name="view_mode">tree</field>
-        <field name="context">{'default_active': True}</field>
+    <record id="runbot_merge_action_commits" model="ir.actions.act_window">
+        <field name="name">Commit Statuses</field>
+        <field name="res_model">runbot_merge.commit</field>
+        <field name="view_mode">tree,form</field>
     </record>
-    <record id="runbot_merge_search_fetches" model="ir.ui.view">
-        <field name="name">Fetches Search</field>
-        <field name="model">runbot_merge.fetch_job</field>
-        <field name="arch" type="xml">
-            <search>
-                <filter string="Active" name="active"
-                        domain="[('active', '=', True)]"/>
-                <field name="repository"/>
-                <field name="number"/>
-            </search>
-        </field>
-    </record>
-    <record id="runbot_merge_tree_fetches" model="ir.ui.view">
-        <field name="name">Fetches Tree</field>
-        <field name="model">runbot_merge.fetch_job</field>
+    <record id="runbot_merge_commits_tree" model="ir.ui.view">
+        <field name="name">commits list</field>
+        <field name="model">runbot_merge.commit</field>
         <field name="arch" type="xml">
             <tree>
-                <field name="repository"/>
-                <field name="number"/>
-            </tree>
-        </field>
-    </record>
-
-    <record id="runbot_merge_action_overrides" model="ir.actions.act_window">
-        <field name="name">CI / statuses overrides</field>
-        <field name="res_model">res.partner.override</field>
-    </record>
-    <record id="runot_merge_tree_overrides" model="ir.ui.view">
-        <field name="name">Overrides List</field>
-        <field name="model">res.partner.override</field>
-        <field name="arch" type="xml">
-            <tree editable="bottom">
-                <field name="context"/>
-                <field name="repository_id"/>
-                <field name="partner_ids" widget="many2many_tags"/>
+                <field name="sha"/>
+                <field name="statuses"/>
             </tree>
         </field>
     </record>
 
     <menuitem name="Mergebot" id="runbot_merge_menu"/>
-    <menuitem name="Projects" id="runbot_merge_menu_project"
-              parent="runbot_merge_menu"
-              action="runbot_merge_action_projects"/>
-    <menuitem name="Pull Requests" id="runbot_merge_menu_prs"
-              parent="runbot_merge_menu"
-              action="runbot_merge_action_prs"/>
-    <menuitem name="Stagings" id="runbot_merge_menu_stagings"
-              parent="runbot_merge_menu"
-              action="runbot_merge_action_stagings"/>
-    <menuitem name="Fetches" id="runbot_merge_menu_fetches"
-              parent="runbot_merge_menu"
-              action="runbot_merge_action_fetches"/>
-    <menuitem name="Configuration" id="runbot_merge_menu_configuration" parent="runbot_merge_menu"/>
-    <menuitem name="CI Overrides" id="runbot_merge_menu_configuration_overrides"
-              parent="runbot_merge_menu"
-              action="runbot_merge_action_overrides"/>
+        <menuitem name="Projects" id="runbot_merge_menu_project"
+                  parent="runbot_merge_menu"
+                  action="runbot_merge_action_projects"/>
+        <menuitem name="Pull Requests" id="runbot_merge_menu_prs"
+                  parent="runbot_merge_menu"
+                  action="runbot_merge_action_prs"/>
+        <menuitem name="Stagings" id="runbot_merge_menu_stagings"
+                  parent="runbot_merge_menu"
+                  action="runbot_merge_action_stagings"/>
+        <menuitem name="Commits" id="runbot_merge_menu_commits"
+                  parent="runbot_merge_menu"
+                  action="runbot_merge_action_commits"/>
 </odoo>
diff --git a/runbot_merge/views/queues.xml b/runbot_merge/views/queues.xml
new file mode 100644
index 00000000..a72571f3
--- /dev/null
+++ b/runbot_merge/views/queues.xml
@@ -0,0 +1,97 @@
+<odoo>
+    <!--
+    Queues mergebot menu: contains various list views inspecting the cron tasks
+    (mostly)
+    -->
+    <record id="action_splits" model="ir.actions.act_window">
+        <field name="name">Splits</field>
+        <field name="res_model">runbot_merge.split</field>
+    </record>
+    <record id="tree_splits" model="ir.ui.view">
+        <field name="name">Splits</field>
+        <field name="model">runbot_merge.split</field>
+        <field name="arch" type="xml">
+            <tree>
+                <field name="id"/>
+                <field name="target"/>
+            </tree>
+        </field>
+    </record>
+
+    <record id="action_feedback" model="ir.actions.act_window">
+        <field name="name">Feedback</field>
+        <field name="res_model">runbot_merge.pull_requests.feedback</field>
+    </record>
+    <record id="tree_feedback" model="ir.ui.view">
+        <field name="name">Feedback</field>
+        <field name="model">runbot_merge.pull_requests.feedback</field>
+        <field name="arch" type="xml">
+            <tree>
+                <field name="repository"/>
+                <field name="pull_request"/>
+                <field name="message"/>
+                <field name="close"/>
+            </tree>
+        </field>
+    </record>
+
+    <record id="action_tagging" model="ir.actions.act_window">
+        <field name="name">Tagging</field>
+        <field name="res_model">runbot_merge.pull_requests.tagging</field>
+    </record>
+    <record id="tree_tagging" model="ir.ui.view">
+        <field name="name">Tagging</field>
+        <field name="model">runbot_merge.pull_requests.tagging</field>
+        <field name="arch" type="xml">
+            <tree editable="bottom">
+                <field name="repository"/>
+                <field name="pull_request"/>
+                <field name="tags_add"/>
+                <field name="tags_remove"/>
+            </tree>
+        </field>
+    </record>
+
+    <record id="action_fetches" model="ir.actions.act_window">
+        <field name="name">PRs to fetch</field>
+        <field name="res_model">runbot_merge.fetch_job</field>
+        <field name="view_mode">tree</field>
+        <field name="context">{'default_active': True}</field>
+    </record>
+    <record id="search_fetches" model="ir.ui.view">
+        <field name="name">Fetches Search</field>
+        <field name="model">runbot_merge.fetch_job</field>
+        <field name="arch" type="xml">
+            <search>
+                <filter string="Active" name="active"
+                        domain="[('active', '=', True)]"/>
+                <field name="repository"/>
+                <field name="number"/>
+            </search>
+        </field>
+    </record>
+    <record id="tree_fetches" model="ir.ui.view">
+        <field name="name">Fetches Tree</field>
+        <field name="model">runbot_merge.fetch_job</field>
+        <field name="arch" type="xml">
+            <tree>
+                <field name="repository"/>
+                <field name="number"/>
+            </tree>
+        </field>
+    </record>
+
+    <menuitem name="Queues" id="menu_queues" parent="runbot_merge_menu"/>
+        <menuitem name="Splits" id="menu_queues_splits"
+                  parent="menu_queues"
+                  action="action_splits"/>
+        <menuitem name="Feedback" id="menu_queues_feedback"
+                  parent="menu_queues"
+                  action="action_feedback"/>
+        <menuitem name="Tagging" id="menu_queues_tagging"
+                  parent="menu_queues"
+                  action="action_tagging"/>
+        <menuitem name="Fetches" id="menu_fetches"
+                  parent="menu_queues"
+                  action="action_fetches"/>
+</odoo>
diff --git a/runbot_merge/views/templates.xml b/runbot_merge/views/templates.xml
index 49e475b3..fab3b1e6 100644
--- a/runbot_merge/views/templates.xml
+++ b/runbot_merge/views/templates.xml
@@ -135,11 +135,15 @@
                     <t t-if="staging_index >= 4">visible-lg-block</t>
                 </t>
                 <t t-set="title">
-                    <t t-if="staging.state == 'canceled'">Cancelled: <t t-esc="staging.reason"/></t>
-                    <t t-if="staging.state == 'ff_failed'">Fast Forward Failed</t>
-                    <t t-if="staging.state not in ('canceled', 'ff_failed')"><t t-esc="staging.reason"/></t>
+                    <t t-if="staging.state == 'ff_failed'">fast forward failed</t>
+                    <t t-if="staging.state == 'pending'">last status</t>
+                    <t t-if="staging.state not in ('pending', 'ff_failed')"><t t-esc="staging.reason"/></t>
                 </t>
-                <li t-attf-class="staging {{stateclass.strip()}} {{decorationclass.strip()}}" t-att-title="title.strip() or None">
+                <!-- separate concatenation to avoid having line-break in title as some browsers trigger it -->
+                <!-- write-date may have microsecond precision, strip that information -->
+                <!-- use write-date under assumption that a staging is last modified when it ends -->
+                <t t-set="title"><t t-esc="title.strip()"/> at <t t-esc="staging.write_date.replace(microsecond=0)"/>Z</t>
+                <li t-attf-class="staging {{stateclass.strip()}} {{decorationclass.strip()}}" t-att-title="title">
                     <ul class="list-unstyled">
                         <li t-foreach="staging.batch_ids" t-as="batch" class="batch">
                             <t t-esc="batch.prs[:1].label"/>
@@ -152,7 +156,7 @@
                     <t t-if="staging.heads">
                         <div class="dropdown">
                             <button class="btn btn-link dropdown-toggle" type="button" data-toggle="dropdown" aria-haspopup="true" aria-expanded="true"
-                                    t-attf-title="{{staging.staged_at}}Z">
+                                    t-attf-title="Staged at {{staging.staged_at}}Z">
                                 Staged <span t-field="staging.staged_at" t-options="{'widget': 'relative'}"/>
                                 <span class="caret"></span>
                             </button>