Skip to content

[17.0][REF] auditlog: fetch field data as user - #3115

Open
gbrito wants to merge 1 commit into
OCA:17.0from
gbrito:17.0_auditlog_2554
Open

[17.0][REF] auditlog: fetch field data as user#3115
gbrito wants to merge 1 commit into
OCA:17.0from
gbrito:17.0_auditlog_2554

Conversation

@gbrito

@gbrito gbrito commented Nov 8, 2024

Copy link
Copy Markdown

Currently auditlog fetch field data as sudo
It doesn't make sense to use sudo to fetch the data since the user only have access to edit fields that he has access too. By using sudo we bypass multi company rules

This commit will fix #2554

@gbrito
[REF] fetch field data as user
c60e829 
Currently auditlog fetch field data as sudo
It doesn't make sense to use sudo to fetch the data since the user only
have access to edit fields that he has access too. By using sudo we
bypass multi company rules

This commit will fix OCA#2554
@gbrito
gbrito force-pushed the 17.0_auditlog_2554 branch from 47e17f3 to c60e829 Compare November 8, 2024 10:25
@gbrito

gbrito commented Nov 8, 2024

Copy link
Copy Markdown
Author

@NL66278 @andreschenkels Can you guys make a functional test on it, as far as I see it works as expected and didn't broke any functionality

NL66278
NL66278 approved these changes Nov 8, 2024
View reviewed changes

@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

andreschenkels
andreschenkels approved these changes Nov 12, 2024
View reviewed changes

@andreschenkels andreschenkels 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.

LGTM

@OCA-git-bot

OCA-git-bot commented Nov 13, 2024

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

andreschenkels
andreschenkels reviewed Nov 13, 2024
View reviewed changes

@andreschenkels andreschenkels 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.

@gbrito on a multi companie environment this still procues an error.

StefanRijnhart
StefanRijnhart reviewed Feb 12, 2025
View reviewed changes

@StefanRijnhart StefanRijnhart left a comment •
edited
Loading

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.

Can you add a reproducing test? There is already a multicompany test in test_multi_company.py that you can tag along to.
Also, please mention the module name in the commit message, and rebase to fix the trivial conflict.

@StefanRijnhart StefanRijnhart changed the title [REF] fetch field data as user [18.0][REF] auditlog: fetch field data as user Feb 12, 2025
@NL66278

NL66278 commented Jul 7, 2025

Copy link
Copy Markdown
Contributor

@gbrito The PR is actually for 17.0, not for 18.0 Please update the description. Also needs rebase.

@StefanRijnhart StefanRijnhart changed the title [18.0][REF] auditlog: fetch field data as user [17.0][REF] auditlog: fetch field data as user Jul 7, 2025
@StefanRijnhart

StefanRijnhart commented Jul 7, 2025

Copy link
Copy Markdown
Member

@NL66278 description was my bad, fixed now.

@StefanRijnhart StefanRijnhart added this to the 17.0 milestone Jul 7, 2025
@imunisasi imunisasi mentioned this pull request Jul 16, 2025
Closed
@n17

n17 commented Aug 1, 2025

Copy link
Copy Markdown

This resolved #2554 in our case.

n17 added a commit to avoinsystems/server-tools that referenced this pull request Aug 6, 2025
@n17
[REF] fetch field data as user
a0a2cfe 
Currently auditlog fetch field data as sudo
It doesn't make sense to use sudo to fetch the data since the user only
have access to edit fields that he has access too. By using sudo we
bypass multi company rules

This commit will fix OCA#2554

This code was copied from OCA#3115.
@StefanRijnhart

StefanRijnhart commented Oct 1, 2025

Copy link
Copy Markdown
Member

#2554 should be solved now with #3373. Does your change have additional value that you want to rebase or can we close this PR?

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

Reviewers

@StefanRijnhart StefanRijnhart StefanRijnhart left review comments
@dreispt dreispt dreispt approved these changes
+2 more reviewers
@andreschenkels andreschenkels andreschenkels approved these changes
@NL66278 NL66278 NL66278 approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

approved ready to merge

Projects

None yet

Milestone

17.0

Development

Successfully merging this pull request may close these issues.

7 participants

@gbrito @OCA-git-bot @NL66278 @StefanRijnhart @n17 @dreispt @andreschenkels