Bug 4985 - Copy a change on the calendar to all libraries
Summary: Copy a change on the calendar to all libraries
Status: Pushed to oldoldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Emmi Takkinen
QA Contact: Bugs List
URL: /cgi-bin/koha/tools/holidays.pl
Keywords:
Depends on:
Blocks:
 
Reported: 2010-07-12 15:59 UTC by Owen Leonard
Modified: 2020-07-26 15:00 UTC (History)
10 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
20.11.00, 20.05.02, 19.11.08, 19.05.14


Attachments
Bug 4985: Edit a calendar entry for all branches (10.58 KB, patch)
2020-02-07 11:31 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 4985: Edit a calendar entry for all branches (10.65 KB, patch)
2020-02-07 14:01 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Deleting holidays on a date range not working (76.76 KB, image/png)
2020-02-09 12:56 UTC, Katrin Fischer
Details
Bug 4985: Edit a calendar entry for all branches (10.71 KB, patch)
2020-03-17 10:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 4985: Add missing parameters (1.70 KB, patch)
2020-03-19 07:26 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 4985: Fix Plack scope errors (3.26 KB, patch)
2020-05-11 09:29 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 4985: Edit a calendar entry for all branches (10.80 KB, patch)
2020-05-15 15:06 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 4985: Fix Plack scope errors (3.32 KB, patch)
2020-05-15 15:06 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 4985: Edit a calendar entry for all branches (10.85 KB, patch)
2020-06-21 14:53 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 4985: Fix Plack scope errors (3.38 KB, patch)
2020-06-21 14:53 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Owen Leonard 2010-07-12 15:59:04 UTC
It is possible to create a holiday and copy it to all branches but it's not possible to edit a holiday and apply the change to all branches.
Comment 1 jos.kohado 2010-08-17 14:44:13 UTC
Not sure if this should be a separate bug, but it also doesn't look like it is possible to delete a holiday from all branches at once.
Comment 2 Katrin Fischer 2013-03-24 18:16:21 UTC
Deleting and editing holidays for all branches at once is still not possible in current versions of Koha.
Comment 3 paxed 2019-06-06 09:39:57 UTC
Still valid, looks like.
Comment 4 Emmi Takkinen 2020-02-07 11:31:44 UTC
Created attachment 98556 [details] [review]
Bug 4985: Edit a calendar entry for all branches

Bug 4985: Edit a calendar entry for all branches

This patch adds functionality to calendar tool to
make edits or deletes to all branches at once.

To test:
-Apply patch
-Add holiday for a library and copy it to all or some
libraries
-Edit previosly made holiday (edit, delete etc.) and
check out "Copy changes to all libraries." checkbox
=> Changes are made to all libraries sharing changed
holiday

Sponsored by: Koha-Suomi Oy
Comment 5 ByWater Sandboxes 2020-02-07 14:01:04 UTC
Created attachment 98565 [details] [review]
Bug 4985: Edit a calendar entry for all branches

This patch adds functionality to calendar tool to
make edits or deletes to all branches at once.

To test:
-Apply patch
-Add holiday for a library and copy it to all or some
libraries
-Edit previosly made holiday (edit, delete etc.) and
check out "Copy changes to all libraries." checkbox
=> Changes are made to all libraries sharing changed
holiday

Sponsored by: Koha-Suomi Oy

Signed-off-by: Christofer <christofer.zorn@ajaxlibrary.ca>
Comment 6 Christofer Zorn 2020-02-07 14:02:40 UTC
Good morning!

Followed test plan and was able to create a single date holiday and apply it to all branches. Was then able to switch branches, make the edit and apply to all and it worked.

I'm not sure if this is intended to edit holidays over a range, but when I tried for a "closure week", the edit didn't apply and only changed the first day.

Thanks!
Comment 7 Katrin Fischer 2020-02-09 12:55:40 UTC
This works really well apart from the case Christopher found. I will attach a screenshot to show what I did. When I click on save, no holiday is deleted - neither on the currently selected branch, nor on others. Adding the range worked nicely!

Can you have a look?
Comment 8 Katrin Fischer 2020-02-09 12:56:27 UTC
Created attachment 98620 [details]
Deleting holidays on a date range not working
Comment 9 Emmi Takkinen 2020-02-10 13:55:22 UTC
Christofer, you mean "Edit this holiday"? Tried it with this patch applied (and on master) and edit indeed applies only to the first day. I'm not sure is this behaviour intended or is it something that needs to be fixed. 

