Commit Graph

329 Commits

Author SHA1 Message Date
Xavier Morel
57162547e0 [FIX] runbot_merge: Odoo 15.0 + Py3.10 compat
Turns out I was running "15.0" except just on the runbot, enterprise
and community were still the 14.0 repos, so some of the changes were
missing.

While at it, bundle fixes for 3.10, as that's what Jammy needs, and
the mergebot/15.0 will be running on that.
2022-11-17 10:30:04 +01:00
Xavier Morel
389ea03544 [MERGE] from 14.0
Get mergebot updates from since the runbot was upgraded.

Also convert assets to new system for compatibility.
2022-08-24 08:40:18 +02:00
Xavier Morel
ebbe77b849 [IMP] runbot_merge: reorg test
Test seems to fail from time to time with one of the PRs getting
lost. Tried to move code around trying to investigate, can't repro
anymore. Possibly a race condition because the `to_pr` call was
performed too early, before the webhook had run (and thus before the
PR object had been created on the odoo side).

By moving the `to_pr` calls to after the cron run, we really ensure
the webhooks will have run.

Also update `to_pr` to ensure exactly one PR was retrieved, as
currently nothing is checked so we might have gotten none (yet), which
should be noticed early and clearly. In theory this also guards
against multiple PRs, but PRs should be unique on (repo, number).
2022-08-05 15:35:51 +02:00
Xavier Morel
c85735870a [FIX] runbot_merge: bugs in log of branch stagings (in frontend)
- Some batches in a few stagings are apparently empty (e.g. batch
  71771 from staging 32511), the listing was not resilient to such
  issues.

  Update the code to suppress the display of empty batches.
- The possibility of accessing `/runbot_merge/<id>` with a
  non-existent branch had not been considered and triggered a
  rendering error in the template.

Fixes #630, fixes #631
2022-08-05 15:35:51 +02:00
Xavier Morel
c3b90454a2 [IMP] runbot_merge: a few backend screens
- override the staging's name_get to provide a slightly more useful
  display_name (though still not great as the staging object remains
  quite technical and inimical to human interaction)
- show individual PRs in a batch (as m2m tags) for readability
- update PR views to show the author and reviewer, except in the list
  of delegations of users where it's a lot less useful

/cc #632
2022-08-05 15:35:51 +02:00
Xavier Morel
0c882fc0df [IMP] runbot_merge: automation around branch deactivation
Currently deactivating a branch kinda leaves users in the dark, with
little way to know what has happened aside from inferring it from the
branch having disappeared from the main dashboard.

- surface the state of the branch in the PR dashboard (also surface
  the target branch at all so users can see if their PR is targeted
  as they expect as far as the mergebot is concerned)
- close & notify every PR to a branch being deactivated
- cancel any current staging to the branch (as a consequence of the
  above)

Closes #632
2022-08-05 15:35:51 +02:00
Xavier Morel
21899af758 [FIX] runbot_merge: brainfart in dashboard warning 2022-08-05 15:35:51 +02:00
Xavier Morel
5176ff094c [FIX] runbot_merge: home page styling
Previously if a branch name could not be line-broken (because it was
full of underscores) it would break the layout by making "its"
staging's column much wider than the expected 1/2 / 1/4 /
1/6 (depending on window width), compressing the width of its
sibling's columns.

By disabling content-based width (only taking in account flex-width)
and setting overflow to hidden, the overlong branch names get cut off
instead.
2022-08-05 15:35:51 +02:00
Xavier Morel
60266a1b23 [FIX] *: git invocation error and a few bits
- if stderr has been rerouted or explicitely rerouted to STDOUT,
  `e.stderr` is `None` and the error reporting blows up (which is
  inconvenient). Handle this case.
