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.

Sign up for GitHub

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

Jump to bottom

[17.0][MIG] hr_holidays_natural_period: Migration to 17.0 #133

Closed
wants to merge 23 commits into from
Closed

[17.0][MIG] hr_holidays_natural_period: Migration to 17.0 #133

wants to merge 23 commits into from

Conversation

peluko00
Copy link
Contributor

No description provided.

victoralmau and others added 5 commits July 22, 2024 17:30
@victoralmau @peluko00
[ADD] hr_holidays_natural_period: Add natural day in request unit lea…
9dbc590 
…ve types

[UPD] Update hr_holidays_natural_period.pot

[UPD] README.rst
@victoralmau @peluko00
[FIX] hr_holidays_natural_period: Prevent error in form leave when em…
cff3a89 
…ployee is not set yet

hr_holidays_natural_period 13.0.1.0.1
@victoralmau @peluko00
[FIX] hr_holidays_natural_period: Prevent error in leave form without…
2b906a4 
… employee set yet

hr_holidays_natural_period 13.0.1.0.2
@joao-p-marques @peluko00
[IMP] *: pre-commit run
e7dbac6
@victoralmau @peluko00
[FIX] hr_holidays_natural_period: Fix _exist_interval_in_date() funct…
18ba33a 
…ion.

Previously, the _exist_interval_in_date () function always returned False, which always created a new interval.
Although this was totally incorrect, it was detected in a real use case when there were working days and holidays in the date range causing incorrect overtime.
TT33779

hr_holidays_natural_period 13.0.1.0.3
@peluko00 peluko00 mentioned this pull request Jul 22, 2024
Open
4 tasks
@peluko00 peluko00 force-pushed the 17.0-mig-hr_holidays_natural_period branch 5 times, most recently from 78c67a4 to df0211e Compare July 23, 2024 09:59
Noel000 and others added 17 commits July 23, 2024 11:59
@Noel000 @peluko00
Added translation using Weblate (Catalan)
5108d68
@victoralmau @peluko00
[IMP] hr_holidays_natural_period: black, isort, prettier
291d9f1
@victoralmau @peluko00
[MIG] hr_holidays_natural_period: Migration to 14.0
ffb27a7 
TT37026

[UPD] Update hr_holidays_natural_period.pot

[UPD] README.rst
@badbole @peluko00
Added translation using Weblate (Croatian)
af2e856
@victoralmau @peluko00
[MIG] hr_holidays_natural_period: Migration to 15.0
05df64b 
TT40362
@victoralmau @peluko00
Added translation using Weblate (Spanish)
7c285bc
@victoralmau @peluko00
[FIX] hr_holidays_natural_period: Avoid error when saving leave
8fc7a31 
TT42608
@OCA-git-bot @peluko00
hr_holidays_natural_period 15.0.1.0.1
9e75479
@victoralmau @peluko00
[IMP] hr_holidays_natural_period: Add freeze_time to prevent errors i…
e09995e 
…n tests
@OCA-git-bot @peluko00
hr_holidays_natural_period 15.0.1.0.2
cda6402
@OsoTranquilo @peluko00
[MIG] hr_holidays_natural_period: Migration to 16.0
9b28236
@mymage @peluko00
Added translation using Weblate (Italian)
0e8d871
@Ivorra78 @peluko00
Translated using Weblate (Spanish)
3753b0e 
Currently translated at 100.0% (6 of 6 strings)

Translation: hr-holidays-16.0/hr-holidays-16.0-hr_holidays_natural_period
Translate-URL: https://translation.odoo-community.org/projects/hr-holidays-16-0/hr-holidays-16-0-hr_holidays_natural_period/es/
@OCA-git-bot @peluko00
[UPD] README.rst
73b58fa
@victoralmau @peluko00
[FIX] hr_holidays_natural_period: Recalculate the duration value on c…
c71d886 
…reation

It is necessary to recalculate the duration value in the create() method when it is done by UX.

Fixes OCA#105
@OCA-git-bot @peluko00
[BOT] post-merge updates
8a4ad45
@peluko00
[IMP] hr_holidays_natural_period: pre-commit auto fixes
5dd84c2
@peluko00 peluko00 force-pushed the 17.0-mig-hr_holidays_natural_period branch from df0211e to 6b1e64c Compare July 23, 2024 09:59
@carlos-lopez-tecnativa
Copy link

@peluko00 please check the CI. The tests are failing.

When I create a new leave, I get an error.
image

@Tecnativa TT50070

@peluko00 peluko00 force-pushed the 17.0-mig-hr_holidays_natural_period branch from 6b1e64c to 0a87084 Compare August 14, 2024 13:05
@peluko00
Copy link
Contributor Author

