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

Update Hr public holidays. #209

Merged
merged 1 commit into from
Apr 5, 2016
Merged

Conversation

feketemihai
Copy link
Member

This is a update from #173 . I will close that one.

@feketemihai feketemihai added this to the 8.0 milestone Mar 9, 2016
@andhit-r
Copy link
Member

andhit-r commented Mar 9, 2016

Hi @feketemihai. I tried:

  1. Create international public holiday for 2016
    selection_1
  2. I tried to create indonesia's public holiday for 2016
    selection_2
  3. Then i got this error
    selection_3

@andhit-r
Copy link
Member

andhit-r commented Mar 9, 2016

Funny though, i could create 2017 international and 2017 indonesia public holiday. But not 2016

@feketemihai
Copy link
Member Author

It depends on the order that you input..if you input by country first the error is not raised..if you input internationally first the error will be raised...i will work on the constraint function.

@andhit-r
Copy link
Member

andhit-r commented Mar 9, 2016

Yes i confirm that. It works just like you said.

@feketemihai
Copy link
Member Author

It should be fixed now.

@andhit-r
Copy link
Member

andhit-r commented Mar 9, 2016

Thanks @feketemihai . Btw is this by design? Or need extra constraints?

selection_5

@feketemihai
Copy link
Member Author

There wasn't any constraint on this...we can have a new constraint on this just like for the year country, here to have it by date/states...

@andhit-r
Copy link
Member

@feketemihai thanks for extra constraints. One more case still bothers me:

selection_6

When a data declared as national holiday then it should not possible to declare that day as state holiday. Is it possible to add more constraint, or just inform it on known issue?

@andhit-r
Copy link
Member

Well at least in my country (Indonesia) that will not happen. Or it's possible in other countries?

@andhit-r
Copy link
Member

Btw let me know if you need any help to close this PR or any untouch PR on OCA/hr. Thanks.

@andhit-r
Copy link
Member

👍

@azmimr67
Copy link

@feketemihai @andhit-r I'm thinking that constraint that you put on public holiday for national and state cannot be for same day.
I think this still can happen example
17/04/2016 Nyepi
17/04/2016 State Election Tangerang

Because I think the purpose of this module is to record all public holiday on country and state.
Even if we see only from holiday side that day is public holiday.

That's my opinion

But over all I have tested it's OK.

Thanks

@mikevhe18
Copy link

👍

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 86.454% when pulling c251909 on feketemihai:hr_public_holidays into 90c4d1f on OCA:8.0.

@azmimr67
Copy link

azmimr67 commented Apr 1, 2016

has been tested

👍

@pedrobaeza
Copy link
Member

@feketemihai, I only ask you before I merge the module to clean the commit history. Please squash a bit some commits that are only flake8 fixes or similar, and remove the changes in other modules like hr_employee_firstname, because it's not the same in conflicts terms to add something and then remove it, than not touching it.

@feketemihai feketemihai force-pushed the hr_public_holidays branch 2 times, most recently from 135adf5 to 15b787c Compare April 4, 2016 05:24
@coveralls
Copy link

Coverage Status

Coverage increased (+0.3%) to 86.454% when pulling 15b787c on feketemihai:hr_public_holidays into c31d97e on OCA:8.0.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.3%) to 86.454% when pulling 15b787c on feketemihai:hr_public_holidays into c31d97e on OCA:8.0.

@feketemihai
Copy link
Member Author

@pedrobaeza Done...

@pedrobaeza pedrobaeza merged commit 8b43a91 into OCA:8.0 Apr 5, 2016
@max3903 max3903 mentioned this pull request Apr 8, 2016
58 tasks
sambarros pushed a commit to sambarros/hr that referenced this pull request Jul 26, 2018
[BSSFL-282]  Technical shipping groups
Mraimou pushed a commit to camptocamp/hr that referenced this pull request Nov 25, 2019
BSIBSO-1069: Sync project and update image version
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.

None yet

6 participants