- handle the case where `fp_github_name` has not been configured (it's
  not a super useful handling but meh, apparently git/hub doesn't
  really care if there's a username when using API tokens)
- minor improvement to the refline parser: per-spec, the trailing
  newline is optional, so don't fail if it's missing
2022-08-05 15:35:51 +02:00
Xavier Morel
3da1874196 [FIX] runbot_merge: correctly handle emptying PR body
The previous version of the code assumed `pr['body']` is always a
string, which is not correct, when the PR body is emptied the body
itself is removed (its value is `None`).

Add a case for this in the PR edition test, and avoid blowing up (or
adding empty newlines) when the PR body is empty. For PR creation this
issue was fixed in c2db5659d8 but
apparently I missed that the exact same issue occurs just a few lines
above.

Also turns out github does *not* send change information when the body
is updated from (or to?) `None`, so don't even bother with that, just
check every time if the overall message has been updated.

Fixes #629
2022-08-05 15:35:51 +02:00
Xavier Morel
b86092de83 [IMP] *: freeze wizard v3, freezer and wizarder
Stop *staging* release PRs: they are normally fairly simple and should
not fail their staging outside of unreliable tests (or possibly a few
edge cases e.g. forgot one version change thing), however staging them
creates the possibility of a "version hole" on the release branch
which is undesirable.

Instead, immediately and unconditionally push the release commits onto
the newly created branches, if there are things which don't work they
can be fixed afterwards (and the process refined, maybe).

Also add the same feature for *bump* PRs, with the difference that the
bump PRs are not created / requested by default (they have to be opted
in individually).

For convenience, add a feature which automatically finds the PRs via
inputting the label (not really tested yet).

Closes #603
2022-08-05 15:35:51 +02:00
Xavier Morel
7eeee99735 [FIX] runbot_merge: avoid race condition in tests
Because the searching of the PR occurs *right* after the PR was
created on the server, despite the additional operations (status,
approval) it's apparently possible for the lookup of the new PR to
occur about the same time the PR is being created, kinda, maybe?

On DS it triggers very reliably for every PR but the first. By moving
the retrieval after the repo timeout & we've run the crons, we near
guarantee the PRs are visible (it's possible for things to fail on
grounds of github reliability, but then they'd have failed *even more*
before).
2022-08-05 15:35:51 +02:00
Xavier Morel
59e730f703 [IMP] runbot_merge: controllers logging
- add a logging entry for PR updates
- change the generic log entry to log the sender (of the event) rather
  than the PR author
- fix the post-facto PR loader to more systematically and reliably set
  a `sender`
2022-08-05 15:35:51 +02:00
Xavier Morel
64eeb6142e [FIX] runbot_merge: also show banners on PR pages
It's likely that the PR pages are seen more commonly than the
dashboard by most users, so add alerts there in case users wonder
what's happening.

Fixes #580
2022-08-05 15:35:51 +02:00
Xavier Morel
2b3ee48ebd [IMP] runbot_merge: staging logging
The log message only indicated whether the PR was squashed or not, but
that's not actually useful.

Improve the message to log the actual merge method, for
information. The old "squash" (aka squash flag set and no merge
method, since an actual squash merge method was reintroduced a while
ago) has been renamed to "single" for the purpose of this display.
2022-06-30 15:07:49 +02:00
Xavier Morel
0016404474 [IMP] runbot_merge: better styling for missing statuses on stagings
Before this, until the first status for a required context the status
would appear as pending, but would be have oddly (e.g. not clickable).

Update the style of such statuses for clarity:

- use a light background to show them as inactive
- use the `wait` cursor to show their status as oddball (and not clickable)

Setting this styling on the link (or even `li`) doesn't seem to work,
so set it on the `ul`, the actual active links will set the relevant
"active" cursor instead, which seems to work fine.

While at it, extract the status menu to its own template and unify the
disparate bits, mainly in that both the main dashboard and the
per-branch list display the staging instant in UTC on hover: before,
the main dashboard would display a relative delta and provide the
UTC-formatted instant on hover, but the branch would only show a zoned
ISO-8601 instant.

While adjusting is easy, it's unnecessary, we can easily provide the UTC
staging instant there).
2022-06-30 15:07:49 +02:00
Xavier Morel
f430c014c1 [IMP] *: review mergebot & forwardbot messages for pinging
Old messages were quite inconsistent in their pinging of the PR author
and reviewer.

