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

[16.0][FIX] hr_holidays_public*: Use the context employee or the user's employee #132

Merged
merged 1 commit into from
Jul 26, 2024

Conversation

victoralmau
Copy link
Member

@victoralmau victoralmau commented Jul 9, 2024

FWP from 15.0: #130

Locked by:

Use the context employee or the user's employee

Use case:

  • Go to Employees to an employee with a different address (country) than our own and with specific public holidays for that country.
  • Go to the Time-off smart-buttons
  • We will have to see there the public holidays according to the employee's address

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

@Tecnativa TT49839

Copy link

Choose a reason for hiding this comment

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

Funcional review LGTM, Thanks :)

@pedrobaeza
Copy link
Member

@carolinafernandez-tecnativa this isn't working without the Odoo patch. How have you reviewed it functionally?

@carolinafernandez-tecnativa

I attached the examples:

  1. Configure public Holidays for Country United States

image

2- Go to an employee that is not the same as user logged in (for example Eli)

image

3- Press Time-off button

image

And calendar its okay.

@pedrobaeza
Copy link
Member

But that's the question: you have to try with an employee with a different country than ours.

image

image

image

image

@carolinafernandez-tecnativa

Ok could you please add that in Use case PR description? Thanks!

@pedrobaeza
Copy link
Member

The PR description is indicating this same use case. It's a bugfix, so nothing to go on the README itself. You have to check the why of the PR, and ask for more details in they are not clear in the PR description.

@pedrobaeza
Copy link
Member

pedrobaeza commented Jul 10, 2024

Re-reading, it's true that the description is not specific about the different address. It should be:

* Go to Employees to an employee with a different address (country) than our
  own and with specific public holidays for that country.

@victoralmau victoralmau force-pushed the 16.0-fix-hr_holidays_public-TT49839 branch from 03c1695 to 0ff825c Compare July 10, 2024 13:01
@victoralmau victoralmau force-pushed the 16.0-fix-hr_holidays_public-TT49839 branch from 0ff825c to ce3a7b3 Compare July 23, 2024 10:44
…loyee

Use case:
- Go to Employees to an employee with a different address (country) than our
  own and with specific public holidays for that country.
- Go to the Time-off smart-buttons
- We will have to see there the public holidays according to the employee's address

TT49839
@victoralmau victoralmau force-pushed the 16.0-fix-hr_holidays_public-TT49839 branch from ce3a7b3 to 571d5a1 Compare July 26, 2024 06:02
@victoralmau
Copy link
Member Author

Odoo has already merged the related PR.

Can you check it now?

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.

/ocabot merge patch

@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-132-by-pedrobaeza-bump-patch, awaiting test results.

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

@OCA-git-bot OCA-git-bot merged commit 111d9b8 into OCA:16.0 Jul 26, 2024
9 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 05881b1. 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.

4 participants