| Summary: | Using additional fields on ERM agreements results in an error when loading the agreements table | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Jan Kissig <bibliothek> |
| Component: | ERM | Assignee: | Martin Renvoize (ashimema) <martin.renvoize> |
| Status: | Pushed to main --- | QA Contact: | Tomás Cohen Arazi (tcohen) <tomascohen> |
| Severity: | critical | ||
| Priority: | P5 - low | CC: | david, jonathan.druart, jonathan.field, lucas, martin.renvoize, matt.blenkinsop, pedro.amorim |
| Version: | Main | ||
| Hardware: | All | ||
| OS: | All | ||
| GIT URL: | Initiative type: | --- | |
| Sponsorship status: | Unsponsored | Comma delimited list of Sponsors: | |
| Crowdfunding goal: | 0 | Patch complexity: | Trivial patch |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: |
This fixes the ERM agreements table, when additional fields are added for agreements.
When loading the ERM agreements table, a 500 error was generated:
Something went wrong when loading the table.
500: Internal Server Error
Properties not allowed: record_table.
Properties not allowed: record_table.
Properties not allowed: record_table.
|
Version(s) released in: |
26.05.00
|
| Circulation function: | |||
| Bug Depends on: | 35451 | ||
| Bug Blocks: | |||
| Attachments: |
Bug 41520: Add tests for extended_attributes in ERM agreements API
Bug 41520: Filter out record_table from AdditionalFieldValue API responses Bug 41520: Add tests for extended_attributes in ERM agreements API Bug 41520: Filter out record_table from AdditionalFieldValue API responses Bug 41520: Add tests for extended_attributes in ERM agreements API Bug 41520: Filter out record_table from AdditionalFieldValue API responses Bug 41520: Add tests for extended_attributes in ERM agreements API Bug 41520: Filter out record_table from AdditionalFieldValue API responses |
||
Created attachment 190892 [details] [review] Bug 41520: Add tests for extended_attributes in ERM agreements API This patch adds api unit tests for the extended_attributes functionality in the ERM agreements API to ensure that: 1. Extended attributes are correctly returned when embedded 2. The internal record_table field is not exposed via the API 3. Both single GET and list endpoints work correctly with extended_attributes Test plan: 1. prove t/db_dependent/api/v1/erm_agreements.t 2. Verify all tests pass Created attachment 190893 [details] [review] Bug 41520: Filter out record_table from AdditionalFieldValue API responses Bug 35451 added a record_table field to the additional_field_values database table for performance optimization (better joins). However, this field was not added to the OpenAPI schema, and with additionalProperties: false, the API validator rejects responses containing it. Rather than adding this internal implementation detail to the public API schema, this patch adds a custom to_api method to Koha::AdditionalFieldValue that filters out the record_table field. This approach: 1. Keeps the database optimization from Bug 35451 2. Prevents internal implementation details from cluttering the API 3. Maintains context that API consumers already have from parent objects 4. Fixes OpenAPI validation errors for ERM agreements, licenses, and packages using extended_attributes Test plan: 1. Create an ERM agreement with additional fields/extended attributes 2. Access the agreement via the API with extended_attributes embedded 3. Verify the response includes extended_attributes but not record_table 4. Confirm no OpenAPI validation warnings in plack-api-error.log 5. prove t/db_dependent/api/v1/erm_agreements.t 6. Verify all tests pass Created attachment 190906 [details] [review] Bug 41520: Add tests for extended_attributes in ERM agreements API This patch adds api unit tests for the extended_attributes functionality in the ERM agreements API to ensure that: 1. Extended attributes are correctly returned when embedded 2. The internal record_table field is not exposed via the API 3. Both single GET and list endpoints work correctly with extended_attributes Test plan: 1. prove t/db_dependent/api/v1/erm_agreements.t 2. Verify all tests pass Signed-off-by: Jan Kissig <bibliothek@th-wildau.de> Created attachment 190907 [details] [review] Bug 41520: Filter out record_table from AdditionalFieldValue API responses Bug 35451 added a record_table field to the additional_field_values database table for performance optimization (better joins). However, this field was not added to the OpenAPI schema, and with additionalProperties: false, the API validator rejects responses containing it. Rather than adding this internal implementation detail to the public API schema, this patch adds a custom to_api method to Koha::AdditionalFieldValue that filters out the record_table field. This approach: 1. Keeps the database optimization from Bug 35451 2. Prevents internal implementation details from cluttering the API 3. Maintains context that API consumers already have from parent objects 4. Fixes OpenAPI validation errors for ERM agreements, licenses, and packages using extended_attributes Test plan: 1. Create an ERM agreement with additional fields/extended attributes 2. Access the agreement via the API with extended_attributes embedded 3. Verify the response includes extended_attributes but not record_table 4. Confirm no OpenAPI validation warnings in plack-api-error.log 5. prove t/db_dependent/api/v1/erm_agreements.t 6. Verify all tests pass Signed-off-by: Jan Kissig <bibliothek@th-wildau.de> Hi Jan. You've signed off the bug, but the status is still showing as "Needs Signoff". David PS: I would add my sign off, so you could do the QA, but I don't know how to do steps 2 and 3 of the test plan. (In reply to David Nind from comment #6) > PS: I would add my sign off, so you could do the QA, but I don't know how to > do steps 2 and 3 of the test plan. Hi David, thanks for the status reminder, I set it to signed off now but please add yours too: 2. Access the agreement via the API with extended_attributes embedded 3. Verify the response includes extended_attributes but not record_table For 2 and 3 you could either use an API testing tool and set - the URL to: /api/v1/erm/agreements - the header to: x-koha-embed: extended_attributes or just open the browser development tools -> network tab and reload that page: /cgi-bin/koha/erm/agreements. Then take a look to for the API request to /api/v1/erm/agreements (it includes some more query params here but we can ignore them) and by double clicking that request you can check for the request headers (they include x-koha-embed with the value extended_attributes) and the response payload will include an extended_attributes object. This is how its done in firefox but I guess the other browsers work alike Hope this was understandable :) Created attachment 190915 [details] [review] Bug 41520: Add tests for extended_attributes in ERM agreements API This patch adds api unit tests for the extended_attributes functionality in the ERM agreements API to ensure that: 1. Extended attributes are correctly returned when embedded 2. The internal record_table field is not exposed via the API 3. Both single GET and list endpoints work correctly with extended_attributes Test plan: 1. prove t/db_dependent/api/v1/erm_agreements.t 2. Verify all tests pass Signed-off-by: Jan Kissig <bibliothek@th-wildau.de> Signed-off-by: David Nind <david@davidnind.com> Created attachment 190916 [details] [review] Bug 41520: Filter out record_table from AdditionalFieldValue API responses Bug 35451 added a record_table field to the additional_field_values database table for performance optimization (better joins). However, this field was not added to the OpenAPI schema, and with additionalProperties: false, the API validator rejects responses containing it. Rather than adding this internal implementation detail to the public API schema, this patch adds a custom to_api method to Koha::AdditionalFieldValue that filters out the record_table field. This approach: 1. Keeps the database optimization from Bug 35451 2. Prevents internal implementation details from cluttering the API 3. Maintains context that API consumers already have from parent objects 4. Fixes OpenAPI validation errors for ERM agreements, licenses, and packages using extended_attributes Test plan: 1. Create an ERM agreement with additional fields/extended attributes 2. Access the agreement via the API with extended_attributes embedded 3. Verify the response includes extended_attributes but not record_table 4. Confirm no OpenAPI validation warnings in plack-api-error.log 5. prove t/db_dependent/api/v1/erm_agreements.t 6. Verify all tests pass Signed-off-by: Jan Kissig <bibliothek@th-wildau.de> Signed-off-by: David Nind <david@davidnind.com> Thanks Jan, with your tips I managed to muddle my way through using Bruno (an open-source API client). Managed to reproduce the error after enabling ERM, adding two additional fields for EZR > Agreements, and creating an agreement using those fields: Something went wrong when loading the table. 500: Internal Server Error Properties not allowed: record_table. Properties not allowed: record_table. Properties not allowed: record_table. After the patches, the tests pass and the agreements table loads in the staff interface. Using an API client (such as Bruno) the request is completed successfully: - GET http://127.0.0.1:8081/api/v1/erm/agreements - Body: No body - Headers: . Key: x-koha-embed . Value: extended_attributes - Auth: Basic Auth with koha + koha As always, many thanks for testing David :) In fact, thank you both.. I somehow completely missed Jan has tested and signed off too! Created attachment 190927 [details] [review] Bug 41520: Add tests for extended_attributes in ERM agreements API This patch adds api unit tests for the extended_attributes functionality in the ERM agreements API to ensure that: 1. Extended attributes are correctly returned when embedded 2. The internal record_table field is not exposed via the API 3. Both single GET and list endpoints work correctly with extended_attributes Test plan: 1. prove t/db_dependent/api/v1/erm_agreements.t 2. Verify all tests pass Signed-off-by: Jan Kissig <bibliothek@th-wildau.de> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 190928 [details] [review] Bug 41520: Filter out record_table from AdditionalFieldValue API responses Bug 35451 added a record_table field to the additional_field_values database table for performance optimization (better joins). However, this field was not added to the OpenAPI schema, and with additionalProperties: false, the API validator rejects responses containing it. Rather than adding this internal implementation detail to the public API schema, this patch adds a custom to_api method to Koha::AdditionalFieldValue that filters out the record_table field. This approach: 1. Keeps the database optimization from Bug 35451 2. Prevents internal implementation details from cluttering the API 3. Maintains context that API consumers already have from parent objects 4. Fixes OpenAPI validation errors for ERM agreements, licenses, and packages using extended_attributes Test plan: 1. Create an ERM agreement with additional fields/extended attributes 2. Access the agreement via the API with extended_attributes embedded 3. Verify the response includes extended_attributes but not record_table 4. Confirm no OpenAPI validation warnings in plack-api-error.log 5. prove t/db_dependent/api/v1/erm_agreements.t 6. Verify all tests pass Signed-off-by: Jan Kissig <bibliothek@th-wildau.de> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Nice work everyone! Pushed to main for 26.05 |
To recreate: a) enable ERM module in system preferences b) create an addition field (/cgi-bin/koha/admin/additional-fields.pl) for Agreements (erm_agreements) c) go to /cgi-bin/koha/erm/agreements and create an agreement, fill the additional field. Save and reload /cgi-bin/koha/erm/agreements d) see browser error 'Something went wrong when loading the table. 500: Internal Server Error. Properties not allowed: record_table.' and error log in ktd: '==> /var/log/koha/kohadev/plack-api-error.log <== [2026/01/05 12:39:19] [WARN] OpenAPI >>> GET api/v1/erm/agreements [{"message":"Properties not allowed: record_table.","path":"\/body\/0\/extended_attributes\/0"}] '