However deleting on range worked as it should for me. I tried scenario from the screenshot and also retested with other deleting options without any problems.
Comment 10 Christofer Zorn 2020-02-10 13:59:22 UTC
Hello Emmi,

I was able to follow the test plan and it worked for single holidays. I created one for a week, then tried to edit it following the test plan again. It adjusted only the first day.

I'm not sure if the bug fix was intended to handle a range of holidays, or just edit the single one. Perhaps I should be deleting the range and reapplying it rather than trying to stretch this functionality.

I'm new to this signing off thing and may have overstepped what is expected of me. 

Thanks!

Christofer
Comment 11 Katrin Fischer 2020-02-10 14:47:09 UTC
> I'm new to this signing off thing and may have overstepped what is expected
> of me. 

Don't worry, you are doing really well - we expect people to question and comment :) Nothing bad about that!
Comment 12 Emmi Takkinen 2020-02-11 10:54:05 UTC
> I'm new to this signing off thing and may have overstepped what is expected of me.

Not at all and I'm sorry if my answer made you feel that way. Just wanted to make sure if I understood last part of your comment correctly :) Your comment was totally valid and in my opinion it pointed out possible need for enhancement. I say "in my opinion" since I have no idea if this edit is intended to affect only single days or not and should it be fixed.          

To make it clear, this patch just adds functionality to copy edits and deletes to all branches at once just the way holidays can be copied to all branches when they are created. So if the changes that are made for single branch are copied to every other branch this patch works as it's intended.  

I'm still unable to repeat problem with delete as in Katrins screenshot. I added and copied holidays on a range, then deleted them on a range and copied change to all libraries. Delete worked without problem also for the single, weekly, yearly and exception holidays. So Katrin, I have no idea why it's not working you :/
Comment 13 Emmi Takkinen 2020-03-03 07:22:16 UTC
I filed a separate bug report (bug 24782) regarding edit only applying to first day.
Comment 14 Katrin Fischer 2020-03-15 16:23:06 UTC
Moving back to signed off for now to help get more eyes on this.
Comment 15 Katrin Fischer 2020-03-16 21:08:17 UTC
I am sorry, I have given this another test run, but I still have issues:

- Add 2 holidays as 'repeated holidays on a range'
- Try to delete them using 'Delete the repeated holidays on a range.'
- Holidays are not deleted

With the patch applied, I am not able to delete them.

Emmi, can you pleas re-check?
Comment 16 Jonathan Druart 2020-03-17 10:10:40 UTC
Created attachment 100880 [details] [review]
Bug 4985: Edit a calendar entry for all branches

This patch adds functionality to calendar tool to
make edits or deletes to all branches at once.

To test:
-Apply patch
-Add holiday for a library and copy it to all or some
libraries
-Edit previosly made holiday (edit, delete etc.) and
check out "Copy changes to all libraries." checkbox
=> Changes are made to all libraries sharing changed
holiday

Sponsored by: Koha-Suomi Oy

