[Koha-bugs] [Bug 22690] Merging records with many items too slow (Elasticsearch)
bugzilla-daemon at bugs.koha-community.org
bugzilla-daemon at bugs.koha-community.org
Mon Jul 19 15:45:58 CEST 2021
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690
--- Comment #152 from Martin Renvoize <martin.renvoize at ptfs-europe.com> ---
(In reply to Jonathan Druart from comment #150)
> 1. Why Koha::Item->item_orders is not named Koha::Item->orders?
> It returns a Koha::Acquisition::Orders.
This should certainly be named 'orders', annoyed I overlooked that during QA :(
>
> 2.
> in move_to_biblio
> 1215 return unless $self->biblionumber != $to_biblio->biblionumber;
> I am a fan of unless, not when there is a negation in the test :)
> return if $self->biblionumber == $to_biblio->biblionumber;
> Read much better IMO.
Agreed.
>
> 3. There is Koha::Item->move_to_biblio($biblio) and
> Koha::Biblio->adopt_items_from_biblio($biblio)
> Don't you think Koha::Biblio->adopt_items_from_biblio could be
> Koha::Items->move_to_biblio actually?
> It would be more consistent and flexible.
Hmmm, I think so long as the method name is clear I was happy for it to live in
either class. I do see what you mean though and for consistency, I certainly
like that.
>
> 4.
> in move_to_biblio you are calling, on a DBIC rs, ->update, then update_all:
> 1237 $hold_fill_target->update({ biblionumber => $to_biblionumber
> });
>
> 1254 $linktrackers->update_all({ biblionumber => $to_biblionumber });
>
> It's not consistent, is there a good reason for that?
>
> Please provide a fast reply, I can write the follow-up patches if needed.
I think this is simply because we didn't have a Koha:: based resultset yet and
he didn't want to add further complexity to the patch by adding that new class
as well. Having said that, it's fairly trivial to add such a class so long as
it's a basic one, so perhaps we should for consistency.
--
You are receiving this mail because:
You are watching all bug changes.
More information about the Koha-bugs
mailing list