Summary: | Standard backend uses plain SQL | ||
---|---|---|---|
Product: | Koha | Reporter: | Tomás Cohen Arazi (tcohen) <tomascohen> |
Component: | ILL | Assignee: | Tomás Cohen Arazi (tcohen) <tomascohen> |
Status: | Pushed to main --- | QA Contact: | Martin Renvoize (ashimema) <martin.renvoize> |
Severity: | normal | ||
Priority: | P5 - low | CC: | lisette, martin.renvoize, pedro.amorim, tomascohen |
Version: | unspecified | ||
Hardware: | All | ||
OS: | All | ||
GIT URL: | Change sponsored?: | --- | |
Patch complexity: | Small patch | Documentation contact: | |
Documentation submission: | Text to go in the release notes: | ||
Version(s) released in: |
25.11.00
|
Circulation function: | |
Bug Depends on: | 40850 | ||
Bug Blocks: | 38341, 40856 | ||
Attachments: |
Bug 40855: Add unit tests for Standard ILL backend edititem method
Bug 40855: Replace raw SQL in Standard backend edititem with add_or_update_attributes Bug 40855: Add unit tests for Standard ILL backend edititem method Bug 40855: Replace raw SQL in Standard backend edititem with add_or_update_attributes |
Description
Tomás Cohen Arazi (tcohen)
2025-09-22 19:05:03 UTC
Created attachment 186740 [details] [review] Bug 40855: Add unit tests for Standard ILL backend edititem method This patch adds comprehensive unit tests for the edititem() method in the Standard ILL backend to establish baseline behavior before refactoring. The tests cover: - Form stage (initial load) - displays current attributes for editing - Commit stage (form submission) - processes updates with validation - Core field updates (title, author, year, etc.) - Custom field handling (null-delimited key/value pairs) - Attribute persistence after updates These tests demonstrate the current raw SQL implementation behavior and will ensure the upcoming refactoring maintains compatibility. Current behavior tested: - edititem() uses multi-stage workflow (init -> form -> commit) - Validation requires 'type' and 'branchcode' fields - Custom fields use null-delimited strings for multiple values - Method returns 'create' (not 'edititem') in commit stage - Raw SQL delete-all-and-recreate pattern for attribute updates Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/ILL/Backend/Standard.t => SUCCESS: Tests pass! Current edititem behavior documented 3. Tests establish baseline for upcoming refactoring 4. Sign off :-D Created attachment 186741 [details] [review] Bug 40855: Replace raw SQL in Standard backend edititem with add_or_update_attributes This patch refactors the edititem() method in the Standard ILL backend to use the modern add_or_update_attributes() method instead of raw SQL delete-all-and-recreate operations. Problems with the old approach: - 40+ lines of raw SQL bypassing ORM validation - Inefficient delete-all-then-insert pattern - Complex conditional SQL based on column existence - Manual transaction handling and error-prone bind parameters - Difficult to maintain and debug Benefits of the new approach: - Single line replaces 40+ lines of raw SQL - Uses ORM validation and relationships - Only updates attributes when values actually change - Automatic backend column handling - Consistent with other modernized Standard backend methods - Better performance and maintainability The refactoring: - Removes all raw SQL queries (DELETE and INSERT statements) - Removes manual transaction handling - Removes conditional column existence checks - Replaces with single add_or_update_attributes() call - Maintains identical functionality and behavior Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/ILL/Backend/Standard.t => SUCCESS: Tests pass! Refactoring maintains compatibility 3. Test ILL request editing in Standard backend: - Edit existing ILL request metadata - Add/modify custom fields - Verify attributes updated correctly => SUCCESS: Same functionality, cleaner implementation 4. Sign off :-D Created attachment 186758 [details] [review] Bug 40855: Add unit tests for Standard ILL backend edititem method This patch adds comprehensive unit tests for the edititem() method in the Standard ILL backend to establish baseline behavior before refactoring. The tests cover: - Form stage (initial load) - displays current attributes for editing - Commit stage (form submission) - processes updates with validation - Core field updates (title, author, year, etc.) - Custom field handling (null-delimited key/value pairs) - Attribute persistence after updates These tests demonstrate the current raw SQL implementation behavior and will ensure the upcoming refactoring maintains compatibility. Current behavior tested: - edititem() uses multi-stage workflow (init -> form -> commit) - Validation requires 'type' and 'branchcode' fields - Custom fields use null-delimited strings for multiple values - Method returns 'create' (not 'edititem') in commit stage - Raw SQL delete-all-and-recreate pattern for attribute updates Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/ILL/Backend/Standard.t => SUCCESS: Tests pass! Current edititem behavior documented 3. Tests establish baseline for upcoming refactoring 4. Sign off :-D Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> Created attachment 186759 [details] [review] Bug 40855: Replace raw SQL in Standard backend edititem with add_or_update_attributes This patch refactors the edititem() method in the Standard ILL backend to use the modern add_or_update_attributes() method instead of raw SQL delete-all-and-recreate operations. Problems with the old approach: - 40+ lines of raw SQL bypassing ORM validation - Inefficient delete-all-then-insert pattern - Complex conditional SQL based on column existence - Manual transaction handling and error-prone bind parameters - Difficult to maintain and debug Benefits of the new approach: - Single line replaces 40+ lines of raw SQL - Uses ORM validation and relationships - Only updates attributes when values actually change - Automatic backend column handling - Consistent with other modernized Standard backend methods - Better performance and maintainability The refactoring: - Removes all raw SQL queries (DELETE and INSERT statements) - Removes manual transaction handling - Removes conditional column existence checks - Replaces with single add_or_update_attributes() call - Maintains identical functionality and behavior Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/ILL/Backend/Standard.t => SUCCESS: Tests pass! Refactoring maintains compatibility 3. Test ILL request editing in Standard backend: - Edit existing ILL request metadata - Add/modify custom fields - Verify attributes updated correctly => SUCCESS: Same functionality, cleaner implementation 4. Sign off :-D Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> Simple internal API improvement, well covered by tests. Straight for QA, Passed. Nice work everyone! Pushed to main for 25.11 |