10.0 public holidays next year #291

jcdrubay · 2016-11-04T10:27:51Z

Allow the creation of public holidays for next year through a wizard.

Minor odoolint fixes

Note: Recent merged PR#290 has been taken into account.

… following year

oca-clabot · 2016-11-04T10:27:53Z

Hey @jcdrubay, thank you for your Pull Request.

It looks like some users haven't signed our Contributor License Agreement, yet.
You can read and sign our full Contributor License Agreement here: http://odoo-community.org/page/website.cla
Here is a list of the users:

@jcdrubay (login unknown in OCA database)

Appreciation of efforts,
OCA CLAbot

LeartS

I think it would be useful to let the user select which hr.public.holiday to replicate, defaulting to the most recent one for each country as it is now; and also let the user choose the year (defaulting to the next).
For example if 2016 was a special year with an additional public holiday for the 150th anniversary of the Italian unification, I'd want to replicate the 2015 Italian holidays to 2017.

LeartS · 2016-11-06T11:50:15Z

hr_public_holidays/wizards/public_holidays_next_year_wizard.xml

+    <field name="model">public.holidays.next.year.wizard</field>
+    <field name="arch" type="xml">
+        <form string="Create Next Year Public Holidays">
+            <footer>


I would add a little description here that briefly explains what the wizard does. The empty wizard looks, well, empty

LeartS · 2016-11-06T11:56:55Z

hr_public_holidays/wizards/public_holidays_next_year_wizard.py

+    def create_next_year_public_holidays(self):
+
+        ph_env = self.env['hr.holidays.public']
+        pholidays = ph_env.search([])


why not directly pholidays = self.env['hr.holidays.public'].search([]), as you never use ph_env again?

Mainly because of an habit.
But in next commit, it will make it easier to use later if necessary. In the next commit, it will also simplify a bit to avoid code on 2 lines for this:

ph_env = self.env['hr.holidays.public'] pholidays = self.template_ids or ph_env.search([])

Also, I think it will mean less changes if we need to re-use it later on.

If required, I can change, let me know.

LeartS · 2016-11-06T13:04:21Z

hr_public_holidays/wizards/public_holidays_next_year_wizard.py

+        last_ph_dict = {}
+        for ph in pholidays:
+
+            curr_country = ph.country_id and ph.country_id.id or NO_COUNTRY


idiomatic python: curr_country = ph.country_id.id if ph.country_id else NO_COUNTRY
But you could even put directly the country recordset as the dict keys and skip the check altoghether. This is possible because recordset implements the hash method.
last_ph_country = last_ph_dict.get(ph.country_id, False)

Nice trick! :)

LeartS · 2016-11-06T13:06:04Z

hr_public_holidays/wizards/public_holidays_next_year_wizard.py

+
+        NO_COUNTRY = 'NO_COUNTRY'
+        last_ph_dict = {}
+        for ph in pholidays:


Maybe a warning/error if no holidays defined?

LeartS · 2016-11-06T13:25:51Z

hr_public_holidays/wizards/public_holidays_next_year_wizard.py

+            for last_ph_line in last_ph.line_ids:
+
+                ph_line_date = fields.Date.from_string(last_ph_line.date)
+                new_date = ph_line_date.replace(ph_line_date.year + 1)


Very rare to have a public holiday in that day, but if the date is February 29 of a leap year, you'll get ValueError: day is out of range for month when doing this

Three cases this year.

Source: http://www.qppstudio.net/monthlyholidays/bank-holidays-february-2016.htm

Handling this rare case will mean quite a lot of complexity because previous or next day might also be a public holiday.

I suggest to block the user with a UserError: "You cannot use a the template of a year that includes public holidays on 29th of February (2016, 2020...), please select a template from another year.". Since I take into account your first comment, the users will be able to select the public holidays from 2015 instead for 2017; I guess it is good enough.

…-hr into 10.0-public-holidays-next-year

…enus

…for a specific year, and display a UserError for templates including February 29th.

jcdrubay · 2016-11-07T08:47:19Z

Thanks @LeartS for your great feedback. I tried to take them into account in the best way. I am a bit scared so about the template and year field on the wizard as they create a lot of complexity for the users. I tried to make it clear on the wizard page with some documentation...

Please let me know if you have any additional suggestion.

LeartS · 2016-11-07T09:13:58Z

hr_public_holidays/wizards/public_holidays_next_year_wizard.xml

+        <form string="Create Next Year Public Holidays">
+            <sheet>
+                <div>
+                        Use this wizard to create pulic holidays based on the 


s/pulic/public

LeartS · 2016-11-07T09:22:21Z

hr_public_holidays/wizards/public_holidays_next_year_wizard.xml

+                    <group string="Optional">
+                        <div colspan="2">
+                        The below optional fields are here only to handle 
+                        special situtaions like "2016 was a special year with 


s/situtaions/situations

(The 150th anniversary of Italian unification was actually in 2011, I used 2016 in my example just to make it more current 😁 )

I would put this group under the first one instead of next to it, it looks clumsy:

Or we could even maybe put the two groups in two notebook pages so that the Optional section is "hidden" unless one explicitly clicks on it; while the "Defaults" explanation is immediately displayed. This could make the wizard look simpler and not overload the user with options/fields, what do you think?