Reviewed messages (probably missed some but...) and try to more
consistently ping when the feedback requires some sort of action in
order to proceed.

Fixes #592
2022-06-30 15:07:49 +02:00
Xavier Morel
4a3cde2faa [IMP] runbot_merge: provisioning features
A few fixes and improvements after testing the feature:

- ensure the provisioned users are created as internal (not portal)
- assume oauth is installed and just crash if it's not
- handle a user not having an email (ignore)
- return value from json handler, otherwise JsonRequest sends no
  payload which is *weird*
2022-06-30 15:07:49 +02:00
Xavier Morel
96f8f4e688 [FIX] runbot_merge: unstage on staging-relevant PR modifications
Stagings would be cancelled automatically if the PR's commits were
updated, but not if the target (base) was changed, even though that
has a drastic impact on staging.

Add hooks to unstage PRs if their base is updated, or if their message
is updated and relevant to staging (merge or rebase-merge methods).

Fixes #604
2022-06-30 15:07:49 +02:00
Xavier Morel
a5fae548fe [IMP] runbot_merge: extract dashboard pr linking
Extract the creation of a PR link (to github) as a dedicated
template for easier updates, and to use `display_name`
everywhere (instead of reimplementing it by name).

Also implement support for repo-level groups setting for information
hiding.

Fixes #590
2022-06-30 15:07:49 +02:00
Xavier Morel
66c2bdc25b [IMP] runbot_merge: error reporting on fast-forward failure
When a staging's fast-forward (to the target branch) fails, the
mergebot would provide no useful information on the staging or the
dashboard.

This is because the reason was set to the HTTP status, which in case
of a fast-forward error is just "422 client error: unprocessable
entity".

Improve this by trying to parse github's response in that case, and
using the JSON error message as failure reason. This provides more
useful failure information like "update is not a fast forward",
"reference does not exist", or a branch protection failure.

Closes #591
2022-06-30 15:07:49 +02:00
Xavier Morel
2204c0410a [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
2022-06-30 15:07:49 +02:00
Xavier Morel
56898df93f [ADD] runbot_merge: remote user provisioning
New accounts endpoint such that the SSO can push new pre-configured
users / employees directly. This lowers maintenance burden.

Also remove one of the source partners from the merge test, as
ordering seems wonky for unclear reasons leading to random failures of
that test.
2022-06-07 13:48:17 +02:00
Xavier Morel
1dcab0744d [IMP] runbot_merge: remove dead code
apparently at one point I wanted to provide an atom feed or something
2022-06-02 12:55:22 +02:00
Xavier Morel
48c9754906 [IMP] runbot_merge: acknowledge check command
The command works, but the 'bot would always reply it was not allowed
2022-06-02 12:55:22 +02:00
Xavier-Do
3071f5a6db [IMP] runbot: adapt for odoo 15.0 2022-06-02 10:50:42 +02:00
Xavier Morel
2898c7edd4 [FIX] runbot_merge: updating of commit date on rebase
On #509, the rebasing process was changed to forcefully update the
commit date of the commits, in order to force trigger builds.

However when squashing was re-enabled for #539 for some fool reason it
implemented its own bespoke rebasing (despite that not actually saving
any API call that I can see), meaning it did *not* update the commit
date. As such, an old commit being squashed would not get picked up by
the runbot, which is what happened to odoo/documentation#1226 (which
ultimately had to be hand-rebased after some confusion as to why it
did not work).

Update `_stage_squash` to go through `rebase` the normal way, also
update `rebase` to pop the commit date entirely instead of setting it
manually, and update the squashing test to check that the commit date
gets properly updated.

