|
Description
Jonathan Druart
2019-12-03 19:34:23 UTC
Created attachment 96274 [details] [review] Bug 24157: New permission - reopen_closed_invoices New permission to reopen a closed invoice. Test plan: - Remove the new permission "reopen_closed_invoices" for a given patron, use it to log in into Koha - Create an invoice, close it => You are not able to reopen the invoice - Add the permission => You are able to reopen the invoice Sponsored-by: Galway-Mayo Institute of Technology Created attachment 96275 [details] [review] Bug 24157: New permission - edit_invoices Add a new permission to edit invoices Test plan: - Remove the new permission "edit_invoices" for a given patron, use it to log in into Koha - Create an invoice, edit it (click "detail") => You can see the detail of the invoice, but cannot edit it. It's a read-only view. - Add the permission => The form is back and you can modify the invoices and save the changes. Also, you are able to create adjustments. Sponsored-by: Galway-Mayo Institute of Technology Created attachment 96276 [details] [review] Bug 24157: Add new method Acq::Invoice::Adjustement->fund Moving to its own commit in case we want to cherry-pick or move it to its own bug report. Sponsored-by: Galway-Mayo Institute of Technology Created attachment 96277 [details] [review] Bug 24157: New permission - delete_invoices Add a new permission to delete invoices Test plan: - Remove the new permission "delete_invoices" for a given patron, use it to log in into Koha - Create an invoice, try to delete it => There is no way to delete it - Add the permission => Now you can delete the invoice Sponsored-by: Galway-Mayo Institute of Technology Created attachment 96278 [details] [review] Bug 24157: New permission - delete_baskets Add a new permission to delete acquisition baskets Test plan: - Remove the new permission "delete_basket" for a given patron, use it to log in into Koha - Create basket, try to delete it => There is no way to delete it - Add the permission => Now you can delete the basket Sponsored-by: Galway-Mayo Institute of Technology Created attachment 98651 [details] [review] Bug 24157: New permission - reopen_closed_invoices New permission to reopen a closed invoice. Test plan: - Remove the new permission "reopen_closed_invoices" for a given patron, use it to log in into Koha - Create an invoice, close it => You are not able to reopen the invoice - Add the permission => You are able to reopen the invoice Sponsored-by: Galway-Mayo Institute of Technology Created attachment 98652 [details] [review] Bug 24157: New permission - edit_invoices Add a new permission to edit invoices Test plan: - Remove the new permission "edit_invoices" for a given patron, use it to log in into Koha - Create an invoice, edit it (click "detail") => You can see the detail of the invoice, but cannot edit it. It's a read-only view. - Add the permission => The form is back and you can modify the invoices and save the changes. Also, you are able to create adjustments. Sponsored-by: Galway-Mayo Institute of Technology Created attachment 98653 [details] [review] Bug 24157: Add new method Acq::Invoice::Adjustement->fund Moving to its own commit in case we want to cherry-pick or move it to its own bug report. Sponsored-by: Galway-Mayo Institute of Technology Created attachment 98654 [details] [review] Bug 24157: New permission - delete_invoices Add a new permission to delete invoices Test plan: - Remove the new permission "delete_invoices" for a given patron, use it to log in into Koha - Create an invoice, try to delete it => There is no way to delete it - Add the permission => Now you can delete the invoice Sponsored-by: Galway-Mayo Institute of Technology Created attachment 98655 [details] [review] Bug 24157: New permission - delete_baskets Add a new permission to delete acquisition baskets Test plan: - Remove the new permission "delete_basket" for a given patron, use it to log in into Koha - Create basket, try to delete it => There is no way to delete it - Add the permission => Now you can delete the basket Sponsored-by: Galway-Mayo Institute of Technology Rebased to allow the application to sandboxes for testing. Created attachment 100283 [details] [review] Bug 24157: New permission - reopen_closed_invoices New permission to reopen a closed invoice. Test plan: - Remove the new permission "reopen_closed_invoices" for a given patron, use it to log in into Koha - Create an invoice, close it => You are not able to reopen the invoice - Add the permission => You are able to reopen the invoice Sponsored-by: Galway-Mayo Institute of Technology Signed-off-by: Owen Leonard <oleonard@myacpl.org> Created attachment 100284 [details] [review] Bug 24157: New permission - edit_invoices Add a new permission to edit invoices Test plan: - Remove the new permission "edit_invoices" for a given patron, use it to log in into Koha - Create an invoice, edit it (click "detail") => You can see the detail of the invoice, but cannot edit it. It's a read-only view. - Add the permission => The form is back and you can modify the invoices and save the changes. Also, you are able to create adjustments. Sponsored-by: Galway-Mayo Institute of Technology Signed-off-by: Owen Leonard <oleonard@myacpl.org> Created attachment 100285 [details] [review] Bug 24157: Add new method Acq::Invoice::Adjustement->fund Moving to its own commit in case we want to cherry-pick or move it to its own bug report. Sponsored-by: Galway-Mayo Institute of Technology Signed-off-by: Owen Leonard <oleonard@myacpl.org> Created attachment 100286 [details] [review] Bug 24157: New permission - delete_invoices Add a new permission to delete invoices Test plan: - Remove the new permission "delete_invoices" for a given patron, use it to log in into Koha - Create an invoice, try to delete it => There is no way to delete it - Add the permission => Now you can delete the invoice Sponsored-by: Galway-Mayo Institute of Technology Signed-off-by: Owen Leonard <oleonard@myacpl.org> Created attachment 100287 [details] [review] Bug 24157: New permission - delete_baskets Add a new permission to delete acquisition baskets Test plan: - Remove the new permission "delete_basket" for a given patron, use it to log in into Koha - Create basket, try to delete it => There is no way to delete it - Add the permission => Now you can delete the basket Sponsored-by: Galway-Mayo Institute of Technology Signed-off-by: Owen Leonard <oleonard@myacpl.org> Created attachment 100288 [details] [review] Bug 24157: (follow-up) Conditionally show controls to reopen and close invoice This follow-up makes minor changes to the template so that the checkbox for reopening or closing an invoice are not displayed when the user doens't have the correct permissions. To test, apply the patch and try the process of viewing or editing an invoice as a user with these various permissions: - edit_invoices ON - edit_invoices OFF - reopen_closed_invoices ON - reopen_closed_invoices OFF Followup looks good to me.. Setting to signedoff :) I think the third commit "Add new method Acq::Invoice::Adjustement->fund" is not the right place here. Right ? I don't see any use of it and no old code removed. Blocker: I still see the "Merge selected invoices" button in invoices list if i don't have the "edit_invoices" permission. So i can select invoices, click on it, but i get an empty invoice form and merge is not done. So IMO there is two options: 1) i can see the merge option and the merge should be done, 2) i can't see the merge option and the checkboxes if i don't have the "edit_invoices" permission. Questioning: should we assume that merging is editing and deleting ? Comment on attachment 100288 [details] [review] Bug 24157: (follow-up) Conditionally show controls to reopen and close invoice Review of attachment 100288 [details] [review]: ----------------------------------------------------------------- Suggested extension of this to conditionally show controls to delete invoices too, so that the checkbox for deleting an invoice is not displayed when the user doesn't have the correct permissions. Created attachment 107132 [details] [review] Bug 24157: New permission - reopen_closed_invoices New permission to reopen a closed invoice. Test plan: - Remove the new permission "reopen_closed_invoices" for a given patron, use it to log in into Koha - Create an invoice, close it => You are not able to reopen the invoice - Add the permission => You are able to reopen the invoice Sponsored-by: Galway-Mayo Institute of Technology Signed-off-by: Owen Leonard <oleonard@myacpl.org> Created attachment 107133 [details] [review] Bug 24157: New permission - edit_invoices Add a new permission to edit invoices Test plan: - Remove the new permission "edit_invoices" for a given patron, use it to log in into Koha - Create an invoice, edit it (click "detail") => You can see the detail of the invoice, but cannot edit it. It's a read-only view. - Add the permission => The form is back and you can modify the invoices and save the changes. Also, you are able to create adjustments. Sponsored-by: Galway-Mayo Institute of Technology Signed-off-by: Owen Leonard <oleonard@myacpl.org> Created attachment 107134 [details] [review] Bug 24157: Add new method Acq::Invoice::Adjustement->fund Moving to its own commit in case we want to cherry-pick or move it to its own bug report. Sponsored-by: Galway-Mayo Institute of Technology Signed-off-by: Owen Leonard <oleonard@myacpl.org> Created attachment 107135 [details] [review] Bug 24157: New permission - delete_invoices Add a new permission to delete invoices Test plan: - Remove the new permission "delete_invoices" for a given patron, use it to log in into Koha - Create an invoice, try to delete it => There is no way to delete it - Add the permission => Now you can delete the invoice Sponsored-by: Galway-Mayo Institute of Technology Signed-off-by: Owen Leonard <oleonard@myacpl.org> Created attachment 107136 [details] [review] Bug 24157: New permission - delete_baskets Add a new permission to delete acquisition baskets Test plan: - Remove the new permission "delete_basket" for a given patron, use it to log in into Koha - Create basket, try to delete it => There is no way to delete it - Add the permission => Now you can delete the basket Sponsored-by: Galway-Mayo Institute of Technology Signed-off-by: Owen Leonard <oleonard@myacpl.org> Created attachment 107137 [details] [review] Bug 24157: (follow-up) Conditionally show controls to reopen and close invoice This follow-up makes minor changes to the template so that the checkbox for reopening or closing an invoice are not displayed when the user doens't have the correct permissions. To test, apply the patch and try the process of viewing or editing an invoice as a user with these various permissions: - edit_invoices ON - edit_invoices OFF - reopen_closed_invoices ON - reopen_closed_invoices OFF Created attachment 107138 [details] [review] Bug 24157: New permission - merge_invoices Add a new permission to merge invoices Test plan: - Remove the new permission "merge_invoices" for a given patron, use it to log in into Koha - Create 2 invoices, try to merge them => There is no way to merge it - Add the permission => Now you can merge the invoices Sponsored-by: Galway-Mayo Institute of Technology Created attachment 107139 [details] [review] Bug 24157: (follow-up) New permission - delete_invoices Hide the "Delete" invoice link if necessary on /cgi-bin/koha/acqui/invoice.pl?invoiceid=X (In reply to Alex Arnaud from comment #19) > I think the third commit "Add new method Acq::Invoice::Adjustement->fund" is > not the right place here. Right ? > I don't see any use of it and no old code removed. Hi Alex, It is used in "Bug 24157: New permission - edit_invoices" [% adjustement.fund.budget_name | html %] (In reply to Alex Arnaud from comment #20) > Blocker: > > I still see the "Merge selected invoices" button in invoices list if i don't > have the "edit_invoices" permission. > So i can select invoices, click on it, but i get an empty invoice form and > merge is not done. > > So IMO there is two options: > > 1) i can see the merge option and the merge should be done, > 2) i can't see the merge option and the checkboxes if i don't have the > "edit_invoices" permission. > > Questioning: should we assume that merging is editing and deleting ? I created a new merge_invoices permissions. (In reply to Daphne Hoolahan from comment #21) > Comment on attachment 100288 [details] [review] [review] > Bug 24157: (follow-up) Conditionally show controls to reopen and close > invoice > > Review of attachment 100288 [details] [review] [review]: > ----------------------------------------------------------------- > > Suggested extension of this to conditionally show controls to delete > invoices too, so that the checkbox > for deleting an invoice is not displayed when the user doesn't have the > correct permissions. Hi Daphne, I found a "Delete" link on the invoice detail page, but I did not find one on a view where a checkbox is visible. Can you detail a bit more please? I have the "Merge invoices" permission, but not "Edit invoices" => I see the merge button but it doesn't work. I get redirect to an empty invoice form and merge is not done. I have the "Reopen closed invoices" but not "Edit invoices" => I see the reopen button ut it doesn't work. I get redirect to an empty invoice form and reopening is not done. Created attachment 107309 [details] [review] Bug 24157: Handle the case where logged in user does not have edit_invoices This patch make possible the reopening and merging of invoices even if the logged in user does not have the edit_invoices permission I don't think it really makes sense but at least it's now possible. This is a bit an edge case and I don't think it really makes sense. But it should be handled correctly now. Thanks Alex! (In reply to Jonathan Druart from comment #32) > Created attachment 107309 [details] [review] [review] > Bug 24157: Handle the case where logged in user does not have edit_invoices > > This patch make possible the reopening and merging of invoices even if > the logged in user does not have the edit_invoices permission > > I don't think it really makes sense but at least it's now possible. It makes sense IMO. If you see the button or link, you should be able to do it. I would have agreed with hidding this buttons also. Created attachment 107346 [details] [review] Bug 24157: New permission - reopen_closed_invoices New permission to reopen a closed invoice. Test plan: - Remove the new permission "reopen_closed_invoices" for a given patron, use it to log in into Koha - Create an invoice, close it => You are not able to reopen the invoice - Add the permission => You are able to reopen the invoice Sponsored-by: Galway-Mayo Institute of Technology Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com> Created attachment 107347 [details] [review] Bug 24157: New permission - edit_invoices Add a new permission to edit invoices Test plan: - Remove the new permission "edit_invoices" for a given patron, use it to log in into Koha - Create an invoice, edit it (click "detail") => You can see the detail of the invoice, but cannot edit it. It's a read-only view. - Add the permission => The form is back and you can modify the invoices and save the changes. Also, you are able to create adjustments. Sponsored-by: Galway-Mayo Institute of Technology Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com> Created attachment 107348 [details] [review] Bug 24157: Add new method Acq::Invoice::Adjustement->fund Moving to its own commit in case we want to cherry-pick or move it to its own bug report. Sponsored-by: Galway-Mayo Institute of Technology Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com> Created attachment 107349 [details] [review] Bug 24157: New permission - delete_invoices Add a new permission to delete invoices Test plan: - Remove the new permission "delete_invoices" for a given patron, use it to log in into Koha - Create an invoice, try to delete it => There is no way to delete it - Add the permission => Now you can delete the invoice Sponsored-by: Galway-Mayo Institute of Technology Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com> Created attachment 107350 [details] [review] Bug 24157: New permission - delete_baskets Add a new permission to delete acquisition baskets Test plan: - Remove the new permission "delete_basket" for a given patron, use it to log in into Koha - Create basket, try to delete it => There is no way to delete it - Add the permission => Now you can delete the basket Sponsored-by: Galway-Mayo Institute of Technology Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com> Created attachment 107351 [details] [review] Bug 24157: (follow-up) Conditionally show controls to reopen and close invoice This follow-up makes minor changes to the template so that the checkbox for reopening or closing an invoice are not displayed when the user doens't have the correct permissions. To test, apply the patch and try the process of viewing or editing an invoice as a user with these various permissions: - edit_invoices ON - edit_invoices OFF - reopen_closed_invoices ON - reopen_closed_invoices OFF Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com> Created attachment 107352 [details] [review] Bug 24157: New permission - merge_invoices Add a new permission to merge invoices Test plan: - Remove the new permission "merge_invoices" for a given patron, use it to log in into Koha - Create 2 invoices, try to merge them => There is no way to merge it - Add the permission => Now you can merge the invoices Sponsored-by: Galway-Mayo Institute of Technology Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com> Created attachment 107353 [details] [review] Bug 24157: (follow-up) New permission - delete_invoices Hide the "Delete" invoice link if necessary on /cgi-bin/koha/acqui/invoice.pl?invoiceid=X Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com> Created attachment 107354 [details] [review] Bug 24157: Handle the case where logged in user does not have edit_invoices This patch make possible the reopening and merging of invoices even if the logged in user does not have the edit_invoices permission I don't think it really makes sense but at least it's now possible. Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com> Pushed to master for 20.11, thanks to everybody involved! enhancement will not be backported to 20.05.x series *** Bug 20375 has been marked as a duplicate of this bug. *** |