Bug 35190 - Additional_fields table should allow null values for authorised_value_category
Summary: Additional_fields table should allow null values for authorised_value_category
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Emily Lamancusa
QA Contact: Katrin Fischer
URL:
Keywords:
Depends on:
Blocks: 29390 35197
  Show dependency treegraph
 
Reported: 2023-10-30 15:05 UTC by Emily Lamancusa
Modified: 2023-12-07 11:57 UTC (History)
3 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:
23.11.00,23.05.06


Attachments
Bug 35190: Set default NULL for authorised_value_category in additional_fields table (3.24 KB, patch)
2023-10-30 17:26 UTC, Emily Lamancusa
Details | Diff | Splinter Review
Bug 35190: Adjust tests (936 bytes, patch)
2023-10-30 17:26 UTC, Emily Lamancusa
Details | Diff | Splinter Review
Bug 35190: Adjust UI handling to avoid inserting an empty starting (3.28 KB, patch)
2023-10-30 17:26 UTC, Emily Lamancusa
Details | Diff | Splinter Review
Bug 35190: Adjust UI handling to avoid inserting an empty string (3.28 KB, patch)
2023-10-30 17:27 UTC, Emily Lamancusa
Details | Diff | Splinter Review
Bug 35190: Set default NULL for authorised_value_category in additional_fields table (3.34 KB, patch)
2023-10-31 13:20 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 35190: Adjust tests (1.01 KB, patch)
2023-10-31 13:20 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 35190: Adjust UI handling to avoid inserting an empty string (3.38 KB, patch)
2023-10-31 13:20 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 35190: Set default NULL for authorised_value_category in additional_fields table (3.35 KB, patch)
2023-11-01 18:13 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 35190: Adjust tests (1.03 KB, patch)
2023-11-01 18:13 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 35190: Adjust UI handling to avoid inserting an empty string (3.39 KB, patch)
2023-11-01 18:13 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 Emily Lamancusa 2023-10-30 15:05:48 UTC
When creating an Additional Field in the staff interface, the UI explicitly allows the Additional Field not to be linked to an authorized value. However, the database doesn't allow NULL values for authorised_value_category in the additional_fields table. Instead, an empty string is inserted into the database if the user chooses not to link an authorized value. This is not good practice, and does not allow a foreign key constraint to be properly implemented.
Comment 1 Emily Lamancusa 2023-10-30 17:26:14 UTC
Created attachment 158087 [details] [review]
Bug 35190: Set default NULL for authorised_value_category in additional_fields table

To test:
1. Start KTD without this patch
2. In the Administration module, create an Additional Field with
   Authorized Value set to None
3. Create another Additional Field with Authorized Value set to an
   existing authorized value
4. Apply patch and updatedatabase
5. Access the database in the command line with koha-mysql kohadev
6. Select all rows from additional_fields
7. Confirm that the two Additional Fields from earlier have values of
   NULL and the correct authorized value, respectively, in their
   authorised_value_category fields.
8. reset_all to test new install
9. Access the database in the command line
10. Show the table structure for additional_fields and confirm that
    default for authorised_value_category is NULL
Comment 2 Emily Lamancusa 2023-10-30 17:26:15 UTC
Created attachment 158088 [details] [review]
Bug 35190: Adjust tests

Test plan:
prove t/db_dependent/Koha/AdditionalField.t
Comment 3 Emily Lamancusa 2023-10-30 17:26:17 UTC Comment hidden (obsolete)
Comment 4 Emily Lamancusa 2023-10-30 17:27:11 UTC
Created attachment 158090 [details] [review]
Bug 35190: Adjust UI handling to avoid inserting an empty string

To test:
1. In the Administration module, create an Additional Field that is
   not linked to an authorized value
2. Create another Additional Field that is linked to an authorized
   value
3. Access the database in the command line (koha-mysql kohadev)
4. Select all from additional_fields
5. Confirm that each field correctly lists either the appropriate
   authorized value, or NULL
Comment 5 Marcel de Rooy 2023-10-31 12:52:41 UTC
    #   Failed test 'no default category'
    #   at /usr/share/koha/t/db_dependent/Koha/AdditionalField.t line 33.
    #          got: ''
    #     expected: undef
     # Looks like you failed 1 test of 4.

