Bug 11880

Summary: Select the barcode index by default when searching for label items
Product: Koha Reporter: Frédérick Capovilla <fcapovilla>
Component: Label/patron card printingAssignee: Frédérick Capovilla <fcapovilla>
Status: CLOSED FIXED QA Contact: Marcel de Rooy <m.de.rooy>
Severity: enhancement    
Priority: P5 - low CC: aleisha, charles.farmer, jonathan.druart, katrin.fischer, liz, m.de.rooy, nengard, nick, philippe.blouin, simith.doliveira, tomascohen, veron
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: Sponsored Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Select the barcode index by default then searching for label items.
[Signed-off] Select the barcode index by default then searching for label items.
Bug 11880: Select what has been selected before
[Signed-off] Bug 11880: Select what has been selected before
Bug 11880: Select the barcode index by default then searching for label items.
Bug 11880: Select what has been selected before

Description Frédérick Capovilla 2014-02-28 21:31:40 UTC Comment hidden (obsolete)
Comment 1 Marc Véron 2014-03-04 16:13:30 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2014-03-18 12:01:57 UTC
Frédérick:

The patch does:
  IF selected:
    select option
  ELSE:
    select option

It seems it works like that currently but it is definitively not a good way to do.
Do you know if the "selected" variable is passed to the template? I don't see any occurrence in the pl script.
Maybe could we remove all previous tests and always select the barcode option?

Marked as Failed QA.
Comment 3 Frédérick Capovilla 2014-03-18 20:06:49 UTC
The code in this template seems strange.
If (selected==true), it looks like ALL the options of the selectbox be set as selected. Am I right?

When we did our patch, we didn't touch the rest of the template because we didn't know why the page was set like that.

I need a second opinion before removing the "selected" option, since this fix goes beyond this simple patch.
Comment 4 Jonathan Druart 2014-03-19 09:44:22 UTC
(In reply to Frédérick Capovilla from comment #3)
> The code in this template seems strange.
> If (selected==true), it looks like ALL the options of the selectbox be set
> as selected. Am I right?

... yes!
But I did not find any situation where "selected" is set.
Comment 5 Nicole C. Engard 2015-03-06 11:14:52 UTC
I'm confused by this. I was going to sign off but see the notes at the bottom so I'm re-marking failed qa.  If that's wrong feel free to fix.

Nicole
Comment 6 Charles Farmer 2015-03-09 15:01:24 UTC
Despite the ugliness of the template in its present form, the patch works and is a development needed by our client.

This patch should be accepted as is. Fixing the template is considerably out of the scope of this development.
Comment 7 Aleisha Amohia 2015-04-24 04:37:23 UTC
Patch applies cleanly, signing off
Comment 8 Jonathan Druart 2015-04-24 08:06:17 UTC Comment hidden (obsolete)
Comment 9 Jonathan Druart 2015-04-24 08:08:44 UTC
(In reply to Charles Farmer from comment #6)
> Despite the ugliness of the template in its present form, the patch works
> and is a development needed by our client.
> 
> This patch should be accepted as is. Fixing the template is considerably out
> of the scope of this development.

It's not because it's needed by your client that the patch should be accepted as you have written it... It's your duty to sent clean patches and make the code better, not worst.
The patch you have sent does not make any sense and adds misunderstanding to the template.

Anyway, here a patch to clean the situation, it took me 5 minutes to write, much more less than argue for hours...

Back to needs signoff.
Comment 10 Marc Véron 2015-04-24 09:19:31 UTC Comment hidden (obsolete)
Comment 11 Marcel de Rooy 2015-07-03 08:19:44 UTC
Created attachment 40774 [details] [review]
Bug 11880: Select the barcode index by default then searching for label items.

Test case:
* Go to "Tools -> Label creator -> Manage batches"
* Click on the "New batch" button
* Click on the "Add item(s)" button
* A search window should open. The "Barcode" value should be selected in the the selectbox.

Followed test case. Patch behaves as expected.
Signed-off-by: Marc Veron <veron@veron.ch>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 12 Marcel de Rooy 2015-07-03 08:19:49 UTC
Created attachment 40775 [details] [review]
Bug 11880: Select what has been selected before

The previous patch wants to select the barcode option when the user is
on the item search for label, in all cases.
So even if 'title' is selected, a search done, and there is no result,
the barcode index is selected anyway.
In this case, the title option should be kept.

Test plan:
Confirm that the barcode option is the default choice, but other values
are kept if the search does not return any result.

Followed test plan, behaves as expected. Tested with all choices.
Signed-off-by: Marc Veron <veron@veron.ch>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 13 Marcel de Rooy 2015-07-03 08:21:17 UTC
Thx for cleaning up, Jonathan.
Comment 14 Tomás Cohen Arazi 2015-08-28 13:27:00 UTC
Patches pushed to master.

Thanks Frederick and Jonathan!
Comment 15 Liz Rea 2015-09-22 01:26:21 UTC
Pushed to 3.18.x will be in 3.18.11.