In 3.22.0 when attempting to use merge.pl, there is strange behavior. 1) When merging biblios, the records appear, but the Desitation Record is not automatically populated. This despite that all the checkboxes on the reference record are checked. One must manually check each box. Unchecking a box resets the whole list. As a side note: the new report generated after the merge forces the librarian to click a link to go to the new record. Previously, it merge simply redirected to the merged record. The report gives no new information. Perhaps I don't understand, but the report seems to add a needless step without significant reason. We can only merge two records at a time, so the merge report will always be quite trivial. 2) When attempting to merge authorities, merge.pl does not display the contents of either records, breaking merging of authorities. I have repeated this behavior on two different systems. I wonder if this has something to do with the new behavior pushed with Bug 8064.
Further research to help resolve this problem shows: 1) merge.pl for biblios does populate the ajax destination record, but this takes nearly 5 times longer than it did previously. (<1 sec in 3.18.6, now 7-10 sec in 3.22.0) So, that part of the bug just stems from my own impatience. Though perhaps recent changes have made this process more demanding on the server. I still suspect some problem with the fix to Bug 8064. I get the behavior in <a href="http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8064#c18">Comment 18</a> when merging biblios. 2) The screen after choosing the reference record and framework on merge.pl for authorities displays a totally blank ajax table. Looking at the changes 8064 made to /cataloguing/merge.tt there do not seem to be corresponding changes made to /authorities/merge.tt. Obviously some of the changes will not apply (the mandatory fields part), but looking at cataloguing/merge.tt around line 88, and then authorities/merge.tt around line 22, there seem to be some inconsistencies, though my coding experience is quite limited. I will attempt on my test server to repair what I can and see about reporting back as time permits.
Created attachment 45666 [details] Blank authorities merge screen after selecting reference record Example of the output of authorities/merge.pl after selecting the reference authority and clicking "Merge" to populate the tabs.
Verified in packages install. This is bad, rising severity.
Another update from some digging around : I am now quite certain Bug 8064 is the main culprit. The changes affect key files shared between the authorities and biblio merge systems. The changes were not benign to authorities. And looking at how things were modified, it seems the authority merging functionality was forgotten. This is most evident is the new code added in the shared merge-record.inc. Its changes are very biblio specific. But also clearly affected are merge.tt, merge.pl, etc. I suspect more is involved, though as replacing with older versions of the files I was not able to get authorities merging back on a test server, however, I spent only a few hours trying to hack it out, and have precious little experience.
Bug 8064 has it's merits in this problem, but is not alone. If you go back far enough( git reset --hard HEAD~144, just before 8064), you get populated fields, but the merge will fail with "Subfields are only for data fields (generally, just tags >= 010) at /home/bgkriegel/kohaclone/C4/AuthoritiesMarc.pm line 1536." You need to go way back ( git reset --hard HEAD~774, just before Bug 14639 ) to have a successful auth merge. In authorities/merge.pl, $recordObj1->authtype method returns 'undef', it's a Koha::MetadataRecord object. That was modified in Bug 14639
Created attachment 45767 [details] [review] Bug 15358: Repopulate fields for authority merge This patch implements a quick and dirty way to revert changes introduced by Bug 8064 that prevent populating fields in authority merge. To test: 1) Verify the problem, i.e. try to merge any two authorities. Check that fields are empty 2) Apply the patch 3) Try again, verify fields are now populated Auth still don't merge, there is another problem but don't know how to fix it, this is only a partial solution.
*** Bug 15370 has been marked as a duplicate of this bug. ***
Bernardo, With Marc planning to move Koha::Authorities over to Koha::MetadataRecord::Authorities, will this help resolve the issues you're seeing once the Bug 8064 changes are reverted? I would hate to be trying to resolve this problem if his fix over at Bug 15380 will further complicate matters. Perhaps, though, it will solve part of the problem and make your patch work. Also, as a fix, perhaps the easier short-term solution would be to separate the biblios and authorities merging systems. We could revert authorities merging to the older files if the MetadataRecord issue could be ironed out, but leave the biblios system as is. Looks like a lot of work to try to harmonize biblios and authorities merge at this point given all the changes that were made before this was noticed.
*** Bug 15410 has been marked as a duplicate of this bug. ***
Julian, could you please have a look at this blocker please? I would like a better solution than the Bernardo's patch.
Created attachment 46343 [details] [review] Bug 15358: Attempt to fix authorities merge This is a quick fix, and I'm quite sure there are other bugs hiding... but I managed to display both authority in tabs and merge them. The result preview requires at least one click on checkboxes to show up, I haven't looked at this issue yet.
Created attachment 46393 [details] [review] Bug 15358: Build result preview after page load
I don't have a git setup yet, but a 3.22.0 package setup on a test server, so for what it's worth ... I have not updated yet to 3.22.1, but will do so tomorrow morning, so can try to test it then on the latest packages. I've manually edited the relevant files in accordance with Julian's patches here. It does seem to fix things back to merging, but probably more thorough test are certainly in order from those who actually are competent here. I'm just a librarian and armchair hacker ... If all works tomorrow when I test on the latest packages, I'll signoff. If anyone is able to test before then, I'm more than happy to let them do it.
Ian, your testing is much appreciated!
(In reply to Ian Palko from comment #13) > I've manually edited the relevant files in accordance with Julian's patches > here. Please, help yourself, don't do that :) git is your friend and can help you.
I just tested the patch, the records and result are displayed as they should. But after merging, the resulted record has all the data from both records!
Failed QA last night because of Josef's comment. Glad to see this move forward, hope remaining problems can be solved.
(In reply to Jonathan Druart from comment #15) > (In reply to Ian Palko from comment #13) > > I've manually edited the relevant files in accordance with Julian's patches > > here. > > Please, help yourself, don't do that :) > git is your friend and can help you. I know... Eventually that will happen. For now, I need a backup server at the ready "just in case", as we have had some issues with the production server. It will be replaced (fingers crossed) thus summer when we move to a new building with a new server waiting.
(In reply to Josef Moravec from comment #16) > I just tested the patch, the records and result are displayed as they > should. But after merging, the resulted record has all the data from both > records! I can confirm this behavior. One step closer, though ...
Patch on bug 6657 could affect this one too.
Created attachment 46531 [details] [review] Bug 15358: Authorities merge - submit only result record This patch removes parts of form that should not be submitted just before submit. The same is done on biblio merge.
It's better. Now the merge form is displayed. Authorities are merged. BUT Biblio records containing the rejected authority are not relinked to the retained authority.
Created attachment 46651 [details] [review] Bug 15358: Attempt to fix authorities merge This is a quick fix, and I'm quite sure there are other bugs hiding... but I managed to display both authority in tabs and merge them. The result preview requires at least one click on checkboxes to show up, I haven't looked at this issue yet.
Created attachment 46652 [details] [review] Bug 15358: Build result preview after page load
Created attachment 46653 [details] [review] Bug 15358: Authorities merge - submit only result record This patch removes parts of form that should not be submitted just before submit. The same is done on biblio merge.
Trying to make this moving forward, I have moved some code to bug 15578 (with tests).
A test plan would be appreciated (even if as simple as 'Merge two authorities') I get this error when merging authorities: Software error: Subfields are only for data fields (generally, just tags >= 010) at /home/vagrant/kohaclone/C4/AuthoritiesMarc.pm line 1488. For help, please send mail to the webmaster ([no address given]), giving this error message and the time and date of the error.
(In reply to Nick Clemens from comment #27) > A test plan would be appreciated (even if as simple as 'Merge two > authorities') > > I get this error when merging authorities: > Software error: > > Subfields are only for data fields (generally, just tags >= 010) at > /home/vagrant/kohaclone/C4/AuthoritiesMarc.pm line 1488. I had a very quick look at the code and I'd say that it existed prior to bug 8064, could you test on a 3.20 release and confirm that the bug already existed?
(In reply to Jonathan Druart from comment #28) > (In reply to Nick Clemens from comment #27) > > A test plan would be appreciated (even if as simple as 'Merge two > > authorities') > > > > I get this error when merging authorities: > > Software error: > > > > Subfields are only for data fields (generally, just tags >= 010) at > > /home/vagrant/kohaclone/C4/AuthoritiesMarc.pm line 1488. > > I had a very quick look at the code and I'd say that it existed prior to bug > 8064, could you test on a 3.20 release and confirm that the bug already > existed? Equinox is still running a demo server with 3.20.1. When merging authorities on that demo server, there was no error. The biblios were correctly re-linked to the merged authority as well.
Yes, it works. The plan was to test with the same records on 3.20 and master + this patch set
Created attachment 47508 [details] [review] Bug 15358: Attempt to fix authorities merge This is a quick fix, and I'm quite sure there are other bugs hiding... but I managed to display both authority in tabs and merge them. The result preview requires at least one click on checkboxes to show up, I haven't looked at this issue yet. Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 47509 [details] [review] Bug 15358: Build result preview after page load Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 47510 [details] [review] Bug 15358: Authorities merge - submit only result record This patch removes parts of form that should not be submitted just before submit. The same is done on biblio merge. Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Retesting I couldn't recreate that error, seems to have been in my records, but I reloaded since then and all tests I tried worked fine.
(In reply to Nick Clemens from comment #34) > Retesting I couldn't recreate that error, seems to have been in my records, > but I reloaded since then and all tests I tried worked fine. You error was about control field (<10), did you try that?
I merged records with control fields and was able to mix and match from separate records with no issue. I tried creating subfields in control fields in MarcEdit but had no success
Created attachment 47537 [details] [review] Bug 15358: Attempt to fix authorities merge This is a quick fix, and I'm quite sure there are other bugs hiding... but I managed to display both authority in tabs and merge them. The result preview requires at least one click on checkboxes to show up, I haven't looked at this issue yet. Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 47538 [details] [review] Bug 15358: Build result preview after page load Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 47539 [details] [review] Bug 15358: Authorities merge - submit only result record This patch removes parts of form that should not be submitted just before submit. The same is done on biblio merge. Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Julian, I have found a bug when testing: I a non-repeatable subfield is added when a repeatable field is added, there is no warning. If I click on the subfield, I get "The subfield is non-repeatable and already exists in the destination record. Therefore, you cannot add it." and it is not added to the destination record.
Created attachment 47543 [details] [review] Bug 15358: Fix authorities merge Bug 8064 (Merge several biblio records) change some code used in both biblios and authorities merge tool without updating the authorities merge tool. This patch fixes that. Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
All 3 patches squashed
(In reply to Jonathan Druart from comment #40) > Julian, I have found a bug when testing: > I a non-repeatable subfield is added when a repeatable field is added, there > is no warning. > If I click on the subfield, I get "The subfield is non-repeatable and > already exists in the destination record. Therefore, you cannot add it." and > it is not added to the destination record. I think it's normal behaviour if I understand correctly. The subfield is not repeatable in its own field, but here you add a new field in the destination record, so it's valid.
(In reply to Julian Maurice from comment #43) > (In reply to Jonathan Druart from comment #40) > > Julian, I have found a bug when testing: > > I a non-repeatable subfield is added when a repeatable field is added, there > > is no warning. > > If I click on the subfield, I get "The subfield is non-repeatable and > > already exists in the destination record. Therefore, you cannot add it." and > > it is not added to the destination record. > > I think it's normal behaviour if I understand correctly. The subfield is not > repeatable in its own field, but here you add a new field in the destination > record, so it's valid. Indeed!
Pushed to Master - Should be in the May 2016 release. Thanks!
Created attachment 47640 [details] [review] [3.22.x] Bug 15358: Fix authorities merge Bug 8064 (Merge several biblio records) change some code used in both biblios and authorities merge tool without updating the authorities merge tool. This patch fixes that.
Patch for 3.22.x is slightly different from original patch. I would like to see it tested on 3.22 before I push it.
Note that it requires [3.22.x] patches from bug 15578
Created attachment 47641 [details] [review] [Signed-off] [3.22.x] Bug 15358: Fix authorities merge Bug 8064 (Merge several biblio records) change some code used in both biblios and authorities merge tool without updating the authorities merge tool. This patch fixes that. Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
To clarify (Sorry): Will this be pushed to 3.22, or just the Master? It would be nice to have the fix before 3.24 comes out for those of us with lots of authorities to clean up. If it hasn't been pushed to 3.22, would it be possible to test and push to that branch? Many thanks.
Hi Ian, it will be pushed to 3.22. I will add a comment when it's done.
Created attachment 48155 [details] [review] [3.22.x] Bug 15358: Fix authorities merge Bug 8064 (Merge several biblio records) change some code used in both biblios and authorities merge tool without updating the authorities merge tool. This patch fixes that. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Patch pushed to 3.22.x, will be in 3.22.4 Thanks Jonathan and Josef
I can't make it work properly on 3.20 branch. Code has diverged too much.
This fixed authority merging, but biblio records merging is still broken in master and stable (3.22.4 +) branches; see Bug 16373 for the follow-up.
In koha 3.22, when I merge authorities without records it works. However, if they have some one I get this error: Software error: Control fields (generally, just tags below 010) do not have subfields, use data() at /var/www/html/koha/lib/C4/AuthoritiesMarc.pm line 1523
(In reply to sepas91 from comment #56) > In koha 3.22, when I merge authorities without records it works. However, if > they have some one I get this error: > > Software error: > Control fields (generally, just tags below 010) do not have subfields, use > data() at /var/www/html/koha/lib/C4/AuthoritiesMarc.pm line 1523 Please have a look at the bugs under bug 17908. Including bug 17913. Pushed to master, but not yet to stable. So, some improvements are coming..