Bug 29030 - Problems introduced by bug 25728
Summary: Problems introduced by bug 25728
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Owen Leonard
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 29137
  Show dependency treegraph
 
Reported: 2021-09-15 13:20 UTC by Klas Blomberg
Modified: 2022-06-06 20:25 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
21.11.00


Attachments
Bug 29030: Make authorized value and description fields required (8.94 KB, patch)
2021-09-15 17:58 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 29030: Make authorized value and description fields required (8.98 KB, patch)
2021-09-16 10:33 UTC, David Nind
Details | Diff | Splinter Review
Bug 29030: Make authorized value and description fields required (9.06 KB, patch)
2021-09-26 14:17 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 Klas Blomberg 2021-09-15 13:20:57 UTC
There are two problems with the feature of creating new authorized values introduced by bugg 25728

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25728

The MINOR problem is that you can save a new authorized value without actually entering a new authorized value, you just need a description

The MAJOR problem is that this feature isn't optional!
Introducing a new authorized value is a complex process where you have to take library limitaions into consideration, authorized values are used in sysprefs and conjobs, often you have to rewrite some reports when you introduce a new authorized value &c, &c.
In short: We certainly don't want anyone to create new authorized values on the fly! Not even I should have that possibility.

We have had this unwelcome feature for some weeks, and there are already dozens of new athorized values created by mistake.
Comment 1 Owen Leonard 2021-09-15 14:23:25 UTC
At the very least the authorized value and description fields should be mandatory in the modal form.
Comment 2 Owen Leonard 2021-09-15 17:58:37 UTC
Created attachment 124905 [details] [review]
Bug 29030: Make authorized value and description fields required

This patch modifies the markup of the "Create a new authorized value"
modal so that a minimum set of fields is required: Authorized value and
description.

The patch also modifies the JavaScript which handles the submission so
that the jQuery Validation plugin can handle the field checks.

The spelling "authorised" is changed to "authorized" following coding
guidelines.

To test, apply the patch and rebuild the staff interface CSS
(https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client).

- Locate a record in the catalog which has items and open an item for
  editing.
- In the add item form, test the process of adding an authorized
  value on the fly with the following fields: Withdrawn, Lost,
  Damaged, Use restrictions, Not for loan, Collection code, Shelving
  location, and Shelving control number.
  - In each case you should be able to type a new value in the
    dropdown's search box and be shown the option "Select to create."
  - Selecting should trigger a modal window, "Create a new authorized
    value."
  - Test that both "Authorized value" and "Description" fields are
    required, and the form can't be submitted without them.
  - Test that an error message shows up when you submit an authorized
    value which already exists, e.g. authval "1" for "DAMAGED."
  - After triggering this error, click the "Cancel" button and try
    creating another new authorized value. When the modal reopens the
    form should be reset: No previously-entered data, no error messages.
  - Submitting a valid form with a new authorized value should work
    correctly. The modal window should close automatically.
Comment 3 Owen Leonard 2021-09-15 18:03:00 UTC
(In reply to Owen Leonard from comment #1)
> At the very least the authorized value and description fields should be
> mandatory in the modal form.

My patch makes this change along with some other interface improvements. It would be nice if errors triggered by duplicate authorized values were better handled... Right now it triggers a 500 error which shows up in the browser console.
Comment 4 David Nind 2021-09-16 10:33:27 UTC
Created attachment 124920 [details] [review]
Bug 29030: Make authorized value and description fields required

This patch modifies the markup of the "Create a new authorized value"
modal so that a minimum set of fields is required: Authorized value and
description.

The patch also modifies the JavaScript which handles the submission so
that the jQuery Validation plugin can handle the field checks.

The spelling "authorised" is changed to "authorized" following coding
guidelines.

To test, apply the patch and rebuild the staff interface CSS
(https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client).

- Locate a record in the catalog which has items and open an item for
  editing.
- In the add item form, test the process of adding an authorized
  value on the fly with the following fields: Withdrawn, Lost,
  Damaged, Use restrictions, Not for loan, Collection code, Shelving
  location, and Shelving control number.
  - In each case you should be able to type a new value in the
    dropdown's search box and be shown the option "Select to create."
  - Selecting should trigger a modal window, "Create a new authorized
    value."
  - Test that both "Authorized value" and "Description" fields are
    required, and the form can't be submitted without them.
  - Test that an error message shows up when you submit an authorized
    value which already exists, e.g. authval "1" for "DAMAGED."
  - After triggering this error, click the "Cancel" button and try
    creating another new authorized value. When the modal reopens the
    form should be reset: No previously-entered data, no error messages.
  - Submitting a valid form with a new authorized value should work
    correctly. The modal window should close automatically.

Signed-off-by: David Nind <david@davidnind.com>
Comment 5 Katrin Fischer 2021-09-26 14:17:33 UTC
Created attachment 125337 [details] [review]
Bug 29030: Make authorized value and description fields required

This patch modifies the markup of the "Create a new authorized value"
modal so that a minimum set of fields is required: Authorized value and
description.

The patch also modifies the JavaScript which handles the submission so
that the jQuery Validation plugin can handle the field checks.

The spelling "authorised" is changed to "authorized" following coding
guidelines.

To test, apply the patch and rebuild the staff interface CSS
(https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client).

- Locate a record in the catalog which has items and open an item for
  editing.
- In the add item form, test the process of adding an authorized
  value on the fly with the following fields: Withdrawn, Lost,
  Damaged, Use restrictions, Not for loan, Collection code, Shelving
  location, and Shelving control number.
  - In each case you should be able to type a new value in the
    dropdown's search box and be shown the option "Select to create."
  - Selecting should trigger a modal window, "Create a new authorized
    value."
  - Test that both "Authorized value" and "Description" fields are
    required, and the form can't be submitted without them.
  - Test that an error message shows up when you submit an authorized
    value which already exists, e.g. authval "1" for "DAMAGED."
  - After triggering this error, click the "Cancel" button and try
    creating another new authorized value. When the modal reopens the
    form should be reset: No previously-entered data, no error messages.
  - Submitting a valid form with a new authorized value should work
    correctly. The modal window should close automatically.

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 6 Jonathan Druart 2021-09-29 10:08:34 UTC
Error display change:

Before: https://snipboard.io/oGy94Y.jpg
After : https://snipboard.io/cN1FnV.jpg

Looks better!
Comment 7 Jonathan Druart 2021-09-29 10:09:06 UTC
Klas, is this patch answering your need? If not, please open a new bug report.
Comment 8 Jonathan Druart 2021-09-29 10:44:36 UTC
Pushed to master for 21.11, thanks to everybody involved!