Bug 16939 - Making all 'add to basket' actions buttons
Summary: Making all 'add to basket' actions buttons
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Aleisha Amohia
QA Contact: Jonathan Druart
URL:
Keywords: Academy
Depends on:
Blocks:
 
Reported: 2016-07-19 05:05 UTC by Aleisha Amohia
Modified: 2020-01-06 20:14 UTC (History)
6 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
This enhancement improves the consistency within our acquisitions module.
Version(s) released in:
Circulation function:


Attachments
Bug 16939: Making all 'add to basket' actions buttons (12.27 KB, patch)
2016-07-19 05:15 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 16939: Making all 'add to basket' actions buttons (12.32 KB, patch)
2016-07-20 21:17 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 16939: Making all 'add to basket' actions buttons (12.70 KB, patch)
2016-07-20 23:16 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 16939: Making all 'add to basket' actions buttons (12.76 KB, patch)
2016-07-21 13:57 UTC, Nicole C. Engard
Details | Diff | Splinter Review
Bug 16939: Making all 'add to basket' actions buttons (12.78 KB, patch)
2016-07-26 04:38 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 16939: Making all 'add to basket' actions buttons (12.92 KB, patch)
2016-07-28 20:46 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 16939: Making all 'add to basket' actions buttons (13.38 KB, patch)
2016-08-16 03:44 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 16939: Making all 'add to basket' actions buttons (13.30 KB, patch)
2016-12-02 02:53 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Strange artifact when ordering from external source (9.58 KB, image/png)
2017-03-17 09:56 UTC, Marc Véron
Details
Bug 16939: Making all 'add to basket' actions buttons (13.48 KB, patch)
2017-03-28 04:11 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 16939: Making all 'add to basket' actions buttons (13.65 KB, patch)
2017-03-28 14:08 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 16939: (follow-up) Correction for orderreceive.tt (1.53 KB, patch)
2018-05-29 18:16 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 16939: Making all 'add to basket' actions buttons (16.31 KB, patch)
2018-10-18 03:23 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 16939: (follow-up) Correction for orderreceive.tt (1.53 KB, patch)
2018-10-18 03:23 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 16939: (follow-up) Adding missing bracket (1.18 KB, patch)
2018-10-23 00:17 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 16939: Making all 'add to basket' actions buttons (16.53 KB, patch)
2018-12-07 16:26 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 16939: (follow-up) Correction for orderreceive.tt (1.58 KB, patch)
2018-12-07 16:26 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 16939: (follow-up) Adding missing bracket (1.23 KB, patch)
2018-12-07 16:26 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 16939: Making all 'add to basket' actions buttons (16.64 KB, patch)
2018-12-12 16:02 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16939: (follow-up) Correction for orderreceive.tt (1.66 KB, patch)
2018-12-12 16:02 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16939: (follow-up) Adding missing bracket (1.31 KB, patch)
2018-12-12 16:02 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Aleisha Amohia 2016-07-19 05:05:00 UTC

    
Comment 1 Aleisha Amohia 2016-07-19 05:15:55 UTC Comment hidden (obsolete)
Comment 2 Héctor Eduardo Castro Avalos 2016-07-19 21:50:59 UTC
Comment on attachment 53484 [details] [review]
Bug 16939: Making all 'add to basket' actions buttons


>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/addorderiso2709.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/addorderiso2709.tt
>@@ -442,7 +442,7 @@
>                         </td>
>                         <td><span title="[% batch_lis.staged_date %]">[% batch_lis.staged_date | $KohaDates with_hours => 1 %]</span></td>
>                         <td>[% batch_lis.num_records %]</td>
>-                        <td><a href="[% batch_lis.scriptname %]?import_batch_id=[% batch_lis.import_batch_id %]&amp;basketno=[% basketno %]&amp;booksellerid=[% booksellerid %]">Add orders</a></td>
>+                        <td><a href="[% batch_lis.scriptname %]?import_batch_id=[% batch_lis.import_batch_id %]&amp;basketno=[% basketno %]&amp;booksellerid=[% booksellerid %]" class="btn btn-mini"><i clas="fa fa-plus"></i> Add orders</a></td>
>                       </tr>

