Created attachment 70427 [details] Screenshot of proposed change See the attachment for how it would look.
Created attachment 70431 [details] [review] Bug 19953: Record page: acquisition details: add column for invoice Test plan: 1. create a vendor with minimal info 2. create a basket with minimal info 3. add a item to the basket 4. close the basket 5. receive a shipment 6. on the "receipt summary" click on "receive" for the wanted item (it should be the only one) 7. items → receive? → tick this checkbox 8. save 9. click on "finish receiving" 10. Go to the record → Acquisition details 11. Apply this patch 12. Refresh the page 13. You should see the new column
Patch tested with a sandbox, by Séverine QUEUNE <severine.queune@bulac.fr>
Created attachment 72676 [details] [review] Bug 19953: Record page: acquisition details: add column for invoice Test plan: 1. create a vendor with minimal info 2. create a basket with minimal info 3. add a item to the basket 4. close the basket 5. receive a shipment 6. on the "receipt summary" click on "receive" for the wanted item (it should be the only one) 7. items → receive? → tick this checkbox 8. save 9. click on "finish receiving" 10. Go to the record → Acquisition details 11. Apply this patch 12. Refresh the page 13. You should see the new column Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Hi Victor, please remember to assign yourself the bugs you work on!
Indeed, thanks :)
Comment on attachment 72676 [details] [review] Bug 19953: Record page: acquisition details: add column for invoice Review of attachment 72676 [details] [review]: ----------------------------------------------------------------- ::: koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt @@ +695,4 @@ > [% END %] > </td> > <td> > + [% IF (order.invoiceid) %] I think it would be nicer to check the permission here like we do on the next column. Strangely it seems we don't have a separate permission for managing invoices in acq, the templates check for "any permission in the acquisitions module". Could you mirror that here?
Created attachment 73515 [details] [review] Bug 19953: Record page: acquisition details: add column for invoice Test plan: 1. create a vendor with minimal info 2. create a basket with minimal info 3. add a item to the basket 4. close the basket 5. receive a shipment 6. on the "receipt summary" click on "receive" for the wanted item (it should be the only one) 7. items → receive? → tick this checkbox 8. save 9. click on "finish receiving" 10. go to the record → Acquisition details 11. apply this patch 12. refresh the page 13. you should see the new column Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
"any permission in the acquisitions module" So I used CAN_user_acquisition Here is my amend: diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt index f130644..9b1eb1a 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt @@ -704,9 +704,13 @@ </td> <td> [% IF (order.invoiceid) %] - <a href="/cgi-bin/koha/acqui/invoice.pl?invoiceid=[% order.invoiceid %]" - title="Invoice detail page"> - [% order.invoicenumber %]</a> + [% IF CAN_user_acquisition %] + <a href="/cgi-bin/koha/acqui/invoice.pl?invoiceid=[% order.invoiceid %]" + title="Invoice detail page"> + [% order.invoicenumber %]</a> + [% ELSE %] + [% order.invoicenumber %] + [% END %] ########################################################## note: I haven't invalidated the sign off. (maybe I should?)
Just make your changes as a follow-up patch (separate) next time. :)
Created attachment 73593 [details] [review] Bug 19953: Record page: acquisition details: add column for invoice Test plan: 1. create a vendor with minimal info 2. create a basket with minimal info 3. add a item to the basket 4. close the basket 5. receive a shipment 6. on the "receipt summary" click on "receive" for the wanted item (it should be the only one) 7. items → receive? → tick this checkbox 8. save 9. click on "finish receiving" 10. go to the record → Acquisition details 11. apply this patch 12. refresh the page 13. you should see the new column Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
> Just make your changes as a follow-up patch (separate) next time. :) Got it, two patches wouldn't have looked clean in the history but if two patches are an issue then the maintainer could just squash them.
(In reply to Victor Grousset/tuxayo from comment #11) > > Just make your changes as a follow-up patch (separate) next time. :) > > Got it, two patches wouldn't have looked clean in the history but if two > patches are an issue then the maintainer could just squash them. Why would they not have looked clean? It's how we roll :) The general rule is that once a patch has been signed off, you shoud not change the signed patches but add changes as seprate patches. This way the sign-offer and QA can verify the changes and the history is clear.
> Why would they not have looked clean? It would be like 2/3 of the work in one patch and 1/3 in the other. > It's how we roll :) > The general rule is that once a patch has been signed off, you shoud not change the signed patches but add changes as seprate patches. This way the sign-offer and QA can verify the changes and the history is clear. Thanks for the clarification, it's simpler like that :D
Pushed to master for 18.05, thanks to everybody involved!
Small enhancement, pushed to stable for 17.11.05