Created attachment 158725 [details] [review] Bug 35287: Add cypress tests Test plan, k-t-d: cd /kohadevbox/koha cypress run --spec "t/cypress/integration/AdditionalFields_spec.ts"
Created attachment 158726 [details] [review] Bug 35287: Add unit tests Test plan, k-t-d: prove t/db_dependent/Koha/Object/Mixin/AdditionalFields.t
Created attachment 158727 [details] [review] Bug 35287: Preparation: Extend AdditionalFields Mixin The mixin now implements the extended_attributes method to be utilized by any Koha class that implements the AdditionalFields Mixin and is exposed through the REST API It also implements a strings_map to be utilized in the same fashion as described above. This is useful because additional fields may be an authorised value
Created attachment 158728 [details] [review] Bug 35287: Preparation: AdditionalFields License Mixin Koha::Object::Mixin::AdditionalFields needs to come before Koha::Object in the inheritance so that the strings_map method that is called is the one from Mixin and not from Koha::Object. This is only required for Koha classes implementing the AdditionalFields Mixin and expose data through the REST API, because strings_map is implement in both the aforementioned classes
Created attachment 158729 [details] [review] Bug 35287: Admin ERM licenses is now a table option in the additional fields admin panel ERM licenses allows for the 'searchable' option (as well as 'repeatable')
Created attachment 158730 [details] [review] Bug 35287: REST API - Add additional fields support to ERM Licenses
Created attachment 158731 [details] [review] Bug 35287: Vue preparation - AdditionalFields New api client: additional-fields-api-client.js to interact with the REST endpoint introduced by bug 35197 2 new components: - AdditionalFieldsEntry: Component used to extend the FormAdd component including the corresponding additional fields - AdditionalFieldsDisplay: Component used to display the additional fields associated to a data record implementing it (makes use of strings_map)
Created attachment 158732 [details] [review] Bug 35287: Vue preparation - Extend KohaTable to consider searchable additional fields This follows the same pattern as in subscription serials searchable additional fields, i.e. if an additional field is searchable, it is added to the list table - with the corresponding search inputs and functionality
Created attachment 158733 [details] [review] Bug 35287: Vue - Add additional fields support to ERM Licenses Test plan, k-t-d: 1) Add a new additional field for the newly supported 'erm_licenses', visit: /cgi-bin/koha/admin/additional-fields.pl?tablename=erm_licenses 2) Create 4 fields by clicking on '+ New field' 2.1) 'text non-repeatable' 2.2) 'text repeatable', check the 'repeatable' box 2.3) 'av non-repeatable', pick an authorised value category 2.4) 'av repeatable', pick an authorised value category and check the 'repeatable' box 3) Add a new ERM license, visit: /cgi-bin/koha/erm/licenses/add 4) Notice there is now a 'Additional fields' section at the bottom listing the fields we created 5) Fill in the mandatory regular license fields and play around with the additional fields 5.1) Put in some text in the text fields, test out the 'clear' and '+new' buttons 5.2) Select some AV options from the AV fields, deselect them, notice the repeatable one allows for multiple selection, the non-repeatable one does not 6) Save the license. On the list table, click on the license name (to navigate to the show page), or if license id=1, visit: /cgi-bin/koha/erm/licenses/1 7) Notice the additional fields are listed there, AV fields show their human readable description, not the AV code. Repeatable fields are shown comma separated 8) Edit the license, visit: /cgi-bin/koha/erm/licenses/edit/1 9) Play around again, do some more inserting and deletion of additional fields. Save. Notice everything is as expected. Searchable testing: 10) Go back to the additional fields admin panel, visit: /cgi-bin/koha/admin/additional-fields.pl?tablename=erm_licenses 11) Edit one field (AV or not) that you have inserted some data in, in the previous steps, and tick the 'searchable' box 12) Go back to licenses, visit: /cgi-bin/koha/erm/licenses 13) Notice the searchable additional field now has its own column in the licenses list table 14) If the searchable field is AV, it has a dropdown with the AV values of the same AV category 15) If the searchable field not AV, it has a text input that allows for normal text search 16) If the searchable field is repeatable and has multiple values, its displayed in comma separated
Created attachment 158734 [details] [review] Bug 35287: perltidy for green QA script
Created attachment 159634 [details] [review] Bug 35287: Add cypress tests Test plan, k-t-d: cd /kohadevbox/koha cypress run --spec "t/cypress/integration/AdditionalFields_spec.ts" Signed-off-by: Edith Speller <edith.speller@ukhsa.gov.uk>
Created attachment 159635 [details] [review] Bug 35287: Add unit tests Test plan, k-t-d: prove t/db_dependent/Koha/Object/Mixin/AdditionalFields.t Signed-off-by: Edith Speller <edith.speller@ukhsa.gov.uk>
Created attachment 159636 [details] [review] Bug 35287: Preparation: Extend AdditionalFields Mixin The mixin now implements the extended_attributes method to be utilized by any Koha class that implements the AdditionalFields Mixin and is exposed through the REST API It also implements a strings_map to be utilized in the same fashion as described above. This is useful because additional fields may be an authorised value Signed-off-by: Edith Speller <edith.speller@ukhsa.gov.uk>
Created attachment 159637 [details] [review] Bug 35287: Preparation: AdditionalFields License Mixin Koha::Object::Mixin::AdditionalFields needs to come before Koha::Object in the inheritance so that the strings_map method that is called is the one from Mixin and not from Koha::Object. This is only required for Koha classes implementing the AdditionalFields Mixin and expose data through the REST API, because strings_map is implement in both the aforementioned classes Signed-off-by: Edith Speller <edith.speller@ukhsa.gov.uk>
Created attachment 159638 [details] [review] Bug 35287: Admin ERM licenses is now a table option in the additional fields admin panel ERM licenses allows for the 'searchable' option (as well as 'repeatable') Signed-off-by: Edith Speller <edith.speller@ukhsa.gov.uk>
Created attachment 159639 [details] [review] Bug 35287: REST API - Add additional fields support to ERM Licenses Signed-off-by: Edith Speller <edith.speller@ukhsa.gov.uk>
Created attachment 159640 [details] [review] Bug 35287: Vue preparation - AdditionalFields New api client: additional-fields-api-client.js to interact with the REST endpoint introduced by bug 35197 2 new components: - AdditionalFieldsEntry: Component used to extend the FormAdd component including the corresponding additional fields - AdditionalFieldsDisplay: Component used to display the additional fields associated to a data record implementing it (makes use of strings_map) Signed-off-by: Edith Speller <edith.speller@ukhsa.gov.uk>
Created attachment 159641 [details] [review] Bug 35287: Vue preparation - Extend KohaTable to consider searchable additional fields This follows the same pattern as in subscription serials searchable additional fields, i.e. if an additional field is searchable, it is added to the list table - with the corresponding search inputs and functionality Signed-off-by: Edith Speller <edith.speller@ukhsa.gov.uk>
Created attachment 159642 [details] [review] Bug 35287: Vue - Add additional fields support to ERM Licenses Test plan, k-t-d: 1) Add a new additional field for the newly supported 'erm_licenses', visit: /cgi-bin/koha/admin/additional-fields.pl?tablename=erm_licenses 2) Create 4 fields by clicking on '+ New field' 2.1) 'text non-repeatable' 2.2) 'text repeatable', check the 'repeatable' box 2.3) 'av non-repeatable', pick an authorised value category 2.4) 'av repeatable', pick an authorised value category and check the 'repeatable' box 3) Add a new ERM license, visit: /cgi-bin/koha/erm/licenses/add 4) Notice there is now a 'Additional fields' section at the bottom listing the fields we created 5) Fill in the mandatory regular license fields and play around with the additional fields 5.1) Put in some text in the text fields, test out the 'clear' and '+new' buttons 5.2) Select some AV options from the AV fields, deselect them, notice the repeatable one allows for multiple selection, the non-repeatable one does not 6) Save the license. On the list table, click on the license name (to navigate to the show page), or if license id=1, visit: /cgi-bin/koha/erm/licenses/1 7) Notice the additional fields are listed there, AV fields show their human readable description, not the AV code. Repeatable fields are shown comma separated 8) Edit the license, visit: /cgi-bin/koha/erm/licenses/edit/1 9) Play around again, do some more inserting and deletion of additional fields. Save. Notice everything is as expected. Searchable testing: 10) Go back to the additional fields admin panel, visit: /cgi-bin/koha/admin/additional-fields.pl?tablename=erm_licenses 11) Edit one field (AV or not) that you have inserted some data in, in the previous steps, and tick the 'searchable' box 12) Go back to licenses, visit: /cgi-bin/koha/erm/licenses 13) Notice the searchable additional field now has its own column in the licenses list table 14) If the searchable field is AV, it has a dropdown with the AV values of the same AV category 15) If the searchable field not AV, it has a text input that allows for normal text search 16) If the searchable field is repeatable and has multiple values, its displayed in comma separated Signed-off-by: Edith Speller <edith.speller@ukhsa.gov.uk>
Created attachment 159643 [details] [review] Bug 35287: perltidy for green QA script Signed-off-by: Edith Speller <edith.speller@ukhsa.gov.uk>
Great work, thanks very much Pedro!
Comment on attachment 159639 [details] [review] Bug 35287: REST API - Add additional fields support to ERM Licenses Review of attachment 159639 [details] [review]: ----------------------------------------------------------------- ::: Koha/REST/V1/ERM/Licenses.pm @@ +183,5 @@ > $license->user_roles($user_roles); > $license->documents($documents); > > + my @extended_attributes = map { {'id' => $_->{field_id}, 'value' => $_->{value}} } @{$extended_attributes}; > + $license->extended_attributes( \@extended_attributes ); Won't you need to check here that @extended_attributes is indeed populated.. else you may wipe them away without meaning to?
(In reply to Martin Renvoize from comment #22) > Comment on attachment 159639 [details] [review] [review] > Bug 35287: REST API - Add additional fields support to ERM Licenses > > Review of attachment 159639 [details] [review] [review]: > ----------------------------------------------------------------- > > ::: Koha/REST/V1/ERM/Licenses.pm > @@ +183,5 @@ > > $license->user_roles($user_roles); > > $license->documents($documents); > > > > + my @extended_attributes = map { {'id' => $_->{field_id}, 'value' => $_->{value}} } @{$extended_attributes}; > > + $license->extended_attributes( \@extended_attributes ); > > Won't you need to check here that @extended_attributes is indeed populated.. > else you may wipe them away without meaning to? Good point. It's working like that by design. The scenario of wiping additional attributes without meaning to should not be possible through the Koha form, as existing data is always pre-populated on the form when editing. Having said that, it is still possible with a third-party request to the REST API but I believe that to be a wider topic. I've opened bug 35747 where I provide a test plan to discuss it.
Created attachment 165451 [details] [review] Bug 35287: Rebase after follow-up to 35197
(In reply to Pedro Amorim from comment #24) > Created attachment 165451 [details] [review] [review] > Bug 35287: Rebase after follow-up to 35197 Thanks Pedro! We tested the additional fields for licenses and we wanted to sign off but have some problems with ktd and also with public sandboxes. Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Created attachment 165637 [details] [review] Bug 35287: Add cypress tests Test plan, k-t-d: cd /kohadevbox/koha cypress run --spec "t/cypress/integration/AdditionalFields_spec.ts" Signed-off-by: Edith Speller <edith.speller@ukhsa.gov.uk> Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Created attachment 165638 [details] [review] Bug 35287: Add unit tests Test plan, k-t-d: prove t/db_dependent/Koha/Object/Mixin/AdditionalFields.t Signed-off-by: Edith Speller <edith.speller@ukhsa.gov.uk> Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Created attachment 165639 [details] [review] Bug 35287: Preparation: Extend AdditionalFields Mixin The mixin now implements the extended_attributes method to be utilized by any Koha class that implements the AdditionalFields Mixin and is exposed through the REST API It also implements a strings_map to be utilized in the same fashion as described above. This is useful because additional fields may be an authorised value Signed-off-by: Edith Speller <edith.speller@ukhsa.gov.uk> Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Created attachment 165640 [details] [review] Bug 35287: Preparation: AdditionalFields License Mixin Koha::Object::Mixin::AdditionalFields needs to come before Koha::Object in the inheritance so that the strings_map method that is called is the one from Mixin and not from Koha::Object. This is only required for Koha classes implementing the AdditionalFields Mixin and expose data through the REST API, because strings_map is implement in both the aforementioned classes Signed-off-by: Edith Speller <edith.speller@ukhsa.gov.uk> Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Created attachment 165641 [details] [review] Bug 35287: Admin ERM licenses is now a table option in the additional fields admin panel ERM licenses allows for the 'searchable' option (as well as 'repeatable') Signed-off-by: Edith Speller <edith.speller@ukhsa.gov.uk> Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Created attachment 165642 [details] [review] Bug 35287: REST API - Add additional fields support to ERM Licenses Signed-off-by: Edith Speller <edith.speller@ukhsa.gov.uk> Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Created attachment 165643 [details] [review] Bug 35287: Vue preparation - AdditionalFields New api client: additional-fields-api-client.js to interact with the REST endpoint introduced by bug 35197 2 new components: - AdditionalFieldsEntry: Component used to extend the FormAdd component including the corresponding additional fields - AdditionalFieldsDisplay: Component used to display the additional fields associated to a data record implementing it (makes use of strings_map) Signed-off-by: Edith Speller <edith.speller@ukhsa.gov.uk> Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Created attachment 165644 [details] [review] Bug 35287: Vue preparation - Extend KohaTable to consider searchable additional fields This follows the same pattern as in subscription serials searchable additional fields, i.e. if an additional field is searchable, it is added to the list table - with the corresponding search inputs and functionality Signed-off-by: Edith Speller <edith.speller@ukhsa.gov.uk> Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Created attachment 165645 [details] [review] Bug 35287: Vue - Add additional fields support to ERM Licenses Test plan, k-t-d: 1) Add a new additional field for the newly supported 'erm_licenses', visit: /cgi-bin/koha/admin/additional-fields.pl?tablename=erm_licenses 2) Create 4 fields by clicking on '+ New field' 2.1) 'text non-repeatable' 2.2) 'text repeatable', check the 'repeatable' box 2.3) 'av non-repeatable', pick an authorised value category 2.4) 'av repeatable', pick an authorised value category and check the 'repeatable' box 3) Add a new ERM license, visit: /cgi-bin/koha/erm/licenses/add 4) Notice there is now a 'Additional fields' section at the bottom listing the fields we created 5) Fill in the mandatory regular license fields and play around with the additional fields 5.1) Put in some text in the text fields, test out the 'clear' and '+new' buttons 5.2) Select some AV options from the AV fields, deselect them, notice the repeatable one allows for multiple selection, the non-repeatable one does not 6) Save the license. On the list table, click on the license name (to navigate to the show page), or if license id=1, visit: /cgi-bin/koha/erm/licenses/1 7) Notice the additional fields are listed there, AV fields show their human readable description, not the AV code. Repeatable fields are shown comma separated 8) Edit the license, visit: /cgi-bin/koha/erm/licenses/edit/1 9) Play around again, do some more inserting and deletion of additional fields. Save. Notice everything is as expected. Searchable testing: 10) Go back to the additional fields admin panel, visit: /cgi-bin/koha/admin/additional-fields.pl?tablename=erm_licenses 11) Edit one field (AV or not) that you have inserted some data in, in the previous steps, and tick the 'searchable' box 12) Go back to licenses, visit: /cgi-bin/koha/erm/licenses 13) Notice the searchable additional field now has its own column in the licenses list table 14) If the searchable field is AV, it has a dropdown with the AV values of the same AV category 15) If the searchable field not AV, it has a text input that allows for normal text search 16) If the searchable field is repeatable and has multiple values, its displayed in comma separated Signed-off-by: Edith Speller <edith.speller@ukhsa.gov.uk> Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Created attachment 165646 [details] [review] Bug 35287: perltidy for green QA script Signed-off-by: Edith Speller <edith.speller@ukhsa.gov.uk> Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Created attachment 165647 [details] [review] Bug 35287: Rebase after follow-up to 35197 Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>