Bug 35190

Summary: Additional_fields table should allow null values for authorised_value_category
Product: Koha Reporter: Emily Lamancusa <emily.lamancusa>
Component: Architecture, internals, and plumbingAssignee: Emily Lamancusa <emily.lamancusa>
Status: RESOLVED FIXED QA Contact: Katrin Fischer <katrin.fischer>
Severity: normal    
Priority: P5 - low CC: fridolin.somers, m.de.rooy, pedro.amorim
Version: Main   
Hardware: All   
OS: All   
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
Bug Depends on:    
Bug Blocks: 29390, 35197    
Attachments: Bug 35190: Set default NULL for authorised_value_category in additional_fields table
Bug 35190: Adjust tests
Bug 35190: Adjust UI handling to avoid inserting an empty starting
Bug 35190: Adjust UI handling to avoid inserting an empty string
Bug 35190: Set default NULL for authorised_value_category in additional_fields table
Bug 35190: Adjust tests
Bug 35190: Adjust UI handling to avoid inserting an empty string
Bug 35190: Set default NULL for authorised_value_category in additional_fields table
Bug 35190: Adjust tests
Bug 35190: Adjust UI handling to avoid inserting an empty string

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