Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[17.0] [FIX] auditlog: Dismiss logging when not needed. #3137

Merged
merged 1 commit into from
Dec 13, 2024

Conversation

rven
Copy link
Contributor

@rven rven commented Dec 4, 2024

When fast logging is enabled on the audit log rules, empty records are created when fields are excluded from the list.
This results in a lot of empty unneeded log records, without any details.

This PR solves the issue so only a log record is created when needed.

@rven rven force-pushed the 17.0-limit-logging branch from c3ef867 to dbb4207 Compare December 4, 2024 13:08
@rven rven changed the title [MOD] Dismiss logging when not needed. [MOD] auditlog: Dismiss logging when not needed. Dec 4, 2024
@rven rven changed the title [MOD] auditlog: Dismiss logging when not needed. [17.0] [MOD] auditlog: Dismiss logging when not needed. Dec 4, 2024
@rven rven force-pushed the 17.0-limit-logging branch 4 times, most recently from b2c8180 to 48bed54 Compare December 4, 2024 14:36
Copy link
Member

@StefanRijnhart StefanRijnhart left a comment

Choose a reason for hiding this comment

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

Thanks! This is literally a ticket in one of my backlogs 😃

The module name is included in the PR title, but not in the commit message. Can you add it there as well? And strictly speaking, [MOD] is not a valid tag listed in https://www.odoo.com/documentation/16.0/contributing/development/git_guidelines.html#tag-and-module-name. I think this counts as a [FIX]. Please update the commit message on that point as well.

Do you think you can include a small test so that we can rule out a regression in the future?

@rven rven force-pushed the 17.0-limit-logging branch 3 times, most recently from 688cdfd to 151c7ac Compare December 5, 2024 11:57
@rven rven changed the title [17.0] [MOD] auditlog: Dismiss logging when not needed. [17.0] [FIX] auditlog: Dismiss logging when not needed. Dec 5, 2024
@rven rven force-pushed the 17.0-limit-logging branch from 151c7ac to cbf5478 Compare December 5, 2024 11:59
@rven rven force-pushed the 17.0-limit-logging branch from cbf5478 to 23bb199 Compare December 5, 2024 11:59
@rven rven requested a review from StefanRijnhart December 5, 2024 12:06
Copy link
Member

@StefanRijnhart StefanRijnhart left a comment

Choose a reason for hiding this comment

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

Excellent, thanks!

Copy link
Contributor

@MiquelRForgeFlow MiquelRForgeFlow left a comment

Choose a reason for hiding this comment

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

LGTM

@OCA-git-bot
Copy link
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). 🤖

@StefanRijnhart
Copy link
Member

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 17.0-ocabot-merge-pr-3137-by-StefanRijnhart-bump-patch, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit b906f8e into OCA:17.0 Dec 13, 2024
9 checks passed
@OCA-git-bot
Copy link
Contributor

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

StefanRijnhart pushed a commit to StefanRijnhart/server-tools that referenced this pull request Dec 27, 2024
StefanRijnhart pushed a commit to StefanRijnhart/server-tools that referenced this pull request Dec 27, 2024
In OCA#3137 it is prevented to create auditlogs without log lines.

In this particular test, it is tested that no log line is created for excluded
fields. We would expect therefore that no auditlog is created after OCA#3137, but
depending on the setup, an log line may be created for field phone_sanitized
which is not an excluded field in the test.

To fix this, the test is adapted to write another, non excluded field so that
the auditlog is created in any case.
@StefanRijnhart
Copy link
Member

StefanRijnhart commented Dec 27, 2024

@rven Please check #3153 that adapts a test to this change

lembregtse added a commit to lembregtse/server-tools that referenced this pull request Dec 27, 2024
adhoc-cicd-bot pushed a commit to adhoc-cicd/oca-server-tools that referenced this pull request Jan 13, 2025
adhoc-cicd-bot pushed a commit to adhoc-cicd/oca-server-tools that referenced this pull request Jan 16, 2025
In OCA#3137 it is prevented to create auditlogs without log lines.

In this particular test, it is tested that no log line is created for excluded
fields. We would expect therefore that no auditlog is created after OCA#3137, but
depending on the setup, an log line may be created for field phone_sanitized
which is not an excluded field in the test.

To fix this, the test is adapted to write another, non excluded field so that
the auditlog is created in any case.
adhoc-cicd-bot pushed a commit to adhoc-cicd/oca-server-tools that referenced this pull request Jan 20, 2025
adhoc-cicd-bot pushed a commit to adhoc-cicd/oca-server-tools that referenced this pull request Jan 27, 2025
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.

4 participants