Signed-off-by: Christofer <christofer.zorn@ajaxlibrary.ca>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 17 Jonathan Druart 2020-03-17 10:11:29 UTC
Oops, did not see Katrin's comment.
Comment 18 Jonathan Druart 2020-03-17 10:14:23 UTC
(In reply to Katrin Fischer from comment #15)
> I am sorry, I have given this another test run, but I still have issues:
> 
> - Add 2 holidays as 'repeated holidays on a range'
> - Try to delete them using 'Delete the repeated holidays on a range.'
> - Holidays are not deleted
> 
> With the patch applied, I am not able to delete them.
> 
> Emmi, can you pleas re-check?

Confirmed. Everything else works great!
Comment 19 Emmi Takkinen 2020-03-19 07:26:45 UTC
Created attachment 100994 [details] [review]
Bug 4985: Add missing parameters

Subroutine edit_holiday was missing parameter.

Sponsored-by: Koha-Suomi Oy
Comment 20 Emmi Takkinen 2020-03-19 07:55:08 UTC
Hopefully latest patch resolved this delete issue. Since deletes still (?!) work for me without it I'm unable to be totally sure. Still I'll change status back to Needs Signoff to get feedback.
Comment 21 Sally 2020-03-24 10:58:06 UTC
Hi Emmi,

I have the same issue as Katrin.  This scenario works:

- Add single holiday on a range and copy to all libraries
- Use 'delete this holiday' to delete one day from the range and copy to all libraries

As does this:

- Add repeated holiday on a range and copy to all libraries
- Use 'delete this holiday' to delete one day from the range and copy to all libraries

But this doesn't work:

- Add single holiday on a range and copy to all libraries
- Use 'delete the single holidays on a range' to delete multiple or all days from the range and copy to all libraries

Nor does this:  

- Add repeated holiday on a range and copy to all libraries
- Use 'delete the repeated holidays on a range' to delete multiple or all days from the range and copy to all libraries
Comment 22 Katrin Fischer 2020-03-26 21:47:19 UTC
Setting FQA because Sally's testing confirms there is still some glitch with the deletes.

I hope the problem can be fixed - this would make a nice addition!
Comment 23 Emmi Takkinen 2020-05-11 09:28:02 UTC
It seems this was a Plack issue. Tested editing with installation using Plack and was able to produce issues with on range deletes. Got several errors like: 

[WARN] Variable "$showoperation" is not available at /var/Koha/tools/exceptionHolidays.pl line 65.
Comment 24 Emmi Takkinen 2020-05-11 09:29:01 UTC
Created attachment 104664 [details] [review]
Bug 4985: Fix Plack scope errors

This patch 'ours' several variables to fix Plack scope errors
in exceptionHolidays.pl.

This also adds '@holidays_list' as parameter to 'edit_holidays'.

Sponsored-by: Koha-Suomi Oy
Comment 25 ByWater Sandboxes 2020-05-15 15:06:56 UTC
Created attachment 104949 [details] [review]
Bug 4985: Edit a calendar entry for all branches

This patch adds functionality to calendar tool to
make edits or deletes to all branches at once.

To test:
-Apply patch
-Add holiday for a library and copy it to all or some
libraries
-Edit previosly made holiday (edit, delete etc.) and
check out "Copy changes to all libraries." checkbox
=> Changes are made to all libraries sharing changed
holiday

Sponsored by: Koha-Suomi Oy

Signed-off-by: Christofer <christofer.zorn@ajaxlibrary.ca>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Comment 26 ByWater Sandboxes 2020-05-15 15:06:59 UTC
Created attachment 104950 [details] [review]
Bug 4985: Fix Plack scope errors

This patch 'ours' several variables to fix Plack scope errors
in exceptionHolidays.pl.

This also adds '@holidays_list' as parameter to 'edit_holidays'.

Sponsored-by: Koha-Suomi Oy

Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Comment 27 Katrin Fischer 2020-06-21 14:53:13 UTC
Created attachment 106129 [details] [review]
Bug 4985: Edit a calendar entry for all branches

This patch adds functionality to calendar tool to
make edits or deletes to all branches at once.

To test:
-Apply patch
-Add holiday for a library and copy it to all or some
libraries
-Edit previosly made holiday (edit, delete etc.) and
check out "Copy changes to all libraries." checkbox
=> Changes are made to all libraries sharing changed
holiday

Sponsored by: Koha-Suomi Oy

Signed-off-by: Christofer <christofer.zorn@ajaxlibrary.ca>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 28 Katrin Fischer 2020-06-21 14:53:16 UTC
Created attachment 106130 [details] [review]
Bug 4985: Fix Plack scope errors

This patch 'ours' several variables to fix Plack scope errors
in exceptionHolidays.pl.

This also adds '@holidays_list' as parameter to 'edit_holidays'.

Sponsored-by: Koha-Suomi Oy

Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 29 Katrin Fischer 2020-06-21 14:54:19 UTC
Glad to see you could figure out the remaining issue - thx for your perseverance!
Comment 30 Jonathan Druart 2020-06-24 13:31:28 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 31 Lucas Gass 2020-07-13 15:03:47 UTC
backported to 20.05.x for 20.05.02
Comment 32 Aleisha Amohia 2020-07-16 04:00:08 UTC
backported to 19.11.x for 19.11.08 because fixes the bug of being unable to delete holidays across multiple libraries
Comment 33 Victor Grousset/tuxayo 2020-07-26 15:00:16 UTC
Backported to 19.05.x branch for 19.05.14