|
Description
Martin Renvoize (ashimema)
2023-11-30 11:00:02 UTC
Created attachment 159390 [details] [review] Bug 35451: Add record_table to additional_field_values This patch updates the database to include a record_table field in the additional_field_values table and adds an index on that field. This should facilitate more performant joins. Created attachment 159391 [details] [review] Bug 35451: Record table in set_additional_fields Created attachment 159392 [details] [review] Bug 35451: Update relations Created attachment 159393 [details] [review] Bug 35451: Schema Created attachment 159394 [details] [review] Bug 35451: Fix test data Needs unit tests adding, but posting here as a proof of concept for now looking for feedback. Created attachment 159747 [details] [review] Bug 35451: Add record_table to additional_field_values This patch updates the database to include a record_table field in the additional_field_values table and adds an index on that field. This should facilitate more performant joins. Created attachment 159748 [details] [review] Bug 35451: Record table in set_additional_fields Created attachment 159749 [details] [review] Bug 35451: Update relations Created attachment 159750 [details] [review] Bug 35451: Schema Created attachment 159751 [details] [review] Bug 35451: Fix test data Is there a test plan for this? I get this message for a dependency when trying to apply the patches for this bug: git bz apply 35451 Bug 35451 Depends on bug 35044 (Signed Off) Follow? [(y)es, (n)o] y Cannot apply cleanly patches from bug 35044. Everything will be left dirty. git bz apply --continue will not continue the process if patches from other bug reports need to be applied. Once bug 35044 is fixed, I'll have another go at testing. My test plan was to: 1. Apply the patches 2. Update the database: updatedatabase 3. Restart everything (flush_memcached, restart_all) 4. Run the tests - they should pass: prove t/db_dependent/Koha/Objects/Mixin/AdditionalFields.t I wasn't sure what else needs to be tested, or if this is sufficient. Bug 35044 is now in main, however I am now getting the sha1/fake ancestor error: git bz apply 35451 Bug 35451 - Add tablename field to additional_field_values 159747 - Bug 35451: Add record_table to additional_field_values 159748 - Bug 35451: Record table in set_additional_fields 159749 - Bug 35451: Update relations 159750 - Bug 35451: Schema 159751 - Bug 35451: Fix test data Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 35451: Add record_table to additional_field_values Applying: Bug 35451: Record table in set_additional_fields Applying: Bug 35451: Update relations Applying: Bug 35451: Schema error: sha1 information is lacking or useless (Koha/Schema/Result/AdditionalFieldValue.pm). error: could not build fake ancestor Patch failed at 0001 Bug 35451: Schema Created attachment 173147 [details] [review] Bug 35451: Add record_table to additional_field_values This patch updates the database to include a record_table field in the additional_field_values table and adds an index on that field. This should facilitate more performant joins. Created attachment 173148 [details] [review] Bug 35451: Record table in set_additional_fields Created attachment 173149 [details] [review] Bug 35451: Update relations Created attachment 173150 [details] [review] Bug 35451: Schema Created attachment 173151 [details] [review] Bug 35451: Fix test data This shouldn't drastically change anything.. it's a performance piece and only really makes a difference where large numbers of additional attributes are defined. No real test plan per say. Created attachment 173421 [details] [review] Bug 35451: Add record_table to additional_field_values This patch updates the database to include a record_table field in the additional_field_values table and adds an index on that field. This should facilitate more performant joins. Signed-off-by: David Nind <david@davidnind.com> Created attachment 173422 [details] [review] Bug 35451: Record table in set_additional_fields Signed-off-by: David Nind <david@davidnind.com> Created attachment 173423 [details] [review] Bug 35451: Update relations Signed-off-by: David Nind <david@davidnind.com> Created attachment 173424 [details] [review] Bug 35451: Schema Signed-off-by: David Nind <david@davidnind.com> Created attachment 173425 [details] [review] Bug 35451: Fix test data Signed-off-by: David Nind <david@davidnind.com> Testing notes (using KTD) - main aim was to make sure everything works as expected:
1. Apply the patch.
2. Update the database: updatedatabase
3. Restart everything: restart_all
4. Run the tests - these should pass: prove t/db_dependent/Koha/Objects/Mixin/AdditionalFields.t
5. Make sure additional fields for acquisition order baskets continue to work as expected:
5.1 Go to Administration > Additional parameters > Additional fields
5.2 Select the table to add additional fields to (in this case, Order baskets (aqbasket))
5.3 Add a new field:
5.3.1 + New field
5.3.2 Name: TESTFIELD
5.3.3 Repeatable: select
5.3.4 Searchable: select
5.4 Add a new basket to "My Vendor" using the additional field
5.4.1 Go to Acquisitions
5.4.2 In the "Search vendors" section, click "Search"
5.4.3 Add a new order basket: + New > Basket
. Basket name: bz35451
. TESTFIELD (add two entries): This is a test + This is a second test
5.4.4 Check that you can edit and delete the entries for your additional field
5.5 Bonus (optional): add different types of additional fields (such as authorized values, or not repeatable) and test that they work
SQL
===
Before the patch, table for additional_field_values - no record_table column:
MariaDB [koha_kohadev]> select * from additional_field_values;
+----+----------+-----------+-----------------------+
| id | field_id | record_id | value |
+----+----------+-----------+-----------------------+
| 1 | 1 | 2 | This is a test |
| 2 | 1 | 2 | This is a second test |
+----+----------+-----------+-----------------------+
2 rows in set (0.000 sec)
After patch, table for additional_field_values - now has a record_table column:
MariaDB [koha_kohadev]> select * from additional_field_values;;
+----+----------+--------------+-----------+-----------------------+
| id | field_id | record_table | record_id | value |
+----+----------+--------------+-----------+-----------------------+
| 5 | 1 | aqbasket | 2 | This is a test |
| 6 | 1 | aqbasket | 2 | This is a second test |
+----+----------+--------------+-----------+-----------------------+
2 rows in set (0.000 sec)
|