April 2017 - Koha-bugs - lists.koha-community.org

[Bug 9988] Leave larger authority merges to merge_authorities cronjob ( pref AuthorityMergeLimit)
by bugzilla-daemon＠bugs.koha-community.org 06 Apr '17

06 Apr '17

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9988 --- Comment #101 from Marcel de Rooy <m.de.rooy(a)rijksmuseum.nl> --- (In reply to Jacek Ablewicz from comment #100) > I have tested a bit earlier version of this patchset (from a ~week ago - not > exactly following the test plans, but focusing on the possibly "scary stuff" > which was not directly covered by the tests instead), and by a look of it > there is no need to fully retest it again. I reviewed all the changes in the > code once more (well, except for the *.t files, to be honest), and they all > look pretty solid to me - the only issue encountered in the previous try got > fixed in the meanwhile. Sign off time ;). Thanks again Jacek. -- You are receiving this mail because: You are watching all bug changes.

1 0

[Bug 14620] Contact information validations
by bugzilla-daemon＠bugs.koha-community.org 05 Apr '17

05 Apr '17

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14620 --- Comment #21 from Marc Véron <veron(a)veron.ch> --- Hi Lari Validation of phone number works fine with the exception of - Fax number - Phone number in Alternative contact (both Staff client and OPAC) -- You are receiving this mail because: You are watching all bug changes.

1 0

[Bug 9988] Leave larger authority merges to merge_authorities cronjob ( pref AuthorityMergeLimit)
by bugzilla-daemon＠bugs.koha-community.org 05 Apr '17

05 Apr '17

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9988 --- Comment #100 from Jacek Ablewicz <abl(a)biblos.pk.edu.pl> --- I have tested a bit earlier version of this patchset (from a ~week ago - not exactly following the test plans, but focusing on the possibly "scary stuff" which was not directly covered by the tests instead), and by a look of it there is no need to fully retest it again. I reviewed all the changes in the code once more (well, except for the *.t files, to be honest), and they all look pretty solid to me - the only issue encountered in the previous try got fixed in the meanwhile. Sign off time ;). -- You are receiving this mail because: You are watching all bug changes.

1 0

[Bug 15395] Allow correct handling of plural translation
by bugzilla-daemon＠bugs.koha-community.org 05 Apr '17

05 Apr '17

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15395 --- Comment #22 from Marc Véron <veron(a)veron.ch> --- I tried to test "Example usage of I18N Template::Toolkit plugin" , using the test plan from comment #7 After perl translate update fr-FR, I edited: koha/misc/translator/po/fr_FR/LC_MESSAGES/Koha.po translated the strings and changed the line: "Plural-Forms: nplurals=INTEGER; plural=EXPRESSION;\n" to: "Plural-Forms: nplurals=2; plural=(n > 1);\n" However, perl translate install fr-FR complains with following warnings: Use of uninitialized value $pref_name in concatenation (.) or string at LangInstaller.pm line 202. Use of uninitialized value $pref_name in concatenation (.) or string at LangInstaller.pm line 244. NO PLURAL FORM HEADER FOUND - DEFAULTING TO 2 print() on closed filehandle $fh at LangInstaller.pm line 689. What did I miss? -- You are receiving this mail because: You are watching all bug changes.

1 0

[Bug 9988] Leave larger authority merges to merge_authorities cronjob ( pref AuthorityMergeLimit)
by bugzilla-daemon＠bugs.koha-community.org 05 Apr '17

