Bug 18680 - sort1/sort1 dropdowns (when mapped to authorized value) have no empty entry
Summary: sort1/sort1 dropdowns (when mapped to authorized value) have no empty entry
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Katrin Fischer
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-05-26 15:23 UTC by Nick Clemens
Modified: 2020-05-14 17:49 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
20.05.00, 19.11.06, 19.05.11


Attachments
Bug 18680: Add an empty entry by default to sort1/sort2 values in patron account (4.71 KB, patch)
2020-04-19 12:21 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 18680: Add an empty entry by default to sort1/sort2 values in patron account (4.85 KB, patch)
2020-04-19 12:23 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 18680: Add an empty entry by default to sort1/sort2 values in patron account (4.90 KB, patch)
2020-04-19 15:52 UTC, David Roberts
Details | Diff | Splinter Review
Bug 18680: Add an empty entry by default to sort1/sort2 values in patron account (4.96 KB, patch)
2020-04-20 10:24 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 Nick Clemens 2017-05-26 15:23:12 UTC

    
Comment 1 Nick Clemens 2017-05-26 15:25:20 UTC
To recreate:
1 - Define values for bsort1 
2 - Edit a patron
3 - note that you must select a value (cannot leave blank)

You can workaround this by instering a blank value into the authorized value list, but it seems we should not force selection.
Comment 2 Katrin Fischer 2020-04-19 12:21:11 UTC
Created attachment 103213 [details] [review]
Bug 18680: Add an empty entry by default to sort1/sort2 values in patron account

When using sort1/sort2 with authorised values to create a pull down
list, there will be no empty entry unless one is manually added to
the authorised values list.

This is not a good default behaviour, as it can easily
cause errors on patron creation. Without the empty entry, the first
alphabetic value is pre-selected and will be saved if not explicitly
changed. It also doesn't allow to mark sort1/sort2 as mandatory,
as the value is always 'set', the required message won't be triggered.

This adds an empty parameter to av-build-dropbox.inc to allow selectively
adding this empty entry without causing side-effects in other places
this include is used.

To test:
- Create authorised values for either Bsort1 or Bsort2 authorised
  value category
- Create a new patron:
  - you should see a pull down list on one and an input field for the other
  - for the pull down list, there will be no empty entry and the first
    alphabetic value will be preselected
- Make your sort1 and sort2 mandatory using BorrowerMandatoryField
- Verify there is still on empty entry
- Apply patch
- Repeat tests and verify that the behaviour is as expected
  - There should now be an empty entry
  - When the field is marked mandatory and empty is left, the required
    warning will be shown.
Comment 3 Katrin Fischer 2020-04-19 12:21:48 UTC
There might be some things to discuss here, but I am putting it out there as we just recently ran into that question in training again.
Comment 4 Katrin Fischer 2020-04-19 12:23:11 UTC
Created attachment 103214 [details] [review]
Bug 18680: Add an empty entry by default to sort1/sort2 values in patron account

When using sort1/sort2 with authorised values to create a pull down
list, there will be no empty entry unless one is manually added to
the authorised values list.

This is not a good default behaviour, as it can easily
cause errors on patron creation. Without the empty entry, the first
alphabetic value is pre-selected and will be saved if not explicitly
changed. It also doesn't allow to mark sort1/sort2 as mandatory,
as the value is always 'set', the required message won't be triggered.

This adds an empty parameter to av-build-dropbox.inc to allow selectively
adding this empty entry without causing side-effects in other places
this include is used.

To test:
- Create authorised values for either Bsort1 or Bsort2 authorised
  value category
- Create a new patron:
  - you should see a pull down list on one and an input field for the other
  - for the pull down list, there will be no empty entry and the first
    alphabetic value will be preselected
- Make your sort1 and sort2 mandatory using BorrowerMandatoryField
- Verify there is still on empty entry
- Apply patch
- Repeat tests and verify that the behaviour is as expected
  - There should now be an empty entry
  - When the field is marked mandatory and empty is left, the required
    warning will be shown.
Comment 5 David Roberts 2020-04-19 15:52:59 UTC
Created attachment 103215 [details] [review]
Bug 18680: Add an empty entry by default to sort1/sort2 values in patron account

When using sort1/sort2 with authorised values to create a pull down
list, there will be no empty entry unless one is manually added to
the authorised values list.

This is not a good default behaviour, as it can easily
cause errors on patron creation. Without the empty entry, the first
alphabetic value is pre-selected and will be saved if not explicitly
changed. It also doesn't allow to mark sort1/sort2 as mandatory,
as the value is always 'set', the required message won't be triggered.

This adds an empty parameter to av-build-dropbox.inc to allow selectively
adding this empty entry without causing side-effects in other places
this include is used.

To test:
- Create authorised values for either Bsort1 or Bsort2 authorised
  value category
- Create a new patron:
  - you should see a pull down list on one and an input field for the other
  - for the pull down list, there will be no empty entry and the first
    alphabetic value will be preselected
- Make your sort1 and sort2 mandatory using BorrowerMandatoryField
- Verify there is still on empty entry
- Apply patch
- Repeat tests and verify that the behaviour is as expected
  - There should now be an empty entry
  - When the field is marked mandatory and empty is left, the required
    warning will be shown.

Signed-off-by: David Roberts <david@koha-ptfs.co.uk>
Comment 6 Jonathan Druart 2020-04-20 10:24:55 UTC
Created attachment 103257 [details] [review]
Bug 18680: Add an empty entry by default to sort1/sort2 values in patron account

When using sort1/sort2 with authorised values to create a pull down
list, there will be no empty entry unless one is manually added to
the authorised values list.

This is not a good default behaviour, as it can easily
cause errors on patron creation. Without the empty entry, the first
alphabetic value is pre-selected and will be saved if not explicitly
changed. It also doesn't allow to mark sort1/sort2 as mandatory,
as the value is always 'set', the required message won't be triggered.

This adds an empty parameter to av-build-dropbox.inc to allow selectively
adding this empty entry without causing side-effects in other places
this include is used.

To test:
- Create authorised values for either Bsort1 or Bsort2 authorised
  value category
- Create a new patron:
  - you should see a pull down list on one and an input field for the other
  - for the pull down list, there will be no empty entry and the first
    alphabetic value will be preselected
- Make your sort1 and sort2 mandatory using BorrowerMandatoryField
- Verify there is still on empty entry
- Apply patch
- Repeat tests and verify that the behaviour is as expected
  - There should now be an empty entry
  - When the field is marked mandatory and empty is left, the required
    warning will be shown.

Signed-off-by: David Roberts <david@koha-ptfs.co.uk>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 7 Martin Renvoize 2020-04-20 11:27:08 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 8 Joy Nelson 2020-05-07 21:51:57 UTC
Backported to 19.11.x for 19.11.06
Comment 9 Lucas Gass 2020-05-14 17:49:31 UTC
backported to 19.05.x for 19.05.11