|
Description
Owen Leonard
2010-07-12 15:59:04 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. Deleting and editing holidays for all branches at once is still not possible in current versions of Koha. Still valid, looks like. 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 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> 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! 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? Created attachment 98620 [details]
Deleting holidays on a date range not working
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. 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
> 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!
> 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 :/
I filed a separate bug report (bug 24782) regarding edit only applying to first day. Moving back to signed off for now to help get more eyes on this. 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? 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> Oops, did not see Katrin's comment. (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! Created attachment 100994 [details] [review] Bug 4985: Add missing parameters Subroutine edit_holiday was missing parameter. Sponsored-by: Koha-Suomi Oy 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. 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 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! 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. 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 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> 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> 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> 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> Glad to see you could figure out the remaining issue - thx for your perseverance! Pushed to master for 20.11, thanks to everybody involved! backported to 20.05.x for 20.05.02 backported to 19.11.x for 19.11.08 because fixes the bug of being unable to delete holidays across multiple libraries Backported to 19.05.x branch for 19.05.14 |