https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690

--- Comment #102 from Ere Maijala <[email protected]> ---
(In reply to Martin Renvoize from comment #96)
> Comment on attachment 113967 [details] [review]
> Bug 22690: Refactor merging of records to improve performance (Elasticsearch)
> 
> Review of attachment 113967 [details] [review]:
> -----------------------------------------------------------------
> 
> ::: Koha/Item.pm
> @@ +1082,5 @@
> > +
> > +$params:
> > +    skip_record_index => 1|0
> > +
> > +Returns undef if the move failed or the biblionumber of the destination 
> > record otherwise
> 
> I wonder if this might be nicer as a fluent interface (i.e returning $self
> so it can be chained.. the undef return would become a no-op and the final
> return would be the updated Koha::Item object?)
> 
> 
> One for later perhaps

Probably yes, but I tried to make it resemble the old MoveItemFromBiblio. The
caller wants to know whether the move succeeeded, and I didn't want to broaden
the scope in trying to avoid making this change any larger.

> 
> ::: Koha/Schema/Result/Item.pm
> @@ +778,4 @@
> >      '+exclude_from_local_holds_priority' => { is_boolean => 1 },
> >  );
> >  
> > +# Relationship with orders via the aqorders_item table that not have 
> > foreign keys
> 
> Was there a reason not to add the foreign key and let dbic generate the
> relationship?

The same as above, to try to manage the scope of change. It would make sense to
do that, but I'm afraid there will be some gotchas, so better handled
separately.

-- 
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[email protected]
https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

Reply via email to