There is a typo in "clas" in "fa fa-plus"
Comment 3 Héctor Eduardo Castro Avalos 2016-07-19 21:52:45 UTC
Looks great, just fix the "clas" in Add orders -> addorderiso2709.tt
Comment 4 Aleisha Amohia 2016-07-20 21:17:17 UTC Comment hidden (obsolete)
Comment 5 Aleisha Amohia 2016-07-20 23:16:15 UTC Comment hidden (obsolete)
Comment 6 Nicole C. Engard 2016-07-21 13:57:40 UTC Comment hidden (obsolete)
Comment 7 Jonathan Druart 2016-07-25 16:24:14 UTC
You are using quote inside a quoted string, I get a JS error when creating a new order:

SyntaxError: missing ; before statement
result += "<td class="actions">" + edit_link + " " + del_link + "</td>";
Comment 8 Aleisha Amohia 2016-07-26 04:38:54 UTC Comment hidden (obsolete)
Comment 9 Owen Leonard 2016-07-27 14:16:36 UTC
Please update circ/reserveratios.tt so that the "plus" icon is not shown when there is no link.
Comment 10 Aleisha Amohia 2016-07-28 20:46:43 UTC Comment hidden (obsolete)
Comment 11 Owen Leonard 2016-08-08 14:46:53 UTC
(In reply to Aleisha Amohia from comment #10)
> Edit: circ/reserveratios.tt updated so that the plus icon doesn't show
>       if there is no link

I'm still getting the plus sign when there is no link. I think the template logic is off somewhere. If it helps, feel free to break up and re-indent that section since right now it's all on one line.
Comment 12 Aleisha Amohia 2016-08-16 03:44:35 UTC Comment hidden (obsolete)
Comment 13 Aleisha Amohia 2016-08-16 03:45:46 UTC
(In reply to Owen Leonard from comment #11)
> I'm still getting the plus sign when there is no link. I think the template
> logic is off somewhere. If it helps, feel free to break up and re-indent
> that section since right now it's all on one line.

I rearranged some stuff, so hopefully this new patch works for you!
Comment 14 Marc Véron 2016-10-03 14:16:46 UTC
Wanted to test, but patch does not apply, sorry.
Comment 15 Aleisha Amohia 2016-12-02 02:53:14 UTC Comment hidden (obsolete)
Comment 16 Marc Véron 2017-03-17 09:56:46 UTC
Created attachment 61206 [details]
Strange artifact when ordering from external source

Testing
11) 'Order' should now show as a button with appropriate icons. MARC and Card preview have been moved into a dropdown menu - confirm this works as expected. Links have not been changed when clicking on a row

Result
Clicking on adropdown menu shows a strange artifact. 
Tested with Firefox 52.0 (32-Bit) Win

Other points are OK
Comment 17 Owen Leonard 2017-03-18 18:28:38 UTC
Please update the classes on the action buttons. Now that we have Bootstrap 3 the button classes should be "btn btn-default btn-xs"

https://wiki.koha-community.org/wiki/Interface_patterns#Action_buttons_in_tables
Comment 18 Aleisha Amohia 2017-03-28 04:11:44 UTC Comment hidden (obsolete)
Comment 19 Marc Véron 2017-03-28 14:08:15 UTC Comment hidden (obsolete)
Comment 20 Jonathan Druart 2017-03-29 19:37:41 UTC
Aleisha,
Take a look at orderreceive.tt, it also uses additem.js when AcqCreateItem = "on receiving".
Comment 21 Aleisha Amohia 2017-06-27 03:43:04 UTC
(In reply to Jonathan Druart from comment #20)
> Aleisha,
> Take a look at orderreceive.tt, it also uses additem.js when AcqCreateItem =
> "on receiving".

Sorry Jonathan, can you explain the relevance to this patch? Been a while since I looked at this but I don't think that is within the scope of my bug.
Comment 22 Jonathan Druart 2017-07-05 23:27:47 UTC
You are modifying a js file that is used from different templates.
You will introduced side-effects with these changes (when receiving an item if AcqCreateItem = "on receiving").
Comment 23 Owen Leonard 2018-05-29 18:16:40 UTC Comment hidden (obsolete)
Comment 24 Aleisha Amohia 2018-10-18 03:23:45 UTC Comment hidden (obsolete)
Comment 25 Aleisha Amohia 2018-10-18 03:23:54 UTC Comment hidden (obsolete)
Comment 26 Aleisha Amohia 2018-10-18 04:13:16 UTC
This has now been rebased but note, it fails with ElasticSearch enabled
Comment 27 Katrin Fischer 2018-10-18 10:44:41 UTC
Does it fail with Elasitc with and without this patch?
Comment 28 Owen Leonard 2018-10-18 18:24:07 UTC
The QA tool fails acqui/z3950_search.tt:  parse error - /home/vagrant/kohaclone/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/z3950_search.tt line 124: unexpected end of directive
  [% IF ( Koha.Preference('AdditionalFieldsInZ3950ResultSearch') != '' %]
Comment 29 Aleisha Amohia 2018-10-23 00:17:20 UTC Comment hidden (obsolete)
Comment 30 Aleisha Amohia 2018-10-23 00:20:03 UTC
(In reply to Katrin Fischer from comment #27)
> Does it fail with Elasitc with and without this patch?

This is a good point. I've retested and can confirm that we cannot add to a basket with elasticsearch enabled, with or without the patch.
Comment 31 Aleisha Amohia 2018-10-23 00:20:13 UTC
(In reply to Owen Leonard from comment #28)
> The QA tool fails acqui/z3950_search.tt:  parse error -
> /home/vagrant/kohaclone/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/
> z3950_search.tt line 124: unexpected end of directive
>   [% IF ( Koha.Preference('AdditionalFieldsInZ3950ResultSearch') != '' %]

Fixed, ready to test
Comment 32 Katrin Fischer 2018-10-23 05:44:24 UTC
(In reply to Aleisha Amohia from comment #30)
> (In reply to Katrin Fischer from comment #27)
> > Does it fail with Elasitc with and without this patch?
> 
> This is a good point. I've retested and can confirm that we cannot add to a
> basket with elasticsearch enabled, with or without the patch.

Hi Aleisha, could you maybe file a bug with your findings? This seems major.
Comment 33 Aleisha Amohia 2018-10-25 21:32:02 UTC
(In reply to Katrin Fischer from comment #32)
> (In reply to Aleisha Amohia from comment #30)
> > (In reply to Katrin Fischer from comment #27)
> > > Does it fail with Elasitc with and without this patch?
> > 
> > This is a good point. I've retested and can confirm that we cannot add to a
> > basket with elasticsearch enabled, with or without the patch.
> 
> Hi Aleisha, could you maybe file a bug with your findings? This seems major.

Done - see Bug 21675.

Would like to see this one get through anyhow.
Comment 34 Owen Leonard 2018-12-07 16:26:49 UTC
Created attachment 82986 [details] [review]
Bug 16939: Making all 'add to basket' actions buttons

Edit: Fixing 'clas' typo in addorderiso2709.tt
Edit: Adding actions class to td
Edit: Fixing js error
Edit: circ/reserveratios.tt updated so that the plus icon doesn't show
      if there is no link
Edit: fixing logic in circ/reserveratios.tt
Edit: merge conflicts
Edit: updating buttons to bootstrap 3, which i believe fixes the problem
      in comment 16

To test:

1) Go to Acqui -> find a vendor -> view a basket
2) Click add to basket -> fill in 'for an existing record' and hit search
3) 'View MARC' and 'Order' buttons should show under an Actions column with appropriate icons
4) Click Order
5) Click add item when you are redirected to the New order page (no need to fill in any details)
6) Confirm that 'Edit' and 'Delete' buttons now show under an Actions column with appropriate icons
7) Go back to your basket and click add to basket -> from a subscription (we are skipping from a suggestion because this already has a button before my patch)
8) Confirm that 'Order' shows as a button with appropriate icon
9) Go back to your basket and click add to basket -> from an external source
10) fill in any fields and click search to bring up results
11) 'Order' should now show as a button with appropriate icons. MARC and Card preview have been moved into a dropdown menu - confirm this works as expected. Links have not been changed when clicking on a row
12) Go back to your basket and click add to basket -> from a staged file
13) 'Add orders' should now show as a button with appropriate icon
14) Go back to your basket and click add to basket -> from titles with highest hold ratios
15) 'X to order' should now show as a button with appropriate icon