Fixes #579, closes #582
2022-02-10 13:51:08 +01:00
Xavier Morel
2285965e22 [MRG] runbot_merge, forwardport: multiple fixes to freeze wizard
Multiple fixes to various issues of the freeze wizard.

Some of the less fix-oriented sub-tasks were moved to #586 for discussion instead.

Closes #559, #587
2022-02-10 13:50:21 +01:00
Xavier Morel
de70bd6f83 [IMP] runbot_merge: show PR titles in freeze wizard
Currently limited to release/freeze PRs: it can be difficult to be
sure the right PR was selected then, and a mistake there seems more
impactful than in the PRs being waited for?

Note: adds a test to make sure I don't break the check that all
      release PRs must have the same label (be linked). This was
      already safe, and in a way this PR adds convenience but not
      really safety, but better sure than sorry.
2022-02-08 12:28:10 +01:00
Xavier Morel
e2887a7473 [IMP] runbot_merge: allow only freezing a subset of a project
- add flag to not select repos for freezing
- allow removing more repositories from the wizard
- when performing the freeze, only create branches for the selected
  repos
2022-02-07 15:15:13 +01:00
Xavier Morel
1add3d4854 [FIX] runbot_merge: freeze being triggered upon reopening the wizard
The freeze wizard was implemented using a single action to open and
validate the dialog.

This was a mistake, as it means if there are no errors left (e.g. all
the PRs being waited for are now validated) trying to view the freeze
wizard will immediately validate it and commit the freeze, which is
unexpected, surprising, and unsafe e.g.

- open wizard
- add freeze prs
- add a required pr or two
- close and go do something else
- be told that more PRs need to be waited for
- reopen wizard
- oops freeze is done

So split the "open action" part of `action_freeze` into opening the
action and performing the freeze. The "freeze" / "view freeze" button
on the project only activates the latter, and the actual freeze
operation is only triggered from the wizard's "Freeze" button.

Part of #559.
2022-02-07 13:23:41 +01:00
Xavier Morel
4c8ab63af8 [REM] runbot_merge: leftover debug print
Found by @d-fence
2022-02-07 08:08:59 +01:00
Xavier Morel
4da0f5df69 [ADD] runbot_merge: ~~tree~~ freeze wizard
Provides a less manual interface for creating the freeze:

* takes the name of the branch to create
* takes any number of PRs which must be part of the freeze
* takes PRs representing the HEADs of the new branches

Then essentially takes care of the test.

Implementation of the actual wizard is not trivial but fairly
straightforward and linear, biggest issue is not being able to
`project_id.branch_ids[1]` to get the new branch, not sure why but it
seems to ignore the ordering, clearing the cache doens't fix it.

When creating the branches, add a sleep after each one for secondary
rate limiting purposes. Same when deleting branches.

Also the forwardbot has been updated to disable the forwardport cron
while a freeze is ongoing, this simplifies the freezing process.

Note: after recommendation of @aab-odoo, tried using `_applyChanges`
in `_checkState` but it simply did not work: the two relational fields
got completely frozen and were impossible to update, which is less
than ideal. Oh well, hopefully it works well enough like this for now.
2021-11-17 10:40:12 +01:00
Xavier Morel
dbc001f841 [FIX] runbot_merge: odd behaviour in try_closing
Could not reproduce it in a shell, but in the original version
`self.env.cr.rowcount` would always be 0 after the `modified`.

Turns out the check is really completely dumb, because if we got any
match in `select for update` we're going to find the same on in the
update, and thus the conditional is unnecessary. I've no idea why I
did that.

Anyway remove the conditional and just always try to unstage the PR.
2021-11-16 14:11:28 +01:00
Xavier Morel
5336e53de2 [FIX] runbot_merge: name_search of PRs
Stray prints were forgotten in
4e4e4303f6, and the model should have a
`_rec_name` so the fallback doesn't trigger warnings.
2021-11-12 15:57:16 +01:00
Xavier Morel
f13c60a018 [IMP] runbot_merge: small reorg of main models file
To prep for the addition of the freeze wizard:

