Bug 21398 - Search term when adding an order from an existing record should be required
Summary: Search term when adding an order from an existing record should be required
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low normal (vote)
Target Milestone: ---
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-09-22 14:07 UTC by Jonathan Druart
Modified: 2018-10-24 09:13 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 21398: Make search field mandatory when adding to a basket from an existing record (1.56 KB, patch)
2018-09-22 14:11 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 21398: Make search field mandatory when adding to a basket from an existing record (1.62 KB, patch)
2018-09-27 08:01 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 21398: Make search field mandatory when adding to a basket from an existing record (1.67 KB, patch)
2018-09-27 10:09 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2018-09-22 14:07:58 UTC
Otherwise we get this ugly screen: https://snag.gy/U8X3ZK.jpg
Comment 1 Jonathan Druart 2018-09-22 14:11:11 UTC
Created attachment 79265 [details] [review]
Bug 21398: Make search field mandatory when adding to a basket from an existing record

Test plan:
- Create a basket
- Click the "Add to basket" button
- Click "Search" on the first line
=> Without this patch you get an ugly screen: https://snag.gy/U8X3ZK.jpg
=> With this patch applied you will not be able to submit without
filling the input in

QA note: Yes, we could handle that controller side as well, but much
more work.
Comment 2 Martin Renvoize 2018-09-27 08:01:24 UTC
Created attachment 79468 [details] [review]
Bug 21398: Make search field mandatory when adding to a basket from an existing record

Test plan:
- Create a basket
- Click the "Add to basket" button
- Click "Search" on the first line
=> Without this patch you get an ugly screen: https://snag.gy/U8X3ZK.jpg
=> With this patch applied you will not be able to submit without
filling the input in

QA note: Yes, we could handle that controller side as well, but much
more work.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 3 Martin Renvoize 2018-09-27 08:02:23 UTC
Simple fix, does what it says on the tin. Signing Off
Comment 4 Katrin Fischer 2018-09-27 10:09:16 UTC
Created attachment 79473 [details] [review]
Bug 21398: Make search field mandatory when adding to a basket from an existing record

Test plan:
- Create a basket
- Click the "Add to basket" button
- Click "Search" on the first line
=> Without this patch you get an ugly screen: https://snag.gy/U8X3ZK.jpg
=> With this patch applied you will not be able to submit without
filling the input in

QA note: Yes, we could handle that controller side as well, but much
more work.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 5 Katrin Fischer 2018-09-27 10:10:26 UTC
This works, but looks a bit different than other places we have required fields (no red label, no *, no red required showing up behind the field - but maybe it would not look right in this case?) No blocker for me, just noting.
Comment 6 Nick Clemens 2018-09-28 19:58:35 UTC
Awesome work all!

Pushed to master for 18.11
Comment 7 Nick Clemens 2018-09-28 19:59:03 UTC
(In reply to Katrin Fischer from comment #5)
> This works, but looks a bit different than other places we have required
> fields (no red label, no *, no red required showing up behind the field -
> but maybe it would not look right in this case?) No blocker for me, just
> noting.

The message when nothing is entered looks good so i think that suffices here
Comment 8 Jonathan Druart 2018-09-29 15:10:55 UTC
I tried the label in red but it looked bad, it takes too much visibility.
Comment 9 Katrin Fischer 2018-09-29 16:37:09 UTC
(In reply to Jonathan Druart from comment #8)
> I tried the label in red but it looked bad, it takes too much visibility.

I think you are right, it would be too prominent, when the search is only one valid option on the form.
Comment 10 Martin Renvoize 2018-10-03 09:22:36 UTC
Pushed to 18.05.x for 18.05.05
Comment 11 Fridolin SOMERS 2018-10-10 09:51:58 UTC
Pushed to 17.11.x for 17.11.11