Bug 33667 - 'Copy to all libraries' doesn't work on editing holidays
Summary: 'Copy to all libraries' doesn't work on editing holidays
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Emmi Takkinen
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on: 4985
Blocks:
  Show dependency treegraph
 
Reported: 2023-05-03 14:52 UTC by Katrin Fischer
Modified: 2023-12-28 20:47 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.11.00,23.05.02,22.11.08


Attachments
Bug 33667: Allow copying holidays to all libraries when editing (7.03 KB, patch)
2023-05-23 12:30 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 33667: Allow copying holidays to all libraries when editing (7.07 KB, patch)
2023-05-23 19:20 UTC, Sam Lau
Details | Diff | Splinter Review
Bug 33667: Allow copying holidays to all libraries when editing (7.17 KB, patch)
2023-06-30 08:25 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Katrin Fischer 2023-05-03 14:52:11 UTC
When editing an existing holiday and checking the "copy to all libraires" checkbox, the other calendars won't get updates.

To test:

A:
* Add a new unique holiday to branch A
* Check 'Copy to all libraries' checkbox
* Save
* Verify the entry shows on all calendars, within the calendar and in list on the right

B:
* Add another unique holiday to branch A
* Don't check checkbox 'Copy to all libraries' 
* Save
* Verify the holidays shows on all calendars as a green box (?)
* Edit the holiday, now check 'copy to all libraries'
* Verify nothing has changed in other calendars: only the green box, no holiday in list on the right
* Edit again, make a change to description, check checkbox, save
* Verify it's still not showing in the other calendars
Comment 1 Jonathan Druart 2023-05-10 07:05:37 UTC
Emmi, can you have a look at this one please?
Comment 2 Emmi Takkinen 2023-05-10 08:04:54 UTC
Modifying holidays only affects to existing holidays and since holiday created and edited in branch A doesn't exists in branches B, C etc. "Copy changes to all libraries" has no effect. 

Maybe we could call "isHoliday" before calling e.g. "ModDaymonthholiday" and then add holiday if it doesn't exists?
Comment 3 Katrin Fischer 2023-05-10 09:24:26 UTC
I think it would be great to at least have some visual clue or not to provide the option if the holiday is not present in all other branches.

I believe the use case here is:

* add a new holiday
* realize I forgot to set the checkbox
* try to fix it with editing (because it looks like it will work)

So either we make it work... or make it somehow clear it won't and you need to delete first.

I don't know the code well enough to make a recommendation or helpful comment in that part. :(
Comment 4 Emmi Takkinen 2023-05-12 12:33:57 UTC
Imo, it would probably be far more simpler just to make it work instead of checking every calendar for matching holiday.
Comment 5 Katrin Fischer 2023-05-12 14:48:57 UTC
You are probably right. Does it need a little hint/warning maybe: may overwrite existing holidays in other calendars?
Comment 6 Emmi Takkinen 2023-05-15 08:40:28 UTC
(In reply to Katrin Fischer from comment #5)
> You are probably right. Does it need a little hint/warning maybe: may
> overwrite existing holidays in other calendars?

Good idea.

I'll assign myself on this for now. Hopefully I'm able to work on this soon.
Comment 7 Emmi Takkinen 2023-05-23 12:30:29 UTC
Created attachment 151562 [details] [review]
Bug 33667: Allow copying holidays to all libraries when editing

When editing an existing holiday and checking the
"copy to all libraires" checkbox, the other calendars won't
get updates. Allow this by first checking if holiday exists
in target calendar and if not, add it.

To test:
1. Add unique holiday to branch A.
2. Don't check checkbox "Copy to all libraries".
3. Save.
4. Verify the holidays shows on all calendars as
a green box.
5. Edit the holiday, now check "Copy to all libraries"
and save.
=> Verify nothing has changed in other calendars:
only the green box, no holiday in list on the right
6. Edit again, make a change to description,
check checkbox, save.
=> Verify it's still not showing in the other
calendars.
9. Apply this patch.
10. Edit holiday again, check "Copy to all libraries"
and save.
=> Verify holiday is now added to other calendars.
11. Edit again, this time do not copy and save.
=> Verify holiday was edited just in branch A.
12. Again edit, check and save.
=> Verify holiday was edited in all libraries.

Sponsored-by: Koha-Suomi Oy
Comment 8 Sam Lau 2023-05-23 19:20:05 UTC
Created attachment 151602 [details] [review]
Bug 33667: Allow copying holidays to all libraries when editing

When editing an existing holiday and checking the
"copy to all libraires" checkbox, the other calendars won't
get updates. Allow this by first checking if holiday exists
in target calendar and if not, add it.

To test:
1. Add unique holiday to branch A.
2. Don't check checkbox "Copy to all libraries".
3. Save.
4. Verify the holidays shows on all calendars as
a green box.
5. Edit the holiday, now check "Copy to all libraries"
and save.
=> Verify nothing has changed in other calendars:
only the green box, no holiday in list on the right
6. Edit again, make a change to description,
check checkbox, save.
=> Verify it's still not showing in the other
calendars.
9. Apply this patch.
10. Edit holiday again, check "Copy to all libraries"
and save.
=> Verify holiday is now added to other calendars.
11. Edit again, this time do not copy and save.
=> Verify holiday was edited just in branch A.
12. Again edit, check and save.
=> Verify holiday was edited in all libraries.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Sam Lau <samalau@gmail.com>
Comment 9 Marcel de Rooy 2023-06-30 08:25:57 UTC
Created attachment 152889 [details] [review]
Bug 33667: Allow copying holidays to all libraries when editing

When editing an existing holiday and checking the
"copy to all libraires" checkbox, the other calendars won't
get updates. Allow this by first checking if holiday exists
in target calendar and if not, add it.

To test:
1. Add unique holiday to branch A.
2. Don't check checkbox "Copy to all libraries".
3. Save.
4. Verify the holidays shows on all calendars as
a green box.
5. Edit the holiday, now check "Copy to all libraries"
and save.
=> Verify nothing has changed in other calendars:
only the green box, no holiday in list on the right
6. Edit again, make a change to description,
check checkbox, save.
=> Verify it's still not showing in the other
calendars.
9. Apply this patch.
10. Edit holiday again, check "Copy to all libraries"
and save.
=> Verify holiday is now added to other calendars.
11. Edit again, this time do not copy and save.
=> Verify holiday was edited just in branch A.
12. Again edit, check and save.
=> Verify holiday was edited in all libraries.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Sam Lau <samalau@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 10 Tomás Cohen Arazi 2023-07-06 13:10:40 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 11 Martin Renvoize 2023-07-17 15:44:52 UTC
Thanks for all the hard work!

Pushed to 23.05.x for the next release
Comment 12 Matt Blenkinsop 2023-07-18 13:33:09 UTC
Nice work everyone!

Pushed to oldstable for 22.11.x