Description
Jonathan Druart
2020-04-02 14:02:01 UTC
Created attachment 102297 [details] [review] Bug 25044: Remove the need to define koha_object[s]_class for standard object class names In Koha::Object[s]->prefetch_whitelist, there is a call to the koha_object[s]_class DBIC resultset class. We should not, otherwise those 2 methods will have to be defined all the time, even when we can guess it easily. Koha::Item <> Schema::Result::Item => standard Koha::Acquisition::Order <> Schema::Result::Aqorder => non-standard sub _get_object_class { my ( $type ) = @_; return unless $type; if( $type->can('koha_object_class') ) { return $type->koha_object_class; } $type =~ s|Schema::Result::||; return ${type}; } Test plan: % prove t/db_dependent/Koha/Object.t t/db_dependent/Koha/Objects.t should return green before and after this patch Good catch, I think we missed that method. Still, wouldn't it be more efficient to just have this information as schema metadata? (not even a method, just metadata in the big schema hash) Created attachment 102298 [details] [review] Bug 25044: Remove the need to define koha_object[s]_class for standard object class names In Koha::Object[s]->prefetch_whitelist, there is a call to the koha_object[s]_class DBIC resultset class. We should not, otherwise those 2 methods will have to be defined all the time, even when we can guess it easily. Koha::Item <> Schema::Result::Item => standard Koha::Acquisition::Order <> Schema::Result::Aqorder => non-standard sub _get_object_class { my ( $type ) = @_; return unless $type; if( $type->can('koha_object_class') ) { return $type->koha_object_class; } $type =~ s|Schema::Result::||; return ${type}; } Test plan: % prove t/db_dependent/Koha/Object.t t/db_dependent/Koha/Objects.t should return green before and after this patch Created attachment 102529 [details] [review] Bug 25044: Remove the need to define koha_object[s]_class for standard object class names In Koha::Object[s]->prefetch_whitelist, there is a call to the koha_object[s]_class DBIC resultset class. We should not, otherwise those 2 methods will have to be defined all the time, even when we can guess it easily. Koha::Item <> Schema::Result::Item => standard Koha::Acquisition::Order <> Schema::Result::Aqorder => non-standard sub _get_object_class { my ( $type ) = @_; return unless $type; if( $type->can('koha_object_class') ) { return $type->koha_object_class; } $type =~ s|Schema::Result::||; return ${type}; } Test plan: % prove t/db_dependent/Koha/Object.t t/db_dependent/Koha/Objects.t should return green before and after this patch Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 102774 [details] [review] Bug 25044: Remove the need to define koha_object[s]_class for standard object class names In Koha::Object[s]->prefetch_whitelist, there is a call to the koha_object[s]_class DBIC resultset class. We should not, otherwise those 2 methods will have to be defined all the time, even when we can guess it easily. Koha::Item <> Schema::Result::Item => standard Koha::Acquisition::Order <> Schema::Result::Aqorder => non-standard sub _get_object_class { my ( $type ) = @_; return unless $type; if( $type->can('koha_object_class') ) { return $type->koha_object_class; } $type =~ s|Schema::Result::||; return ${type}; } Test plan: % prove t/db_dependent/Koha/Object.t t/db_dependent/Koha/Objects.t should return green before and after this patch Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Trusting Tomas and Jonathan here as experts - not a big code change and tests pass. (In reply to Katrin Fischer from comment #6) > Trusting Tomas and Jonathan here as experts - not a big code change and > tests pass. Yeah, we are just 'doing it right' on something Agustin and I overlooked a bit. Nice work everyone! Pushed to master for 20.05 Tomas, one test is failing: t/Koha/REST/Plugin/Query.t .. 1/6 # Failed test 'exact match for JSON Pointer "/prefetch/0"' # at t/Koha/REST/Plugin/Query.t line 350. # Structures begin differing at: # $got = HASH(0x55daaa8747c8) # $expected = 'biblio' # Looks like you failed 1 test of 10. I do not understand what was supposed to do the code prior to this change. Koha::Schema::Result::Biblio does not have a koha_object_class, so the try was failing, and undef was assigned. I am suspecting the tests to be wrong, but it would be good if you or Agustin could confirm it. Created attachment 102927 [details] [review] Bug 25044: (follow-up) Fix wrong tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Nice work everyone! Pushed to master for 20.05 missing dependencies. not backported to 19.11.x |