* move projects out of `pull_requests.py`
* then realize half the methods there have no relation to projects and
  move them to more relevant places in `pull_requests.py`
* update corresponding crons (and tests using those crons) as the
  methods have changed model, and the cron definitions thus need to be
  updated
* split update to labels out of sending feedback comments while at it:
  labels are not used much during tests so their manipulation can be
  avoided; and labels are not as urgent as feedback so the crons can
  be quite a bit slower
* move the project view out of `mergebot.xml` as well
2021-11-10 13:13:34 +01:00
Xavier Morel
9f7cfcc7ac [FIX] runbot_merge: miswritten view file 2021-11-10 08:11:09 +01:00
Xavier Morel
6d35b4e6f8 [IMP] runbot_merge: allow finding partners by github login
Also remove search on ref which we don't use
2021-10-25 10:14:01 +02:00
Xavier Morel
1cf33f9635 [IMP] runbot_merge: add misconfiguration warning to partner view
Following #531 reviews from reviewers without an email set are
rejected.

For delegates this isn't very helpful, however for specifically
configured reviewers we can warn the configurer that they need to set
an email for things to work out.
2021-10-25 09:10:25 +02:00
Xavier Morel
0e087e7433 [ADD] mergebot, forwardbot: changelog
* Adds a changelog page, linked from the main, with content
  automatically loaded from the source. To avoid conflicts, each entry
  is its own file and entries are grouped by the month during which
  the update will (probably) be deployed
* The last group (most likely "last update") doesn't have a title, the
  rest do.
* Add changelog entries from the last update so it's not too empty.
* Also update the layout for the alerts a bit: remove bottom margin to
  reduce loss of whitespace.
2021-10-20 15:16:48 +02:00
Xavier Morel
bce0836aa9 [FIX] runbot_merge: avoid editing title line of commits
When a commit is lacking the purpose (?) tag e.g. `[FIX]`, `[IMP]`,
..., a normal commit message of the form `<module>: <info>` marches
the looks of a git pseudo-header.

This results in a commit rewrite rejiggering the entire thing and
breaking the message by moving the title to the pseudo-headers and
mis-promoting either the `closes` line of body content to "title",
resulting in a really crappy commit message
e.g. odoo/odoo@d4aa9ad031.

Update the commit rewriting procedure to specifically skip the title
line, and re-inject it without processing in the output.

Fixes #540
2021-10-20 15:16:48 +02:00
Xavier Morel
df8ccf8500 [ADD] runbot_merge: squash mode (partial)
Re-introduce a "squash" mode solely for the purpose of fixing up
commit messages without having to go and edit them: for now "squash"
only works for single-commit PRs, acts as a normal
integration (`rebase-ff`) *but* replaces the message of the commit
itself by that of the PR, similar to the `merge` modes.

This means maintainers can update commit messages to standards by
editing the PR description (though this is obviously sensible to
edition races with the original author).

Fixes #539
2021-10-20 15:16:48 +02:00
Xavier Morel
6c60d59b11 [IMP] forwardport: hall of fame layout and informations
The list of outstanding forwardports was pretty messy as the ordering
was unclear and there was little way to really drill into the thing.

* Shows outstanding forward ports sorted by merged date ascending, the
  oldest-merged PRs are the ones most in need of fixing while PRs
  which were only just merged can safely be ignored.
* List reviewers with outstanding forward-ports, allow filtering by
  clicking on their name, allow deseleting through the subtitle of the
  page.
* Don't display reviewer in list when page is already filtered by
  reviewer.

Also improve PR page a bit:

