[18.0][MIG] auditlog: Migration to 18.0

[lembregtse]

No description provided.

[gurneyalex]

Warning, there is an issue on 17.0 resulting in some workers not implementing the correct logging.

#3088 is supposed to fix it.

[etobella]

/ocabot migration auditlog

[StefanRijnhart]

@lembregtse Can you update your branch to include #3088 which is now merged?

[mylbco]

@lembregtse any update on this ;-)

[lembregtse]

@StefanRijnhart my bad, I seem to have "Ctrl+R'ed" into a commit message I used back then to try (as you mentioned) something locally for Raf's fix in the other PR.

I've removed the message from the commit, it was not relevant here, it was for a squash I did to fix the cron data file (removing doall and numbercall).

I've changed the version number as well!

[StefanRijnhart]

Thanks for the update, looking even better than before!

[ristecona]

if possible please merge, thanks a lot

[StefanRijnhart]

@ristecona A technical of functional review would be appreciated!

[MiquelRForgeFlow]

@lembregtse please cherry-pick #3147

[StefanRijnhart]

And #3137

[lembregtse]

@StefanRijnhart I've cherrypicked both commits, fascinatingly it fails in the pipeline, didn't locally. Difference seems to be the "phonenumbers" python package (I did not have it installed locally). Checking it out.

[lembregtse]

It makes sense, since the phone field was excluded, with @rven changes the test-case should have failed in 17.0 as well as it normally should not log any auditlog. I added an extra "changed field" to restore the test-cases original purpose. But I do not understand why installing the phonenumbers makes any difference in how auditlog "logs" stuff. Because I believe this should have failed in 17.0 as well.

[StefanRijnhart]

Thanks for including the most recent changes! You are right, we would expect the tests to fail on 17 as well (they did for me locally) but in some cases the field phone_sanitized is written in the same call. I picked your fix in #3153.
Can you therefore align the commit message of the fixing commit to [FIX] auditlog: adapt exclude fields test to #3137?

[lembregtse]

@StefanRijnhart done!

[StefanRijnhart]

Thanks!

[miikanissi]

Can this get merged?

[StefanRijnhart]

@miikanissi the rules here are:

The code will be merged upon 3 positive reviews within 5 days (or 2 after more than 5 days). At least one of the review above must be from a member of the PSC or someone from the OCA Core Maintainer.

As mine is the only review so far, this PR does not qualify for merging yet. Your review can help! At this point, a functional test would make most sense I think. You can test using this PRs runboat container, see https://odoo-community.org/resources/review. If you are familiar with this module's code, a code review is fine too, of course.

[miikanissi]

LGTM - did a functional test with multiple different rulesets and models.

[OCA-git-bot]

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

[StefanRijnhart]

/ocabot merge nobump

[OCA-git-bot]

On my way to merge this fine PR!
Prepared branch 18.0-ocabot-merge-pr-3054-by-StefanRijnhart-bump-nobump, awaiting test results.

[OCA-git-bot]

Congratulations, your PR was merged at e88484e. Thanks a lot for contributing to OCA. ❤️