To reproduce: 1- Connect as superlibrarian. 2- Create a rule for all libraries in tools/letter.pl 3- Copy this letter on a precise library, different from the one you are registred on 4- Try to delete the letter => it wants to delete the one from all libraries
Created attachment 63484 [details] [review] Bug 18613: Correct behaviour Remove a part of the code which automatically redirects on the library of logged-in user ( not ergonomic behaviour ). TEST PLAN: 1- Connect as superlibrarian. 2- Create a rule for all libraries in tools/letter.pl 3- Copy this letter on a precise library, different from the one you are registred on 4- Try to delete the letter => it wants to delete the one from all libraries 5- Apply Patch 6- Try again to delete the letter => it proposes you the good letter
Created attachment 63485 [details] [review] Bug 18613: Correct behaviour Remove a part of the code which automatically redirects on the library of logged-in user ( not ergonomic behaviour ). TEST PLAN: 1- Connect as superlibrarian. 2- Create a rule for all libraries in tools/letter.pl 3- Copy this letter on a precise library, different from the one you are registred on 4- Try to delete the letter => it wants to delete the one from all libraries 5- Apply Patch 6- Try again to delete the letter => it proposes you the good letter
Created attachment 63486 [details] [review] Bug 18613: Correct behaviour Remove a part of the code which automatically redirects on the library of logged-in user ( not ergonomic behaviour ). TEST PLAN: 1- Connect as superlibrarian. 2- Create a rule for all libraries in tools/letter.pl 3- Copy this letter on a precise library, different from the one you are registred on 4- Try to delete the letter => it wants to delete the one from all libraries 5- Apply Patch 6- Try again to delete the letter => it proposes you the good letter
Created attachment 63578 [details] [review] Bug 18613: Correct the behaviour TEST PLAN (reproducing bug) 1 - Set 'IndependantBranches' to enable 2 - Log in as Superlibrarian. 3 - Create 3 rules - 1 for all librairies (1) - 1 for the library of the superlibrarian (2) - 1 for another library (3) 4 - Try to delete them and see them and see that (3) isn't the good one (checking patch) 5 - Apply patch 6 - Try again 3- 7 - Connect as not superlibrarian 8 - Try again 3-
Created attachment 64616 [details] [review] Bug 18613: Correct the behaviour TEST PLAN (reproducing bug) 1 - Set 'IndependantBranches' to enable 2 - Log in as Superlibrarian. 3 - Create 3 rules - 1 for all librairies (1) - 1 for the library of the superlibrarian (2) - 1 for another library (3) 4 - Try to delete them and see them and see that (3) isn't the good one (checking patch) 5 - Apply patch 6 - Try again 3- 7 - Connect as not superlibrarian 8 - Try again 3- Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Comment on attachment 64616 [details] [review] Bug 18613: Correct the behaviour Review of attachment 64616 [details] [review]: ----------------------------------------------------------------- ::: C4/Letters.pm @@ +204,5 @@ > > > + my $only_my_library = C4::Context->only_my_library; > + if ( $only_my_library and $branchcode ) { > + $branchcode = $only_my_library; C4::Context->only_my_library is supposed to return a boolean value. You should use C4::Context::mybranch() here
Also, if you could change the patch subject to a more meaningful one, that would be great! :)
Created attachment 64912 [details] [review] Bug 18613: Remove letter rule correctly as superlibrarian TEST PLAN (reproducing bug) 1 - Set 'IndependantBranches' to enable 2 - Log in as Superlibrarian. 3 - Create 3 rules - 1 for all librairies (1) - 1 for the library of the superlibrarian (2) - 1 for another library (3) 4 - Try to delete them and see them and see that (3) isn't the good one (checking patch) 5 - Apply patch 6 - Try again 3- 7 - Connect as not superlibrarian 8 - Try again 3- Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Created attachment 64914 [details] [review] Bug 18613: Remove letter rule correctly as superlibrarian TEST PLAN (reproducing bug) 1 - Set 'IndependantBranches' to enable 2 - Log in as Superlibrarian. 3 - Create 3 rules - 1 for all librairies (1) - 1 for the library of the superlibrarian (2) - 1 for another library (3) 4 - Try to delete them and see them and see that (3) isn't the good one (checking patch) 5 - Apply patch 6 - Try again 3- 7 - Connect as not superlibrarian 8 - Try again 3- Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Please provide tests.
Created attachment 64987 [details] [review] Bug 18613: Add unit tests for getletter
Pushed to master for 17.11, thanks to everybody involved!
Pushed to 17.05.x, will be in 17.05.02
These patches have been pushed to 16.11.x and will be in 16.11.10. Tests pass, but there is an additional warn about userenv: kohadev-koha@kohadevbox:/home/vagrant/kohaclone$ prove t/db_dependent/Letters.t t/db_dependent/Letters.t .. 1/74 C4::Context->userenv not defined! at /home/vagrant/kohaclone/C4/Letters.pm line 206. Fake sendmail at t/db_dependent/Letters.t line 32. t/db_dependent/Letters.t .. ok All tests successful. Files=1, Tests=74, 1 wallclock secs ( 0.02 usr 0.00 sys + 1.17 cusr 0.36 csys = 1.55 CPU) Result: PASS