https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41097
Nick Clemens (kidclamp) <[email protected]> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off --- Comment #5 from Nick Clemens (kidclamp) <[email protected]> --- (In reply to Marcel de Rooy from comment #4) > Looking at the code: > > This seems to be the only call: > my ( $recordid_to_keep, @recordids_to_merge ) = _choose_records( > $authority->authid, @recordids ); > > The POD says: returns first the record to merge to and list of records to > merge from > > The sub makes no distinction between $authority->authid and @recordids. > Note btw that @recordids is a BAD name; it contains MARC records ! > sub _choose_records { > my @recordids = @_; > > It returns candidate authids (picked from 001) but they are sorted. Why > should the original first argument still be first? It shouldn't - I think that was just a mistake > Shouldn't we add the first parameter (which is NOT a MARC record), not > include that one while sorting and push it in front when returning values? The whole point of sorting is to determine the best candidate to merge into, and we don't know which record that is before we sort. Julian wrote the original, I just got it through QA, so I can't say why we originally wanted the first record, but I think the testing can demonstrate we don't want the same record id and to end up trying to merge into the same record > This needs a bit more attention to clarify current obscureness while > touching those lines here. I welcome any additions to clarify the code, but I do think this fix is pretty striaghtforward -- 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/
