Bug 38273

Summary: Koha::Object->discard_changes should return the Koha::Object for chaining
Product: Koha Reporter: Martin Renvoize (ashimema) <martin.renvoize>
Component: Architecture, internals, and plumbingAssignee: Jonathan Druart <jonathan.druart>
Status: RESOLVED FIXED QA Contact: Kyle M Hall (khall) <kyle>
Severity: normal    
Priority: P5 - low CC: dcook, jonathan.druart, kyle, lucas, nick, pedro.amorim, tomascohen
Version: Main   
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:
24.11.00
Circulation function:
Bug Depends on: 17594    
Bug Blocks:    
Attachments: Bug 38273: Make Koha::Object->discard_changes chainable
Bug 38273: Make Koha::Object->discard_changes chainable
Bug 38273: Make sure same object is returned
Bug 38273: Make Koha::Object->discard_changes chainable
Bug 38273: Make sure same object is returned
Bug 38273: Adjust t/db_dependent/api/v1/holds.t

Description Martin Renvoize (ashimema) 2024-10-25 13:08:30 UTC
The dbic discard_changes method returns the current schema object ready for chaining.. for some reason we didn't wrap it to return the current Koha object when we do the same and as such we have to always call it in a distinct statement and we lose chainability.


my $patron = Koha::Patron->new()->store()->discard_changes;

This returns a Koah::Schema::Result::Borrower object.. it should return the fully updated from the database Koha::Patron object.

Instead, one has to do 

my $patron = Koha::Patron->new()->store();
$patron->discard_changes;

This is confusing and inconsistent with the parent library.
Comment 1 Tomás Cohen Arazi (tcohen) 2024-10-25 14:11:29 UTC
Love this. Been meaning to file this for ages.
Comment 2 Jonathan Druart 2024-10-28 10:49:34 UTC
Created attachment 173507 [details] [review]
Bug 38273: Make Koha::Object->discard_changes chainable
Comment 3 Tomás Cohen Arazi (tcohen) 2024-10-28 12:15:45 UTC
Created attachment 173528 [details] [review]
Bug 38273: Make Koha::Object->discard_changes chainable

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 4 Tomás Cohen Arazi (tcohen) 2024-10-28 12:15:48 UTC
Created attachment 173529 [details] [review]
Bug 38273: Make sure same object is returned

This patch adds a test to make sure the same object is returned. A code
change is also added to make sure the tests pass.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 5 Kyle M Hall (khall) 2024-10-28 14:29:31 UTC
Created attachment 173538 [details] [review]
Bug 38273: Make Koha::Object->discard_changes chainable

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 6 Kyle M Hall (khall) 2024-10-28 14:29:44 UTC
Created attachment 173539 [details] [review]
Bug 38273: Make sure same object is returned

This patch adds a test to make sure the same object is returned. A code
change is also added to make sure the tests pass.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 7 Martin Renvoize (ashimema) 2024-10-28 14:35:15 UTC
Blimey!.. This went through quick!.. nice one chaps.
Comment 8 Katrin Fischer 2024-10-29 14:38:36 UTC
(In reply to Martin Renvoize (ashimema) from comment #7)
> Blimey!.. This went through quick!.. nice one chaps.

Let's turn them all around that quickly :D
Comment 9 Katrin Fischer 2024-10-29 15:18:29 UTC
Pushed for 24.11!

Well done everyone, thank you!
Comment 10 Pedro Amorim 2024-10-30 09:20:02 UTC
Created attachment 173705 [details] [review]
Bug 38273: Adjust t/db_dependent/api/v1/holds.t
Comment 11 Jonathan Druart 2024-10-30 10:07:09 UTC
(In reply to Pedro Amorim from comment #10)
> Created attachment 173705 [details] [review] [review]
> Bug 38273: Adjust t/db_dependent/api/v1/holds.t

Good catch!
Comment 12 Katrin Fischer 2024-10-30 11:00:35 UTC
Follow-up pushed to main, fingers crossed! Thanks, Pedro!
Comment 13 Lucas Gass (lukeg) 2024-12-05 17:03:26 UTC
Failing tests on 24.05.x with this patch set applied. No backport.