Bug 19953

Summary: Add column for invoice in acquisition details tab
Product: Koha Reporter: Victor Grousset/tuxayo <victor>
Component: Staff interfaceAssignee: Victor Grousset/tuxayo <victor>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: gmcharlt, katrin.fischer, martin.renvoize, nick, sandboxes
Version: master   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Screenshot of proposed change
Bug 19953: Record page: acquisition details: add column for invoice
Bug 19953: Record page: acquisition details: add column for invoice
Bug 19953: Record page: acquisition details: add column for invoice
Bug 19953: Record page: acquisition details: add column for invoice

Description Victor Grousset/tuxayo 2018-01-11 16:21:12 UTC
Created attachment 70427 [details]
Screenshot of proposed change

See the attachment for how it would look.
Comment 1 Victor Grousset/tuxayo 2018-01-11 16:34:39 UTC
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
Comment 2 Biblibre Sandboxes 2018-03-12 12:28:10 UTC
Patch tested with a sandbox, by Séverine QUEUNE <severine.queune@bulac.fr>
Comment 3 Biblibre Sandboxes 2018-03-12 12:28:35 UTC
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>
Comment 4 Katrin Fischer 2018-03-12 15:34:02 UTC
Hi Victor, please remember to assign yourself the bugs you work on!
Comment 5 Victor Grousset/tuxayo 2018-03-12 15:55:39 UTC
Indeed, thanks :)
Comment 6 Katrin Fischer 2018-03-12 15:55:48 UTC
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?
Comment 7 Victor Grousset/tuxayo 2018-03-31 22:42:47 UTC
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>
Comment 8 Victor Grousset/tuxayo 2018-03-31 22:45:57 UTC
"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?)
Comment 9 Katrin Fischer 2018-04-03 21:29:27 UTC
Just make your changes as a follow-up patch (separate) next time. :)
Comment 10 Katrin Fischer 2018-04-03 21:32:51 UTC
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>
Comment 11 Victor Grousset/tuxayo 2018-04-04 06:59:20 UTC
> 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.
Comment 12 Katrin Fischer 2018-04-04 07:42:45 UTC
(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.
Comment 13 Victor Grousset/tuxayo 2018-04-04 07:46:16 UTC
> 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
Comment 14 Jonathan Druart 2018-04-04 19:08:02 UTC
Pushed to master for 18.05, thanks to everybody involved!
Comment 15 Nick Clemens 2018-04-11 13:14:24 UTC
Small enhancement, pushed to stable for 17.11.05