I like your notebook idea 👍

feketemihai · 2017-02-23T06:32:09Z

@jcdrubay Thanks for the contribution, good module...

jcdrubay · 2017-02-28T07:45:43Z

Yeah! 3 approvals reached 🎉 Thank you everyone 👍

* [ADD] public holidays: add a wizard to create public holidays for the following year * [REF] odoolint improvements * [REF] adjustments for flake8 and pylint test * Move public holidays menu under Leaves menu. (OCA#290) * [ADJ] hr_public_holidays: take into account the need of several sub-menus * [ADD] Allow the creation of public holidays from a specific template for a specific year, and display a UserError for templates including February 29th. * [REF] removing useless comments * [ADJ] hr_public_holidays: Improving wizard display * [REF] Removing tab characters

oca-clabot · 2017-06-24T02:51:47Z

Hey @jcdrubay,
We acknowledge that the following users have signed our Contributor License Agreement:

@jcdrubay

Appreciation of efforts,
OCA CLAbot

* [ADD] public holidays: add a wizard to create public holidays for the following year * [REF] odoolint improvements * [REF] adjustments for flake8 and pylint test * Move public holidays menu under Leaves menu. (OCA#290) * [ADJ] hr_public_holidays: take into account the need of several sub-menus * [ADD] Allow the creation of public holidays from a specific template for a specific year, and display a UserError for templates including February 29th. * [REF] removing useless comments * [ADJ] hr_public_holidays: Improving wizard display * [REF] Removing tab characters

…_account2 [BSSFL-410] Update product account

* [ADD] public holidays: add a wizard to create public holidays for the following year * [REF] odoolint improvements * [REF] adjustments for flake8 and pylint test * Move public holidays menu under Leaves menu. (OCA#290) * [ADJ] hr_public_holidays: take into account the need of several sub-menus * [ADD] Allow the creation of public holidays from a specific template for a specific year, and display a UserError for templates including February 29th. * [REF] removing useless comments * [ADJ] hr_public_holidays: Improving wizard display * [REF] Removing tab characters

Jean-Charles Drubay added 2 commits November 4, 2016 17:15

[ADD] public holidays: add a wizard to create public holidays for the…

e1b997e

… following year

[REF] odoolint improvements

b8901d9

[REF] adjustments for flake8 and pylint test

7a4cf11

LeartS reviewed Nov 6, 2016

View reviewed changes

feketemihai and others added 4 commits November 6, 2016 21:08

Move public holidays menu under Leaves menu. (OCA#290)

716f474

Merge branch '10.0-public-holidays-next-year' of github.com:trobz/oca…

456363e

…-hr into 10.0-public-holidays-next-year

[ADJ] hr_public_holidays: take into account the need of several sub-m…

d87753f

…enus

[ADD] Allow the creation of public holidays from a specific template …

04eace2

…for a specific year, and display a UserError for templates including February 29th.

[REF] removing useless comments

7435040

LeartS approved these changes Nov 7, 2016

View reviewed changes

Jean-Charles Drubay added 2 commits November 7, 2016 17:26

[ADJ] hr_public_holidays: Improving wizard display

db091b1

[REF] Removing tab characters

cc3c32a

feketemihai added this to the 10.0 milestone Feb 23, 2017

feketemihai added the needs review label Feb 23, 2017

feketemihai approved these changes Feb 23, 2017

View reviewed changes

gurneyalex approved these changes Feb 28, 2017

View reviewed changes

feketemihai merged commit cd26253 into OCA:10.0 Feb 28, 2017

sambarros pushed a commit to sambarros/hr that referenced this pull request Jul 26, 2018

Merge pull request OCA#291 from LoicSenseFly/BSSFL-410_Update_Product…

29ce23b

…_account2 [BSSFL-410] Update product account

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

10.0 public holidays next year #291

10.0 public holidays next year #291

jcdrubay commented Nov 4, 2016 •

edited

Loading

oca-clabot commented Nov 4, 2016

LeartS left a comment •

edited

Loading

LeartS Nov 6, 2016

LeartS Nov 6, 2016

jcdrubay Nov 7, 2016 •

edited

Loading

LeartS Nov 6, 2016

jcdrubay Nov 7, 2016

LeartS Nov 6, 2016 •

edited

Loading

LeartS Nov 6, 2016

jcdrubay Nov 7, 2016

jcdrubay commented Nov 7, 2016

LeartS Nov 7, 2016

LeartS Nov 7, 2016

jcdrubay Nov 7, 2016

feketemihai commented Feb 23, 2017

jcdrubay commented Feb 28, 2017

oca-clabot commented Jun 24, 2017

10.0 public holidays next year #291

10.0 public holidays next year #291

Conversation

jcdrubay commented Nov 4, 2016 • edited Loading

oca-clabot commented Nov 4, 2016

LeartS left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jcdrubay Nov 7, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LeartS Nov 6, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jcdrubay commented Nov 7, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

feketemihai commented Feb 23, 2017

jcdrubay commented Feb 28, 2017

oca-clabot commented Jun 24, 2017

jcdrubay commented Nov 4, 2016 •

edited

Loading

LeartS left a comment •

edited

Loading

jcdrubay Nov 7, 2016 •

edited

Loading

LeartS Nov 6, 2016 •

edited

Loading