@peluko00 please check the CI. The tests are failing.

When I create a new leave, I get an error. image

@Tecnativa TT50070

I know but in my dev environment the test aren't failing and the module works good

carlos-lopez-tecnativa
carlos-lopez-tecnativa suggested changes Aug 14, 2024
View reviewed changes
Copy link

@carlos-lopez-tecnativa carlos-lopez-tecnativa left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@peluko00 the computation seems incorrect from my perspective. Please compare version 16 with version 17.

V16
image

V17
image

If an employee works from Monday to Friday, it counts as 7 days according to the employee calendar. However, this module should not consider the employee calendar. Therefore, from the 1st to the 10th should be counted as 10 natural days.

@peluko00 peluko00 force-pushed the 17.0-mig-hr_holidays_natural_period branch from 0a87084 to 551ea48 Compare August 15, 2024 06:20
@peluko00
Copy link
Contributor Author

@peluko00 the computation seems incorrect from my perspective. Please compare version 16 with version 17.

V16 image

V17 image

If an employee works from Monday to Friday, it counts as 7 days according to the employee calendar. However, this module should not consider the employee calendar. Therefore, from the 1st to the 10th should be counted as 10 natural days.

I fix that problem but the tests are failing but no in dev environment

@carlos-lopez-tecnativa
Copy link

I fix that problem but the tests are failing but no in dev environment

@peluko00 I ran the tests on my local machine and encountered the same error. Could you please share how you're running the tests since they seem to be working for you?
image

@peluko00
Copy link
Contributor Author

peluko00 commented Aug 21, 2024 •
edited
Loading

I fix that problem but the tests are failing but no in dev environment

@peluko00 I ran the tests on my local machine and encountered the same error. Could you please share how you're running the tests since they seem to be working for you? image

I ran the tests in my local machine with the normal command for executing tests and only installing hr_holidays_natural_period without installing any more modules.

image

@pedrobaeza
Copy link
Member

@peluko00 there should be something incorrect in your way to run tests, as the log is not correct:

  • Starting with, there's only 2 tests, while your log talks about 4 tests.
  • There's no trace in the log about the execution of the test odoo.addons.hr_holidays_natural_period.tests.test_hr_leave, while it should be.

Maybe you have some touched Odoo version for running the tests or you are not executing the proper command? If you are putting test tags, make sure you put the correct ones.

@peluko00
Copy link
Contributor Author

@peluko00 there should be something incorrect in your way to run tests, as the log is not correct:

  • Starting with, there's only 2 tests, while your log talks about 4 tests.
  • There's no trace in the log about the execution of the test odoo.addons.hr_holidays_natural_period.tests.test_hr_leave, while it should be.

Maybe you have some touched Odoo version for running the tests or you are not executing the proper command? If you are putting test tags, make sure you put the correct ones.

The 4 tests only i see in both images is at the final.
I send another picture with the complete log and i execute the tests with the normal command acording to documentation.

image

@pedrobaeza
Copy link
Member

But are you testing it on installation? Anyway, there's a problem, as stated in this CI or local testing on my colleague, so this migration is not correct while the problem is not fixed. And the test failure highlights that the module may not be working.

@peluko00
Copy link
Contributor Author

peluko00 commented Aug 22, 2024 •
edited
Loading

But are you testing it on installation? Anyway, there's a problem, as stated in this CI or local testing on my colleague, so this migration is not correct while the problem is not fixed. And the test failure highlights that the module may not be working.

I follow the next steps:

  • Create demo database
  • Install the module
  • Run the tests

All tests are running but i don't know why don't throws an error.

@pedrobaeza
Copy link
Member

You have to run tests at the same time of the installation: odoo -i hr_holidays_natural_period --test-enable --test-tags=hr_holidays_natural_period --workers=0 --stop-after-init.

@carlos-lopez-tecnativa
Copy link

Hi, @peluko00, have you reviewed the error in the test? Can I help you with anything?

@peluko00
Copy link
Contributor Author

peluko00 commented Sep 6, 2024 •
edited
Loading

Hi, @peluko00, have you reviewed the error in the test? Can I help you with anything?

Yes, i review but i can't find the error

victoralmau
victoralmau suggested changes Sep 10, 2024
View reviewed changes
Copy link
Member

@victoralmau victoralmau left a comment

Choose a reason for hiding this comment

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

I tried in runboat and it doesn't work (that's why the tests fail too)

ejemplo

The days that should be displayed are 31

@carlos-lopez-tecnativa
Copy link

Superseded by #141

@pedrobaeza pedrobaeza closed this Sep 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@carlos-lopez-tecnativa carlos-lopez-tecnativa carlos-lopez-tecnativa requested changes

@victoralmau victoralmau victoralmau requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.