Sponsored-by: Catalyst IT

Signed-off-by: Nicole C Engard <nengard@bywatersolutions.com>

Followed test plan, works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 35 Owen Leonard 2018-12-07 16:26:53 UTC
Created attachment 82987 [details] [review]
Bug 16939: (follow-up) Correction for orderreceive.tt

This patch makes a change to orderreceive.tt which corresponds to the
changes made in the previous patch.

To test, apply the patch and set the AcqCreateItem system preference to
"receiving an order."

- Go to Acquisitions -> Vendor -> Receive shipment.
- Submit the "Receive a new shipment" form.
- Click "Receive" for an order line.
- Fill out the item form and click "Add item."
- The table of existing items should display correctly, with a single
  "Actions" column containing an "Edit" and "Delete" button.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 36 Owen Leonard 2018-12-07 16:26:57 UTC
Created attachment 82988 [details] [review]
Bug 16939: (follow-up) Adding missing bracket

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 37 Jonathan Druart 2018-12-12 16:02:07 UTC
Created attachment 83133 [details] [review]
Bug 16939: Making all 'add to basket' actions buttons

Edit: Fixing 'clas' typo in addorderiso2709.tt
Edit: Adding actions class to td
Edit: Fixing js error
Edit: circ/reserveratios.tt updated so that the plus icon doesn't show
      if there is no link
