Skip to content

[16.0][FIX] maintenance_timesheet: Define the correct rule to show only what is related to maintenance_request_id #501

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

Merged
merged 1 commit into from
Jul 24, 2025

Conversation

victoralmau
Copy link
Member

@victoralmau victoralmau commented Jul 24, 2025

Define the correct rule to show only what is related to maintenance_request_id

Locked by:

Please @pedrobaeza and @pilarvargas-tecnativa can you review it?

@Tecnativa TT56859

@pedrobaeza pedrobaeza added this to the 16.0 milestone Jul 24, 2025
Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

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

The change is not correct as is. I don't say it was correct, but the solution isn't to change = to !=. That way, you give access to all the timesheets related to maintenance. Maybe you need to just remove that part?

@victoralmau
Copy link
Member Author

Right now, I don't know exactly what is not being taken into account.

Before
With the existing rule, you had access to everything that was not related to maintenance_request (maintenance_request=False). That is definitely a total mistake (or at least it is for me).

After
The existing rule is intended to be related only to maintenance requests, which is why the rule only takes into account what is related to maintenance requests (maintenance_request!=False) and what is “linked to its user”.

So what is the appropriate rule?

@dalonsod
Copy link
Contributor

dalonsod commented Jul 24, 2025

The change is not correct as is. I don't say it was correct, but the solution isn't to change = to !=. That way, you give access to all the timesheets related to maintenance. Maybe you need to just remove that part?

Sure, the original code seems to be incorrect, you're right 😕

The new one already makes more changes than moving = to !=, it also removes an extra OR, so it's not only, for the timesheet, required belonging to a maintenance request, but following it as well. I see it now right.

…t is related to maintenance_request_id

TT56859
@victoralmau victoralmau force-pushed the 16.0-fix-maintenance_timesheet-rule branch from ced87c2 to f08beb8 Compare July 24, 2025 10:20
@pedrobaeza
Copy link
Member

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 16.0-ocabot-merge-pr-501-by-pedrobaeza-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit ccccc62 into OCA:16.0 Jul 24, 2025
7 checks passed
@OCA-git-bot
Copy link
Contributor

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

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.

5 participants