Will take a look now.
Comment 6 Marcel de Rooy 2023-10-31 12:54:51 UTC
(In reply to Marcel de Rooy from comment #5)
>     #   Failed test 'no default category'
>     #   at /usr/share/koha/t/db_dependent/Koha/AdditionalField.t line 33.
>     #          got: ''
>     #     expected: undef
>      # Looks like you failed 1 test of 4.
> 
> Will take a look now.

Easy fix: run the dbrev LOL
Comment 7 Marcel de Rooy 2023-10-31 13:20:09 UTC
Created attachment 158116 [details] [review]
Bug 35190: Set default NULL for authorised_value_category in additional_fields table

To test:
1. Start KTD without this patch
2. In the Administration module, create an Additional Field with
   Authorized Value set to None
3. Create another Additional Field with Authorized Value set to an
   existing authorized value
4. Apply patch and updatedatabase
5. Access the database in the command line with koha-mysql kohadev
6. Select all rows from additional_fields
7. Confirm that the two Additional Fields from earlier have values of
   NULL and the correct authorized value, respectively, in their
   authorised_value_category fields.
8. reset_all to test new install
9. Access the database in the command line
10. Show the table structure for additional_fields and confirm that
    default for authorised_value_category is NULL

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 8 Marcel de Rooy 2023-10-31 13:20:11 UTC
Created attachment 158117 [details] [review]
Bug 35190: Adjust tests

Test plan:
prove t/db_dependent/Koha/AdditionalField.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 9 Marcel de Rooy 2023-10-31 13:20:14 UTC
Created attachment 158118 [details] [review]
Bug 35190: Adjust UI handling to avoid inserting an empty string

To test:
1. In the Administration module, create an Additional Field that is
   not linked to an authorized value
2. Create another Additional Field that is linked to an authorized
   value
3. Access the database in the command line (koha-mysql kohadev)
4. Select all from additional_fields
5. Confirm that each field correctly lists either the appropriate
   authorized value, or NULL

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 10 Katrin Fischer 2023-11-01 18:11:21 UTC
(In reply to Marcel de Rooy from comment #6)
> (In reply to Marcel de Rooy from comment #5)
> >     #   Failed test 'no default category'
> >     #   at /usr/share/koha/t/db_dependent/Koha/AdditionalField.t line 33.
> >     #          got: ''
> >     #     expected: undef
> >      # Looks like you failed 1 test of 4.
> > 
> > Will take a look now.
> 
> Easy fix: run the dbrev LOL

Thx for leaving the comment, I ran into this too :)
Comment 11 Katrin Fischer 2023-11-01 18:13:45 UTC
Created attachment 158203 [details] [review]
Bug 35190: Set default NULL for authorised_value_category in additional_fields table

To test:
1. Start KTD without this patch
2. In the Administration module, create an Additional Field with
   Authorized Value set to None
3. Create another Additional Field with Authorized Value set to an
   existing authorized value
4. Apply patch and updatedatabase
5. Access the database in the command line with koha-mysql kohadev
6. Select all rows from additional_fields
7. Confirm that the two Additional Fields from earlier have values of
   NULL and the correct authorized value, respectively, in their
   authorised_value_category fields.
8. reset_all to test new install
9. Access the database in the command line
10. Show the table structure for additional_fields and confirm that
    default for authorised_value_category is NULL

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 12 Katrin Fischer 2023-11-01 18:13:48 UTC
Created attachment 158204 [details] [review]
Bug 35190: Adjust tests

Test plan:
prove t/db_dependent/Koha/AdditionalField.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 13 Katrin Fischer 2023-11-01 18:13:50 UTC
Created attachment 158205 [details] [review]
Bug 35190: Adjust UI handling to avoid inserting an empty string

To test:
1. In the Administration module, create an Additional Field that is
   not linked to an authorized value
2. Create another Additional Field that is linked to an authorized
   value
3. Access the database in the command line (koha-mysql kohadev)
4. Select all from additional_fields
5. Confirm that each field correctly lists either the appropriate
   authorized value, or NULL

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 14 Tomás Cohen Arazi 2023-11-03 17:23:20 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 15 Fridolin Somers 2023-11-10 07:00:49 UTC
Pushed to 23.05.x for 23.05.06