Edit: fixing logic in circ/reserveratios.tt
Edit: merge conflicts
Edit: updating buttons to bootstrap 3, which i believe fixes the problem
      in comment 16

To test:

1) Go to Acqui -> find a vendor -> view a basket
2) Click add to basket -> fill in 'for an existing record' and hit search
3) 'View MARC' and 'Order' buttons should show under an Actions column with appropriate icons
4) Click Order
5) Click add item when you are redirected to the New order page (no need to fill in any details)
6) Confirm that 'Edit' and 'Delete' buttons now show under an Actions column with appropriate icons
7) Go back to your basket and click add to basket -> from a subscription (we are skipping from a suggestion because this already has a button before my patch)
8) Confirm that 'Order' shows as a button with appropriate icon
9) Go back to your basket and click add to basket -> from an external source
10) fill in any fields and click search to bring up results
11) 'Order' should now show as a button with appropriate icons. MARC and Card preview have been moved into a dropdown menu - confirm this works as expected. Links have not been changed when clicking on a row
12) Go back to your basket and click add to basket -> from a staged file
13) 'Add orders' should now show as a button with appropriate icon
14) Go back to your basket and click add to basket -> from titles with highest hold ratios
15) 'X to order' should now show as a button with appropriate icon

Sponsored-by: Catalyst IT

Signed-off-by: Nicole C Engard <nengard@bywatersolutions.com>

Followed test plan, works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 38 Jonathan Druart 2018-12-12 16:02:12 UTC
Created attachment 83134 [details] [review]
Bug 16939: (follow-up) Correction for orderreceive.tt

This patch makes a change to orderreceive.tt which corresponds to the
changes made in the previous patch.

To test, apply the patch and set the AcqCreateItem system preference to
"receiving an order."

- Go to Acquisitions -> Vendor -> Receive shipment.
- Submit the "Receive a new shipment" form.
- Click "Receive" for an order line.
- Fill out the item form and click "Add item."
- The table of existing items should display correctly, with a single
  "Actions" column containing an "Edit" and "Delete" button.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 39 Jonathan Druart 2018-12-12 16:02:17 UTC
Created attachment 83135 [details] [review]
Bug 16939: (follow-up) Adding missing bracket

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 40 Nick Clemens (kidclamp) 2018-12-26 19:58:40 UTC
Please rebase, thanks
Comment 41 Aleisha Amohia 2018-12-28 01:19:43 UTC
(In reply to Nick Clemens from comment #40)
> Please rebase, thanks

This applies cleanly for me? Where does it need rebasing?
Comment 42 Nick Clemens (kidclamp) 2019-01-02 20:31:29 UTC
I must have hit the wrong bug number, apologies
Comment 43 Nick Clemens (kidclamp) 2019-01-02 21:09:29 UTC
Awesome work all!

Pushed to master for 19.05
Comment 44 Martin Renvoize (ashimema) 2019-01-07 15:09:03 UTC
Enhancement, will not be backported.