Bug 20621 - Add ability for Koha::Object objects to update themselves with database generated value automatically
Summary: Add ability for Koha::Object objects to update themselves with database gener...
Status: CLOSED WONTFIX
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Kyle M Hall
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-04-19 13:27 UTC by Kyle M Hall
Modified: 2023-12-28 20:42 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 20621: Add ability for Koha::Object objects to update themselves with database generated value automatically (1.60 KB, patch)
2018-04-19 13:28 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall 2018-04-19 13:27:17 UTC
Koha objects should, on store() update themselves from the db if the database creates any of the columns. That way we don't have to call discard_changes() manually.

This patch has no particular use yet, but I wanted to put it here for posterity.
Comment 2 Kyle M Hall 2018-04-19 13:28:24 UTC
Created attachment 74555 [details] [review]
Bug 20621: Add ability for Koha::Object objects to update themselves with database generated value automatically

Koha objects should, on store() update themselves from the db if the database creates any of the columns. That way we don't have to call discard_changes() manually.

See https://wiki.koha-community.org/wiki/Koha_Objects#store_doesn.27t_return_the_DB_object for details.
Comment 3 Fridolin Somers 2020-09-25 09:49:13 UTC
Ahhh ok that is why in unit tests we see for example in t/db_dependent/Members.t :
  $borrowernumber = Koha::Patron->new( \%data )->store->borrowernumber;
  my $patron = Koha::Patrons->find( $borrowernumber );

discard_changes() has a disturbing name i feel.

Should it use get_from_storage ?
  my $patron = Koha::Patron->new( \%data )->store->get_from_storage;

In my opinion store() should update the object, that is a common behavior in any ORM i think.
Comment 4 Kyle M Hall 2020-10-09 11:40:51 UTC
(In reply to Fridolin SOMERS from comment #3)
> Ahhh ok that is why in unit tests we see for example in
> t/db_dependent/Members.t :
>   $borrowernumber = Koha::Patron->new( \%data )->store->borrowernumber;
>   my $patron = Koha::Patrons->find( $borrowernumber );
> 
> discard_changes() has a disturbing name i feel.
> 
> Should it use get_from_storage ?
>   my $patron = Koha::Patron->new( \%data )->store->get_from_storage;
> 
> In my opinion store() should update the object, that is a common behavior in
> any ORM i think.

My understanding is that get_from_storage has more overhead than discard_changes, though they have the same end result. get_from_storage creates and entirely new object, where discard_changes simply updates the existing object.

https://metacpan.org/pod/DBIx::Class::Row#get_from_storage
Comment 5 Victor Grousset/tuxayo 2023-01-24 09:36:35 UTC
"Needs Signoff" => what is needed to do for signoff?

I guess for QA we need an automated test, right? So it's needed and having it run as a signoff would be enough.
Comment 6 Kyle M Hall 2023-01-24 11:39:21 UTC
(In reply to Victor Grousset/tuxayo from comment #5)
> "Needs Signoff" => what is needed to do for signoff?
> 
> I guess for QA we need an automated test, right? So it's needed and having
> it run as a signoff would be enough.

This is so old it still checks wantarray. I do not believe it is worth pursuing at this time.