05 Apr '17

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9988 Jacek Ablewicz <ablewicz(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #61904|0 |1 is obsolete| | --- Comment #99 from Jacek Ablewicz <ablewicz(a)gmail.com> --- Created attachment 61925 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=61925&action=ed… Bug 9988: Add the cronjob and print an upgrade message The Debian cron file and the misc crontab example are updated. A message is printed when upgrading. Note: At this moment the merge cron job is run once a day. This is imo a good starting point. The load for this job greatly depends on the value of pref AuthorityMergeLimit. Of course you can schedule the job more often, and if this need is felt more globally, we can adjust it later. Test plan: [1] Run the dbrev and see the message. [2] Read the changes to the cron files. Signed-off-by: Marcel de Rooy <m.de.rooy(a)rijksmuseum.nl> Signed-off-by: Marc Véron <veron(a)veron.ch> Signed-off-by: Jacek Ablewicz <abl(a)biblos.pk.edu.pl> -- You are receiving this mail because: You are watching all bug changes.

1 0

[Bug 9988] Leave larger authority merges to merge_authorities cronjob ( pref AuthorityMergeLimit)
by bugzilla-daemon＠bugs.koha-community.org 05 Apr '17

05 Apr '17

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9988 Jacek Ablewicz <ablewicz(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #61903|0 |1 is obsolete| | --- Comment #98 from Jacek Ablewicz <ablewicz(a)gmail.com> --- Created attachment 61924 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=61924&action=ed… Bug 9988: Refactor the cron script The cron job is moved from migration tools to cronjobs. And renamed to a plural form. The script is now based on Koha objects. It does no longer include the code to merge one record. This can be done via the interface, and will be added to a maintenance script on bug 18071. Should not be part of this cron job. Adding a cron_cleanup method to MergeRequests; this method is called from the cron script to reset older entries still marked in progress and to also remove old processed entries. Tested in a separate unit test. Test plan: [1] Run t/db_dependent/Authorities/MergeRequests.t [2] Set AuthorityMergeLimit to 0. (All merges are postponed.) [3] Modify an authority linked to a few records. [4] Delete an authority linked to a few records with batch delete tool. [5] And select two auth records with linked records. Merge these two records with authority/merge.pl. Note: Do not select Default. See also bug 17380. [6] Check the need_merge_authorities table for inserted records. [7] Run misc/cronjobs/merge_authorities.pl -b and inspect the linked records and the record status in need_merge_authorities. Signed-off-by: Marc Véron <veron(a)veron.ch> Signed-off-by: Jacek Ablewicz <abl(a)biblos.pk.edu.pl> -- You are receiving this mail because: You are watching all bug changes.

1 0

[Bug 9988] Leave larger authority merges to merge_authorities cronjob ( pref AuthorityMergeLimit)
by bugzilla-daemon＠bugs.koha-community.org 05 Apr '17

05 Apr '17

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9988 Jacek Ablewicz <ablewicz(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #61902|0 |1 is obsolete| | --- Comment #97 from Jacek Ablewicz <ablewicz(a)gmail.com> --- Created attachment 61923 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=61923&action=ed… Bug 9988: Few subtle changes for postponed merge The fails in the previous test showed that we need the first three changes here. Some final polishing in points 4 to 6. [1] Sub merge: Refine the condition for initializing $tags_new. A postponed 'modify'-merge (A to B) makes that $authtypefrom is not defined when running merge later. When crossing the type boundary, we need a new field too. [2] Sub merge: Add condition for an empty @record_to array. This indicates also that a field should be removed, since we should otherwise only add a $9 subfield. [3] Sub merge: Adjust initializing @record_from. This change is tested by verifying a cleared subfield in the test. [4] DelAuthority: Adding a skipmerge parameter to allow the call from authorities/merge.pl to skip an unneeded merge. This also prevents that the 'delete-merge' would precede the 'modify-merge' under a hypothetical race condition. [5] DelAuthority: There is actually no need to call GetAuthority. The subfields of the old record are not relevant in this case. [6] Added a few POD lines to merge. [7] Removed a trailing space in a comment line in merge. Test plan: [1] Run t/db_dependent/Authorities/Merge.t. The last subtest should no longer fail now. [2] See test plan of next patch. Signed-off-by: Marcel de Rooy <m.de.rooy(a)rijksmuseum.nl> Signed-off-by: Marc Véron <veron(a)veron.ch> Signed-off-by: Jacek Ablewicz <abl(a)biblos.pk.edu.pl> -- You are receiving this mail because: You are watching all bug changes.

1 0

[Bug 9988] Leave larger authority merges to merge_authorities cronjob ( pref AuthorityMergeLimit)
by bugzilla-daemon＠bugs.koha-community.org 05 Apr '17

05 Apr '17

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9988 Jacek Ablewicz <ablewicz(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #61901|0 |1 is obsolete| | --- Comment #96 from Jacek Ablewicz <ablewicz(a)gmail.com> --- Created attachment 61922 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=61922&action=ed… Bug 9988: Add a subtest for specific postponed merge issues This subtest shows that we need a few little tweaks to make merge work in some specific postponed merge situations. Test plan: [1] Run t/db_dependent/Authorities/Merge.t. The last subtest should fail. Signed-off-by: Marcel de Rooy <m.de.rooy(a)rijksmuseum.nl> Signed-off-by: Marc Véron <veron(a)veron.ch> Signed-off-by: Jacek Ablewicz <abl(a)biblos.pk.edu.pl> -- You are receiving this mail because: You are watching all bug changes.

1 0

[Bug 9988] Leave larger authority merges to merge_authorities cronjob ( pref AuthorityMergeLimit)
by bugzilla-daemon＠bugs.koha-community.org 05 Apr '17

05 Apr '17

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9988 Jacek Ablewicz <ablewicz(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #61900|0 |1 is obsolete| | --- Comment #95 from Jacek Ablewicz <ablewicz(a)gmail.com> --- Created attachment 61921 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=61921&action=ed… Bug 9988: Remove further references to dontmerge [1] The preference was sent to HEA. We can now send both AuthorityMergeMode as well as AuthorityMergeLimit. [2] A comment in authorities/merge.pl is removed. Note that a subsequent patch will modify and test the cron job. [3] Script misc/batchRebuildItemsTables.pl temporarily enabled dontmerge. This is equivalent to setting the mergelimit to zero. The function defnonull is no longer needed. (If the pref was NULL, we restore that value. Sub merge won't mind.) Test plan: [1] Run t/db_dependent/UsageStats.t [2] Run misc/batchRebuildItemsTables.pl -t This just ensures you it still compiles; the changes speak for itself. [3] Now git grep on dontmerge. You should only find hits in atomicupdate and misc/translator/po. Signed-off-by: Marcel de Rooy <m.de.rooy(a)rijksmuseum.nl> Signed-off-by: Marc Véron <veron(a)veron.ch> Signed-off-by: Jacek Ablewicz <abl(a)biblos.pk.edu.pl> -- You are receiving this mail because: You are watching all bug changes.

1 0

[Bug 9988] Leave larger authority merges to merge_authorities cronjob ( pref AuthorityMergeLimit)
by bugzilla-daemon＠bugs.koha-community.org 05 Apr '17

05 Apr '17

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9988 Jacek Ablewicz <ablewicz(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #61899|0 |1 is obsolete| | --- Comment #94 from Jacek Ablewicz <ablewicz(a)gmail.com> --- Created attachment 61920 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=61920&action=ed… Bug 9988: Check the merge limit in sub merge At this point, we are replacing dontmerge functionality by the new AuthorityMergeLimit logic. Instead of doing this check before calling merge, we just call merge and check it there. In order to let the cron job do the larger (postponed) merges, we add a parameter override_limit. A subtest is added in Merge.t to test the 'postponed merge' feature. Since merge now also calls get_usage_count, an additional mock is added. All references to dontmerge are removed. In merge two lines, initializing $dbh and $counteditbiblios, are moved. The dontmerge test in DelAuthority and ModAuthority is removed. Since this did not leave much in ModAuthority, I fixed the whitespace on the remaining lines rightaway (yes, I know). A minimal set of changes is applied to the cron script; it will get further attention on a next patch. Test plan: [1] Run t/db_dependent/Authorities/Merge.t [2] Set AuthorityMergeLimit to 2. Modify an authority with two linked biblios. Check that the merge was done immediately. [3] Now modify an authority with more than 2 linked records. Verify that the merge was postponed; a record must be inserted in the need_merge_authorities table. [4] Testing of the merge cron job is *postponed* to a next patch. Note: I tested a modification, but the script just needs more attention. Signed-off-by: Marcel de Rooy <m.de.rooy(a)rijksmuseum.nl> Signed-off-by: Marc Véron <veron(a)veron.ch> Signed-off-by: Jacek Ablewicz <abl(a)biblos.pk.edu.pl> -- You are receiving this mail because: You are watching all bug changes.

1 0