* Add reviewer.
* Add direct link to backend (closes #524).

Closes #529
2021-10-20 15:16:35 +02:00
Xavier Morel
bd041c9f4a [FIX] runbot_merge: always log staging failures
Because only the first staging failure is considered "hard" and will
put the PR in error, when looking at staging logs it's possible to see
the same PR get staged over and over and over again, which is quite
confusing.

To make the logs less weird, always log a staging failure even when it
doesn't put the PR in error. Sadly this can't be tested as `capsys` is
not able to intercept an stderr inherited by a child process (capfd
doesn't work either).

Fixes #527
2021-10-20 14:36:50 +02:00
Xavier Morel
a7808425e3 [IMP] runbot_merge: reject review without email
If a reviewer doesn't have an email set, the Signed-Off-By is an
`@users.noreply.github.com` address which just looks weird in the
final result.

Initially the thinking was that emails would be required for users to
*be* reviewers or self-reviewers, but since those are now o2ms / m2ms
it's a bit of a pain in the ass.

Instead, provide an action to easily try and fetch the public email of
a user from github.

Fixes #531
2021-10-20 14:36:50 +02:00
Xavier Morel
d32ca9a1b3 [IMP] Model emulator: better support method calls
Since we have the model fields loaded up, we can just check into that
and assume anything that's not a field is a method.

That avoids having to go through `_call`, making things way less awkward.
2021-10-20 14:36:50 +02:00
Xavier Morel
c036c7a28f [CHG] allow delegate reviewers to configure the merge method
After internal discussions it was concluded that this didn't extend
much more trust than allowing authors to accept their single-PR
commits without additional supervisions, and it would avoid some
inconveniences and PR-blocking.

Fixes #69 (nice)
2021-10-20 14:36:50 +02:00
Xavier Morel
bf34e9aa95 [FIX] runbot_merge: ensure PR description is correct on merge
Because sometimes github updates are missed (usually because github
never triggers it), it's possible for the mergebot's view of a PR
description to be incorrect. In that case, the PR may get merged with
the wrong merge message entirely, through no fault of the user.

Since we already fetch the PR info when staging it, there's very
little overhead to checking that the PR message we store is correct
then, and update it if it's not. This means the forward-port's
description should also be correct.

While at it, clean the forward port PR's creation a bit:

- there should always be a message since the title is required on
  PRs (only the body can be missing), therefore no need to check that
- as we're adding a bunch of pseudo-headers, there always is a body,
  no need for the condition
- inline the `pr_data` and `URL`: they were extracted for the support
  of draft PRs, since that's been removed it's now unnecessary

Fixes #530
2021-09-24 15:18:36 +02:00
xmo-odoo
874719870d
[FIX] runbot_message: error on PR page
The page of PRs in "error" is currently kinda broken: it does not show
any feedback aside from the PR being in error which is not very
useful.

The intent was always to show an explanation, but when adding the page
I just deref'd `staging_id` which always fails though in two different
ways:

* when the PR can not be staged at all (because of a conflict) there
  is no staging at all with a reason to show, so there should be
  a fallback that the PR could not even be staged
* `staging_id` is a related field which deref's to the staging_ids
  of the first *active* batch, except when a staging completes
  (successfully or not) both staging and batch are disabled.

  Plus the first batch will be the one for the first staging so if the
  PR is retried and fails again the wrong reason may be displayed.

  So update the section to show what we want: the reason of the
  staging of the *last* batch attached to the PR.

NOTE: there's one failure mode remaining, namely if a staging fails
      then on retry the PR conflicts with the new state of the
      repository (so it can't be staged at all), the "reason" will
      remain that of the staging. This could be mitigated by attaching
      a "nonsense" batch on failure to stage (similar to the
      forwardport stuff), that batch would have no staging, therefore
      no staging reason, therefore fallback.

Closes #525
2021-08-30 14:40:38 +02:00
xmo-odoo
e5f84dc380
[FIX] runbot_merge: add migration for draft column (#523)
Draft was added in 82174ae66e but turns
out the v13 ORM is not able to create a required column (even when
given a default value), at least for booleans.

So create it by hand.
2021-08-25 15:59:22 +02:00