Summary: | Select the barcode index by default when searching for label items | ||
---|---|---|---|
Product: | Koha | Reporter: | Frédérick Capovilla <fcapovilla> |
Component: | Label/patron card printing | Assignee: | 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)
Created attachment 25820 [details] [review] [Signed-off] Select the barcode index by default then searching for label items. http://bugs.koha-community.org/show_bug.cgi?id=11880 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 Véron <veron@veron.ch> 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. 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. (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. 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 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. Patch applies cleanly, signing off Created attachment 38475 [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 is the search does not return any result. (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. Created attachment 38479 [details] [review] [Signed-off] 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 Véron <veron@veron.ch> 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> 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> Thx for cleaning up, Jonathan. Patches pushed to master. Thanks Frederick and Jonathan! Pushed to 3.18.x will be in 3.18.11. |