A nice feature would be, if you can use a implemented country for the standard holidays and add specific holidays by OHM.
There is a hook hook_storm_holidays_alter($holidays, $year, $country). This could be implemented and OHM holidays should be added there, if OHM is not selected as a country (use storm_holidays_consider_holidays() to get selected country)
At the moment this holidays do not appear in the storm_holidays admin settings, maybe you could use the node status to determine, if this OHM holiday is active or not.

CommentFileSizeAuthor
#3 add_ohm_to_country-1310660-3.patch2.45 KBkfritsche

Comments

d34dman’s picture

d34dman commented

This feature is closely related to implementation of Newly added Holidays aren't active by default. Am trying to figure out as many use cases. I want to decide upon possible implementation that would be least confusing to the user. i feel this is more than just a feature request. Cause this feature needs to be implemented if the Newly added Holidays aren't active by default has to be resolved. :)

d34dman’s picture

d34dman commented
Assigned: Unassigned » d34dman

A nice feature would be, if you can use a implemented country for the standard holidays and add specific holidays by OHM.

Right now this feature is implemented.

There is a hook hook_storm_holidays_alter($holidays, $year, $country). This could be implemented and OHM holidays should be added there, if OHM is not selected as a country (use storm_holidays_consider_holidays() to get selected country)

This hook needs to be implemented so that holidays shows up when a country other than Organization Holiday Manager is selected.

At the moment this holidays do not appear in the storm_holidays admin settings, maybe you could use the node status to determine, if this OHM holiday is active or not.

If the selected country is Organization Holiday Manager, then the holidays are listed in there. My next aim is to notify in the node page if the settings that has been defined in node ( ie. enable and half day) is over-ridden by Holiday Manager). This will be fixed in next committ that you can expect today.

kfritsche’s picture

kfritsche
Primary language German
Location 🇩🇪🇪🇺
commented
StatusFileSize
new add_ohm_to_country-1310660-3.patch2.45 KB

This hook needs to be implemented so that holidays shows up when a country other than Organization Holiday Manager is selected.

The hook hook_storm_holidays_alter($holidays, $year, $country) is implemented and should be working. (implemented in stormholidays.module line 308)

Right now this feature is implemented.

I think there is a missunderstanding, because i can't see, that this is working right now.
You can select OHM or a Country, but not both yet.
My idea was, if you select OHM, all works like it does right now, no changed needed.
If you select a Country, than OHM will add his holidays.

I created a patch, which does that. The problem is that when a Country is selected, the OHM holidays aren't displayed anywhere, instead of admin/content/node.
There are three options:
1. Somewhere there is documented, that holidays only shows up in the content list, if OHM is not selected as country. -> absolutely not user-friendly
2. There is a separated OHM holiday list (like on admin/settings/storm/ohm)
3. The OHM holidays are added somehow to the holiday form (as form_alter or adding an hook or something to storm_holiday)
Maybe there is also another solution, i didn't think of now.
These are all only proposals.

d34dman’s picture

d34dman commented

Hi kfritsche,
I have committed your patch with certain modification, but it ain't working as expected. I am sorry that i reported its done when actually it was not pushed to server.

Am thinking of altering storm_holidays form to include a checkbox that would let user choose how storm ohm holidays should interact with country holidays.

1. if checked -> OHM holidays will be included with country holidays.
2. if unchecked -> OHM holidays will come into picture only if Storm Organization Holidays Manager is selected as a country.

savy?

kfritsche’s picture

kfritsche
Primary language German
Location 🇩🇪🇪🇺
commented

sounds reasonably

d34dman’s picture

d34dman commented
Status: Active » Needs review

Hi there,

i have committed the feature into storm_ohm. The configuration checkbox is available in storm_contrib form. Right now the holidays wont show up in country list due to a bug storm holiday.

To test the feature one can run following code and test it.

<?php
$country = storm_holidays_consider_holidays();
$var = storm_holidays_get_holidays($country);
dsm($var);
?>