Skip to content

[16.0][FIX] auditlog: fix caching; [ADD] test_auditlog - #3371

Merged
OCA-git-bot merged 2 commits into
OCA:16.0from
StefanRijnhart:16.0-auditlog-fix-cache_sanitation
Oct 1, 2025
Merged

OCA-git-bot merged 2 commits into
OCA:16.0from
StefanRijnhart:16.0-auditlog-fix-cache_sanitation

Conversation

@StefanRijnhart

@StefanRijnhart StefanRijnhart commented Sep 16, 2025

Copy link
Copy Markdown
Member

Fix caching and add a dedicated test module to run tests on the accounting models.

Fixes creating credit notes with audit rules on account.move.line, writing taxes on products as a user that does not have access to all companies for which there are taxes on the product, and more.

Fixes #3308, #3224, #2561, #2554

@StefanRijnhart StefanRijnhart changed the title [ADD] test_auditlog [16.0][ADD] test_auditlog Sep 16, 2025
@StefanRijnhart StefanRijnhart added this to the 16.0 milestone Sep 16, 2025
@StefanRijnhart StefanRijnhart changed the title [16.0][ADD] test_auditlog [16.0][FIX] auditlog: fix caching; [ADD] test_auditlog Sep 16, 2025
@StefanRijnhart
StefanRijnhart force-pushed the 16.0-auditlog-fix-cache_sanitation branch from e03ad0e to 24bbfcc Compare September 16, 2025 08:40
@StefanRijnhart
StefanRijnhart marked this pull request as ready for review September 16, 2025 08:51
@StefanRijnhart
StefanRijnhart force-pushed the 16.0-auditlog-fix-cache_sanitation branch from 24bbfcc to 15a1e67 Compare September 16, 2025 14:28

@NL66278 NL66278 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 LGTM and extensive unit tests

@StefanRijnhart
StefanRijnhart marked this pull request as draft September 16, 2025 16:06
@StefanRijnhart

Copy link
Copy Markdown
Member Author

Setting this to draft as I've encountered an issue swapping out the cache as is done here.

@StefanRijnhart

Copy link
Copy Markdown
Member Author

Fixed now. Issue is explained in the docstring of the new context manager.

@StefanRijnhart
StefanRijnhart marked this pull request as ready for review September 16, 2025 19:55
@NL66278

NL66278 commented Sep 17, 2025

Copy link
Copy Markdown
Contributor

@StefanRijnhart The contextmanager seems to be a quite clean solution. Maybe useful in more places.

@StefanRijnhart
StefanRijnhart force-pushed the 16.0-auditlog-fix-cache_sanitation branch from dbf5d80 to 23e8e4e Compare September 18, 2025 11:42
@StefanRijnhart

Copy link
Copy Markdown
Member Author

Rebased to fix conflicts

@CasVissers-360ERP CasVissers-360ERP left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Functional review. Fixed the issue for me.

@StefanRijnhart
StefanRijnhart force-pushed the 16.0-auditlog-fix-cache_sanitation branch from 23e8e4e to c6aedd0 Compare September 24, 2025 17:18
@StefanRijnhart

Copy link
Copy Markdown
Member Author

Rebased after the merge of #3364, which was originally included here.

@OCA-git-bot

Copy link
Copy Markdown
Contributor

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). 🤖

Comment thread auditlog/models/rule.py
@NL66278

NL66278 commented Sep 24, 2025

Copy link
Copy Markdown
Contributor

@thomaspaulb Could you merge this one?

@amh-mw amh-mw left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review only, LGTM. Though superior to cache poisoning, I'm curious how much slower this makes full audit logging

@LoisRForgeFlow LoisRForgeFlow left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this! 👍

/ocabot merge patch

@OCA-git-bot

Copy link
Copy Markdown
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 16.0-ocabot-merge-pr-3371-by-LoisRForgeFlow-bump-patch, awaiting test results.

@OCA-git-bot

Copy link
Copy Markdown
Contributor

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

@OCA-git-bot
OCA-git-bot merged commit 71f0de4 into OCA:16.0 Oct 1, 2025
9 checks passed
@StefanRijnhart

Copy link
Copy Markdown
Member Author

See #3409 for an important fix for this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants