Bug 27981 - Add option to automatically set the 001 control number to the biblionumber
Summary: Add option to automatically set the 001 control number to the biblionumber
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Nick Clemens
QA Contact: Marcel de Rooy
URL:
Keywords: release-notes-needed
: 9921 23832 29694 (view as bug list)
Depends on:
Blocks: 28685
  Show dependency treegraph
 
Reported: 2021-03-18 13:16 UTC by Nick Clemens
Modified: 2023-12-28 20:43 UTC (History)
12 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact: Caroline Cyr La Rose
Documentation submission: https://gitlab.com/koha-community/koha-manual/-/merge_requests/665
Text to go in the release notes:
This patch adds a new system preference: autoControlNumber The systempreference has two options, Control Number (001) is: - generated as biblionumber: will set field 001 to the biblionumber when you create a new record or edit an existing record and clear the 001 field. If a value is present in 001 when saving the record it will be preserved.When duplicating a record the pre-existing 001 will be removed. - not generated automatically: the 001 field will not be touched
Version(s) released in:
22.11.00


Attachments
Bug 27981: Add option to automatically set 001 to the biblionumber (6.86 KB, patch)
2021-03-18 13:24 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 27981: (follow-up) Set pref off for existing tests (1.38 KB, patch)
2021-03-19 11:30 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 27981: Add option to automatically set 001 to the biblionumber (6.86 KB, patch)
2021-04-16 10:22 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 27981: (follow-up) Set pref off for existing tests (1.50 KB, patch)
2021-04-16 10:22 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 27981: (follow-up) Set pref off for existing tests (1.63 KB, patch)
2021-04-16 12:05 UTC, Frank Hansen
Details | Diff | Splinter Review
Bug 27981: (follow-up) Set pref off for existing tests (1.68 KB, patch)
2021-06-10 18:11 UTC, Salman Ali
Details | Diff | Splinter Review
Bug 27981: (follow-up) Handle duplciation and remove debug (7.04 KB, patch)
2021-06-22 13:04 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 27981: (follow-up) Handle duplciation and remove debug (7.08 KB, patch)
2021-06-22 18:00 UTC, Hakam Almotlak
Details | Diff | Splinter Review
Bug 27981: Add option to automatically set 001 to the biblionumber (11.74 KB, patch)
2021-06-23 14:42 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 27981: (QA follow-up) Adjust atomic update (2.20 KB, patch)
2021-06-24 07:07 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 27981: Add option to automatically set 001 to the biblionumber (11.09 KB, patch)
2021-07-08 11:59 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 27981: (QA follow-up) Adjust atomic update (1.24 KB, patch)
2021-07-08 13:36 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 27981: Add option to automatically set 001 to the biblionumber (10.36 KB, patch)
2021-07-08 13:36 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 27981: (QA follow-up) Adjust atomic update (1.21 KB, patch)
2021-08-31 13:00 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 27981: Add option to automatically set 001 to the biblionumber (10.42 KB, patch)
2021-08-31 13:00 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 27981: Adjust imported records, svc/import_bibs, records from Z3950 (3.25 KB, patch)
2021-08-31 13:00 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 27981: DB update (1.07 KB, patch)
2021-12-03 14:58 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 27981: Add option to automatically set 001 to the biblionumber (10.42 KB, patch)
2021-12-03 14:58 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 27981: Adjust imported records, svc/import_bibs, records from Z3950 (3.25 KB, patch)
2021-12-03 14:58 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 27981: DB update (1.26 KB, patch)
2022-03-22 18:45 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 27981: Add option to automatically set 001 to the biblionumber (10.42 KB, patch)
2022-03-22 18:46 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 27981: Adjust imported records, svc/import_bibs, records from Z3950 (3.25 KB, patch)
2022-03-22 18:46 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 27981: DB update (1.31 KB, patch)
2022-05-03 13:45 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 27981: Add option to automatically set 001 to the biblionumber (10.43 KB, patch)
2022-05-03 13:45 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 27981: Adjust imported records, svc/import_bibs, records from Z3950 (3.29 KB, patch)
2022-05-03 13:46 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 27981: (follow-up) Correct variable (727 bytes, patch)
2022-05-03 13:46 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 27981: (follow-up) Fix tests (1.78 KB, patch)
2022-06-13 12:23 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 27981: DB update (1.36 KB, patch)
2022-09-02 09:43 UTC, Andrii Nugged
Details | Diff | Splinter Review
Bug 27981: Add option to automatically set 001 to the biblionumber (10.47 KB, patch)
2022-09-02 09:43 UTC, Andrii Nugged
Details | Diff | Splinter Review
Bug 27981: Adjust imported records, svc/import_bibs, records from Z3950 (3.33 KB, patch)
2022-09-02 09:44 UTC, Andrii Nugged
Details | Diff | Splinter Review
Bug 27981: (follow-up) Correct variable (775 bytes, patch)
2022-09-02 09:44 UTC, Andrii Nugged
Details | Diff | Splinter Review
Bug 27981: (follow-up) Fix tests (1.83 KB, patch)
2022-09-02 09:44 UTC, Andrii Nugged
Details | Diff | Splinter Review
Bug 27981: (QA follow-up) Unify JavaScript to false/true from 0/1/"" (2.40 KB, patch)
2022-09-02 09:44 UTC, Andrii Nugged
Details | Diff | Splinter Review
Bug 27981: (QA follow-up) GetMarcBiblio was removed from codebase (2.34 KB, patch)
2022-09-12 18:29 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2021-03-18 13:16:54 UTC
For some installations it would be nice to be able to force MARC field 001 to contain the biblionumber. This should obviously be governed by a syspref.

See bug 9921 for more discussion - there was considered option of 'incremental' but this had issues (see bug 10454)
Comment 1 Nick Clemens 2021-03-18 13:24:56 UTC
Created attachment 118461 [details] [review]
Bug 27981: Add option to automatically set 001 to the biblionumber

This patch adds a new system preference:
autoControlNumber

The option "biblionumber" will set field 001 to the biblionumber when you create a new record
or edit an existing record

If set to 'OFF' the 001 field wil not be touched

To test:
 1 - Apply patches and updatedatabase
 2 - Create a new record with no 001 field
 3 - Save and view the MAC, confirm there is no 001
 4 - Set the system preference to 'biblionumber'
 5 - Edit the record you created previously
 6 - Note the 001 is prepopulated with the biblionumber
 7 - Delete the field
 8 - Save the record
 9 - View the MARC, the 001 is filled with biblionumber
10 - Edit the record
11 - Set the 001 to a different value "Not the biblionumber"
12 - Save
13 - View the marc and confirm the value you entered is retained
14 - Edit a record with an existing 001 that is not the biblionumber
15 - Save and confirm 001 is not updated
Comment 2 Nick Clemens 2021-03-18 13:26:13 UTC
*** Bug 9921 has been marked as a duplicate of this bug. ***
Comment 3 Marcel de Rooy 2021-03-19 07:34:07 UTC
# Subtest: AddBiblio
    1..4
    not ok 1 - expected warnings when adding too long LCCN
    #   Failed test 'expected warnings when adding too long LCCN'
    #   at t/db_dependent/Biblio.t line 61.
    # found warning: Use of uninitialized value in string eq at /usr/share/koha/C4/Biblio.pm line 2630.
    # expected to find warning: (?^u:Data too long for column 'lccn')
    # expected to find warning: (?^u:Data too long for column 'lccn')
    not ok 2 - AddBiblio returns undef for biblionumber if something went wrong
    #   Failed test 'AddBiblio returns undef for biblionumber if something went wrong'
    #   at t/db_dependent/Biblio.t line 63.
    #          got: '1572'
    #     expected: undef
    not ok 3 - AddBiblio returns undef for biblioitemnumber if something went wrong
    #   Failed test 'AddBiblio returns undef for biblioitemnumber if something went wrong'
    #   at t/db_dependent/Biblio.t line 65.
    #          got: '1508'
    #     expected: undef
    not ok 4 - No biblio should have been added if something went wrong
    #   Failed test 'No biblio should have been added if something went wrong'
    #   at t/db_dependent/Biblio.t line 68.
    #          got: '1031'
    #     expected: '1030'
    # Looks like you failed 4 tests of 4.
not ok 2 - AddBiblio
#   Failed test 'AddBiblio'
#   at t/db_dependent/Biblio.t line 70.
Comment 4 Nick Clemens 2021-03-19 11:30:14 UTC
Created attachment 118513 [details] [review]
Bug 27981: (follow-up) Set pref off for existing tests
Comment 5 Nick Clemens 2021-03-19 11:33:22 UTC
I don't recreate this, can you test against master too? 
I don't see how these changes would case these failures

(In reply to Marcel de Rooy from comment #3)
> #   Failed test 'AddBiblio'
> #   at t/db_dependent/Biblio.t line 70.
Comment 6 Frank Hansen 2021-04-15 13:48:51 UTC
The patch doesn't apply:

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 27981: Add option to automatically set 001 to the biblionumber
Applying: Bug 27981: (follow-up) Set pref off for existing tests
error: sha1 information is lacking or useless (members/memberentry.pl).
error: could not build fake ancestor
Patch failed at 0001 Bug 27981: (follow-up) Set pref off for existing tests

Needs to be rebased?
Comment 7 Nick Clemens 2021-04-16 10:22:17 UTC
Created attachment 119704 [details] [review]
Bug 27981: Add option to automatically set 001 to the biblionumber

This patch adds a new system preference:
autoControlNumber

The option "biblionumber" will set field 001 to the biblionumber when you create a new record
or edit an existing record

If set to 'OFF' the 001 field wil not be touched

To test:
 1 - Apply patches and updatedatabase
 2 - Create a new record with no 001 field
 3 - Save and view the MAC, confirm there is no 001
 4 - Set the system preference to 'biblionumber'
 5 - Edit the record you created previously
 6 - Note the 001 is prepopulated with the biblionumber
 7 - Delete the field
 8 - Save the record
 9 - View the MARC, the 001 is filled with biblionumber
10 - Edit the record
11 - Set the 001 to a different value "Not the biblionumber"
12 - Save
13 - View the marc and confirm the value you entered is retained
14 - Edit a record with an existing 001 that is not the biblionumber
15 - Save and confirm 001 is not updated
Comment 8 Nick Clemens 2021-04-16 10:22:21 UTC
Created attachment 119705 [details] [review]
Bug 27981: (follow-up) Set pref off for existing tests
Comment 9 Frank Hansen 2021-04-16 12:05:53 UTC
Created attachment 119717 [details] [review]
Bug 27981: (follow-up) Set pref off for existing tests

The patches applies now. I have tested it and can confirm that it works
as intended.

Signed-off-by: Frank Hansen <frank.hansen@ub.lu.se>
Comment 10 Salman Ali 2021-06-10 18:11:48 UTC
Created attachment 121805 [details] [review]
Bug 27981: (follow-up) Set pref off for existing tests

The patches applies now. I have tested it and can confirm that it works
as intended.

Signed-off-by: Frank Hansen <frank.hansen@ub.lu.se>
Signed-off-by: Salman Ali <salman@outaouais>
Comment 11 Marcel de Rooy 2021-06-17 06:42:22 UTC
 FAIL   members/memberentry.pl
   OK     critic
   FAIL   forbidden patterns
                forbidden pattern: Data::Dumper::Dumper (line 112)
Comment 12 Marcel de Rooy 2021-06-17 06:42:58 UTC
Processing additional checks

        * Signed-off-by line 'Signed-off-by: Salman Ali <salman@outaouais>' seems invalid

This email adress does not work, Salman ;)
Comment 13 Marcel de Rooy 2021-06-17 06:44:29 UTC
Note that the Biblio.t test already fails without this patch..
Comment 14 Marcel de Rooy 2021-06-17 06:47:29 UTC
+        unless($record->field('001')){
+            $record->insert_fields_ordered(MARC::Field->new('001', $biblionumber));
+        }elsif($record->field('001')->data() eq 'biblionumber'){
+            $record->field('001')->update($biblionumber);
+        }

Please explain.
Why do you test eq 'biblionumber' ??
Why not always overwrite 001 with $biblionumber if the pref is set ?
Comment 15 Nick Clemens 2021-06-17 12:50:05 UTC
(In reply to Marcel de Rooy from comment #14)
> +        unless($record->field('001')){
> +            $record->insert_fields_ordered(MARC::Field->new('001',
> $biblionumber));
> +        }elsif($record->field('001')->data() eq 'biblionumber'){
> +            $record->field('001')->update($biblionumber);
> +        }
> 
> Please explain.
> Why do you test eq 'biblionumber' ??
> Why not always overwrite 001 with $biblionumber if the pref is set ?

Similar to autobarcode or automembernum - we will default, but if you force a new value we let it stand - I think allowing the librarian to force a different number allows for collections that fall outside the norm.

It could be useful for an OAI set for instance
Comment 16 Marcel de Rooy 2021-06-17 13:30:08 UTC
(In reply to Nick Clemens from comment #15)
> (In reply to Marcel de Rooy from comment #14)
> > +        unless($record->field('001')){
> > +            $record->insert_fields_ordered(MARC::Field->new('001',
> > $biblionumber));
> > +        }elsif($record->field('001')->data() eq 'biblionumber'){
> > +            $record->field('001')->update($biblionumber);
> > +        }
> > 
> > Please explain.
> > Why do you test eq 'biblionumber' ??
> > Why not always overwrite 001 with $biblionumber if the pref is set ?
> 
> Similar to autobarcode or automembernum - we will default, but if you force
> a new value we let it stand - I think allowing the librarian to force a
> different number allows for collections that fall outside the norm.
> 
> It could be useful for an OAI set for instance

Maybe you do not understand my point?
Read this line:
> > +        }elsif($record->field('001')->data() eq 'biblionumber'){

You are comparing with the string biblionumber !
Comment 17 Marcel de Rooy 2021-06-17 13:31:37 UTC
Might start to get what you mean btw..
Comment 18 Marcel de Rooy 2021-06-17 13:34:15 UTC
The problem with leaving a number might be when you are cloning records with Edit as new. When you do not modify that 001, it is probably wrong.
Comment 19 Marcel de Rooy 2021-06-17 13:35:28 UTC
This still needs more attention, I am afraid?
Comment 20 Marcel de Rooy 2021-06-17 13:37:10 UTC
+        }elsif($record->field('001')->data() eq 'biblionumber'){

How and when will the 001 ever be string biblionumber ?
Comment 21 Nick Clemens 2021-06-22 13:04:39 UTC
Created attachment 122280 [details] [review]
Bug 27981: (follow-up) Handle duplciation and remove debug

There were some additional lines in the commits on accident

I add a 'duplication' option to svc/bib and pass the parameter from rancor
to remove the 001 if autoControlNumber is set

I add code to addbiblio.pl to remove 001 in standard editor

To test:
1 - Follow above test plan
2 - Edit a record as duplicate when using the advanced editor
3 - Confirm the 001 does not load, but record saves correctly
4 - Edit the record
5 - Switch to 'basic editor'
6 - Save, then view record
7 - Edit as duplicate in basic editor
8 - Confirm the 001 is removed
9 - Confirm the 001 is added on save
Comment 22 Nick Clemens 2021-06-22 13:05:26 UTC
(In reply to Marcel de Rooy from comment #20)
> +        }elsif($record->field('001')->data() eq 'biblionumber'){
> 
> How and when will the 001 ever be string biblionumber ?

Either I was thinking this would be a way to force it, or I was editing my conditionals and got confused between syspref and record data. Removed in any case
Comment 23 Nick Clemens 2021-06-22 13:05:46 UTC
(In reply to Marcel de Rooy from comment #18)
> The problem with leaving a number might be when you are cloning records with
> Edit as new. When you do not modify that 001, it is probably wrong.

New patch, back to NSO
Comment 24 Hakam Almotlak 2021-06-22 18:00:38 UTC
Created attachment 122301 [details] [review]
Bug 27981: (follow-up) Handle duplciation and remove debug

There were some additional lines in the commits on accident

I add a 'duplication' option to svc/bib and pass the parameter from rancor
to remove the 001 if autoControlNumber is set

I add code to addbiblio.pl to remove 001 in standard editor

To test:
1 - Follow above test plan
2 - Edit a record as duplicate when using the advanced editor
3 - Confirm the 001 does not load, but record saves correctly
4 - Edit the record
5 - Switch to 'basic editor'
6 - Save, then view record
7 - Edit as duplicate in basic editor
8 - Confirm the 001 is removed
9 - Confirm the 001 is added on save

Signed-off-by: hakam <hakam@inlibro.com>
Comment 25 Katrin Fischer 2021-06-22 21:38:49 UTC
Hi Hakam, should this be switched to signed off? 

I notice you only reattached the last patch. Usually we will add sign-off lines to all patches and then reattach them all with them. If you need help on how to do so, please let us know.
Comment 26 Marcel de Rooy 2021-06-23 11:19:26 UTC
(In reply to Katrin Fischer from comment #25)
> Hi Hakam, should this be switched to signed off? 
> 
> I notice you only reattached the last patch. Usually we will add sign-off
> lines to all patches and then reattach them all with them. If you need help
> on how to do so, please let us know.

Hakam signed off on two patches
Comment 27 Marcel de Rooy 2021-06-23 11:20:08 UTC
(In reply to Katrin Fischer from comment #25)
> Hi Hakam, should this be switched to signed off? 
> 
> I notice you only reattached the last patch. Usually we will add sign-off
> lines to all patches and then reattach them all with them. If you need help
> on how to do so, please let us know.

Oops it is somebody else :)
Comment 28 Marcel de Rooy 2021-06-23 11:20:58 UTC
I will pickup QA again here
Comment 29 Marcel de Rooy 2021-06-23 11:29:25 UTC
# Subtest: AddBiblio
    1..5
    not ok 1 - expected warnings when adding too long LCCN
    #   Failed test 'expected warnings when adding too long LCCN'
    #   at t/db_dependent/Biblio.t line 61.
    # found warning: Use of uninitialized value in string eq at /usr/share/koha/C4/Biblio.pm line 2632.
    # expected to find warning: (?^u:Data too long for column 'lccn')
    # expected to find warning: (?^u:Data too long for column 'lccn')
    not ok 2 - AddBiblio returns undef for biblionumber if something went wrong
    #   Failed test 'AddBiblio returns undef for biblionumber if something went wrong'
    #   at t/db_dependent/Biblio.t line 63.
    #          got: '129'
    #     expected: undef
    not ok 3 - AddBiblio returns undef for biblioitemnumber if something went wrong
    #   Failed test 'AddBiblio returns undef for biblioitemnumber if something went wrong'
    #   at t/db_dependent/Biblio.t line 65.
    #          got: '129'
    #     expected: undef
    not ok 4 - No biblio should have been added if something went wrong
    #   Failed test 'No biblio should have been added if something went wrong'
    #   at t/db_dependent/Biblio.t line 68.
    #          got: '3'
    #     expected: '2'
    not ok 5 - The biblionumber is correctly passed to BiblioAutoLink
    #   Failed test 'The biblionumber is correctly passed to BiblioAutoLink'
    #   at t/db_dependent/Biblio.t line 81.
    # found warning: Use of uninitialized value in string eq at /usr/share/koha/C4/Biblio.pm line 2632.
    # found warning: My biblionumber is 130 and my frameworkcode is  at t/db_dependent/Biblio.t line 78.
    # expected to find warning: (?^u:My biblionumber is \d+ and my frameworkcode is )
    # Looks like you failed 5 tests of 5.
not ok 2 - AddBiblio
Comment 30 Marcel de Rooy 2021-06-23 11:32:27 UTC
Please cleanup your patches!

diff --git a/members/memberentry.pl b/members/memberentry.pl
index abae2ec53e..da8a324c78 100755
--- a/members/memberentry.pl
+++ b/members/memberentry.pl
@@ -109,9 +109,7 @@ $template->param( relationships => scalar $patron->guarantor_relationships ) if

 my @relations = split /\|/, C4::Context->preference('borrowerRelationship'), -1;
 @relations = ('') unless @relations;
-warn Data::Dumper::Dumper( \@relations );
 my $empty_relationship_allowed = grep {$_ eq ""} @relations;
-warn $empty_relationship_allowed;
 $template->param( empty_relationship_allowed => $empty_relationship_allowed );

 my $guarantor_id = $input->param('guarantor_id');

This really does not need to be in any of these patches.

Please amend the first patch where you delete the buggy lines too ;)
Comment 31 Nick Clemens 2021-06-23 14:42:45 UTC
Created attachment 122344 [details] [review]
Bug 27981: Add option to automatically set 001 to the biblionumber

This patch adds a new system preference:
autoControlNumber

The option "biblionumber" will set field 001 to the biblionumber when you create a new record
or edit an existing record

If set to 'OFF' the 001 field wil not be touched

When duplicating a record the 001 will be removed if autoControlNumber is set
A 'duplication' option is add to svc/bib and rancor passes the parameter,
the basic editor simply removes the field

To test:
 1 - Apply patches and updatedatabase
 2 - Create a new record with no 001 field
 3 - Save and view the MAC, confirm there is no 001
 4 - Set the system preference to 'biblionumber'
 5 - Edit the record you created previously
 6 - Note the 001 is prepopulated with the biblionumber
 7 - Delete the field
 8 - Save the record
 9 - View the MARC, the 001 is filled with biblionumber
10 - Edit the record
11 - Set the 001 to a different value "Not the biblionumber"
12 - Save
13 - View the marc and confirm the value you entered is retained
14 - Edit a record with an existing 001 that is not the biblionumber
15 - Save and confirm 001 is not updated

To testi duplication:
1 - Edit a record as duplicate when using the advanced editor
2 - Confirm the 001 does not load, but record saves correctly
3 - Edit the record
4 - Switch to 'basic editor'
5 - Save, then view record
6 - Edit as duplicate in basic editor
7 - Confirm the 001 is removed
8 - Confirm the 001 is added on save
Comment 32 Nick Clemens 2021-06-23 14:45:25 UTC
(In reply to Marcel de Rooy from comment #30)
> Please cleanup your patches!
> Please amend the first patch where you delete the buggy lines too ;)

Patches squashed, back to NSO


(In reply to Marcel de Rooy from comment #29)

>     # found warning: Use of uninitialized value in string eq at
> /usr/share/koha/C4/Biblio.pm line 2632.
>     # found warning: My biblionumber is 130 and my frameworkcode is  at
> t/db_dependent/Biblio.t line 78.
>     # expected to find warning: (?^u:My biblionumber is \d+ and my
> frameworkcode is )
>     # Looks like you failed 5 tests of 5.
> not ok 2 - AddBiblio

I get the extra warning if i run the test before the DB update, but all is good after
Comment 33 Marcel de Rooy 2021-06-24 07:07:25 UTC
Created attachment 122356 [details] [review]
Bug 27981: (QA follow-up) Adjust atomic update

[1] Insert ignore.
[2] Remove references to bug 9921.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 34 Marcel de Rooy 2021-06-24 07:08:40 UTC
(In reply to Nick Clemens from comment #32)
> I get the extra warning if i run the test before the DB update, but all is
> good after

Still have it.
Comment 35 Marcel de Rooy 2021-06-24 07:13:53 UTC
> A 'duplication' option is add to svc/bib and rancor passes the parameter,
> the basic editor simply removes the field

I dont think that we should go that way. The changes to svc/bib do not look good to me. The strange d parameter in the path svc/bib/d1 is rather hacky.

Personally, I would rather go away from the whole path_info tric to get the biblionumber and just have a real biblionumber parameter. But we should not spend too much time on this old service script when we have an API.

Instead of changing svc/bib, could you move the change to where rancor saves a duplicated record?

Adding Jonathan to the discussion.
Comment 36 Marcel de Rooy 2021-06-24 07:48:29 UTC
(In reply to Marcel de Rooy from comment #34)
> (In reply to Nick Clemens from comment #32)
> > I get the extra warning if i run the test before the DB update, but all is
> > good after
> 
> Still have it.

Forget this one: strict sql mode stuff..
Comment 37 Jonathan Druart 2021-06-25 09:09:19 UTC
Yes, that's definitely hacky and it will complicate the move when we will replace svc/bib with a REST API call.
Comment 38 Marcel de Rooy 2021-06-25 09:55:10 UTC
Nick: There is also another point still. If we update 001, we should update the orgcode too in 003.
See also bug 28491 for authority counterpart.
Comment 39 Nick Clemens 2021-07-08 11:59:47 UTC
Created attachment 122672 [details] [review]
Bug 27981: Add option to automatically set 001 to the biblionumber

This patch adds a new system preference:
autoControlNumber

The option "biblionumber" will set field 001 to the biblionumber when you create a new record
or edit an existing record

If set to 'OFF' the 001 field wil not be touched

When duplicating a record the 001 will be removed if autoControlNumber is set

To test:
 1 - Apply patches and updatedatabase
 2 - Create a new record with no 001 field
 3 - Save and view the MAC, confirm there is no 001
 4 - Set the system preference to 'biblionumber'
 5 - Edit the record you created previously
 6 - Note the 001 is prepopulated with the biblionumber
 7 - Delete the field
 8 - Save the record
 9 - View the MARC, the 001 is filled with biblionumber
10 - Edit the record
11 - Set the 001 to a different value "Not the biblionumber"
12 - Save
13 - View the marc and confirm the value you entered is retained
14 - Edit a record with an existing 001 that is not the biblionumber
15 - Save and confirm 001 is not updated

To test duplication:
1 - Edit a record as duplicate when using the advanced editor
2 - Confirm the 001 does not load, but record saves correctly
3 - Edit the record
4 - Switch to 'basic editor'
5 - Save, then view record
6 - Edit as duplicate in basic editor
7 - Confirm the 001 is removed
8 - Confirm the 001 is added on save
Comment 40 Nick Clemens 2021-07-08 12:01:52 UTC
(In reply to Marcel de Rooy from comment #38)
> Nick: There is also another point still. If we update 001, we should update
> the orgcode too in 003.
> See also bug 28491 for authority counterpart.

adding orgcode feels like scope creep - file another bug?
Comment 41 Marcel de Rooy 2021-07-08 13:28:01 UTC
(In reply to Nick Clemens from comment #40)
> (In reply to Marcel de Rooy from comment #38)
> > Nick: There is also another point still. If we update 001, we should update
> > the orgcode too in 003.
> > See also bug 28491 for authority counterpart.
> 
> adding orgcode feels like scope creep - file another bug?

Done. See also
Comment 42 Marcel de Rooy 2021-07-08 13:35:16 UTC
Commit promises:
The option "biblionumber" will set field 001 to the biblionumber when you create a new record     or edit an existing record

Create works, modify does not.
Comment 43 Marcel de Rooy 2021-07-08 13:36:35 UTC
Created attachment 122674 [details] [review]
Bug 27981: (QA follow-up) Adjust atomic update

[1] Insert ignore.
[2] Remove references to bug 9921.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 44 Marcel de Rooy 2021-07-08 13:36:39 UTC
Created attachment 122675 [details] [review]
Bug 27981: Add option to automatically set 001 to the biblionumber

This patch adds a new system preference:
autoControlNumber

The option "biblionumber" will set field 001 to the biblionumber when you create a new record
or edit an existing record

If set to 'OFF' the 001 field wil not be touched

When duplicating a record the 001 will be removed if autoControlNumber is set

To test:
 1 - Apply patches and updatedatabase
 2 - Create a new record with no 001 field
 3 - Save and view the MAC, confirm there is no 001
 4 - Set the system preference to 'biblionumber'
 5 - Edit the record you created previously
 6 - Note the 001 is prepopulated with the biblionumber
 7 - Delete the field
 8 - Save the record
 9 - View the MARC, the 001 is filled with biblionumber
10 - Edit the record
11 - Set the 001 to a different value "Not the biblionumber"
12 - Save
13 - View the marc and confirm the value you entered is retained
14 - Edit a record with an existing 001 that is not the biblionumber
15 - Save and confirm 001 is not updated

To test duplication:
1 - Edit a record as duplicate when using the advanced editor
2 - Confirm the 001 does not load, but record saves correctly
3 - Edit the record
4 - Switch to 'basic editor'
5 - Save, then view record
6 - Edit as duplicate in basic editor
7 - Confirm the 001 is removed
8 - Confirm the 001 is added on save

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 45 Marcel de Rooy 2021-07-08 13:37:20 UTC
(In reply to Marcel de Rooy from comment #42)
> Commit promises:
> The option "biblionumber" will set field 001 to the biblionumber when you
> create a new record     or edit an existing record
> 
> Create works, modify does not.

And it contradicts the test plan:
    14 - Edit a record with an existing 001 that is not the biblionumber
    15 - Save and confirm 001 is not updated
Comment 46 David Cook 2021-08-04 03:05:49 UTC
Thanks for working on this, Nick. This is a feature I've long wanted in Koha.

I've just taken a quick peek at the code, so just going to ask for some clarification.

At a glance, it looks like a 001 will be assigned for original cataloguing or copy cataloguing using the Cataloguing UI (even if it already has a 001), is that right? That sounds good to me.

I wonder about Staged MARC Imports though. At a glance, I think those will retain their original 001 and won't get updated. I think that might be problematic. 

Same goes for the APIs.
Comment 47 Nick Clemens 2021-08-31 13:00:35 UTC
Created attachment 124279 [details] [review]
Bug 27981: (QA follow-up) Adjust atomic update

[1] Insert ignore.
[2] Remove references to bug 9921.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 48 Nick Clemens 2021-08-31 13:00:39 UTC
Created attachment 124280 [details] [review]
Bug 27981: Add option to automatically set 001 to the biblionumber

This patch adds a new system preference:
autoControlNumber

The option "biblionumber" will set field 001 to the biblionumber when you create a new record
or edit an existing record and clear the 001 field.

If a value is present in 001 when saving the record it will be preserved.

If set to 'OFF' the 001 field wil not be touched

When duplicating a record the 001 will be removed if autoControlNumber is set

To test:
 1 - Apply patches and updatedatabase
 2 - Create a new record with no 001 field
 3 - Save and view the MAC, confirm there is no 001
 4 - Set the system preference to 'biblionumber'
 5 - Edit the record you created previously
 6 - Note the 001 is prepopulated with the biblionumber
 7 - Delete the field
 8 - Save the record
 9 - View the MARC, the 001 is filled with biblionumber
10 - Edit the record
11 - Set the 001 to a different value "Not the biblionumber"
12 - Save
13 - View the marc and confirm the value you entered is retained
14 - Edit a record with an existing 001 that is not the biblionumber
15 - Save and confirm 001 is not updated

To test duplication:
1 - Edit a record as duplicate when using the advanced editor
2 - Confirm the 001 does not load, but record saves correctly
3 - Edit the record
4 - Switch to 'basic editor'
5 - Save, then view record
6 - Edit as duplicate in basic editor
7 - Confirm the 001 is removed
8 - Confirm the 001 is added on save

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 49 Nick Clemens 2021-08-31 13:00:44 UTC
Created attachment 124281 [details] [review]
Bug 27981: Adjust imported records, svc/import_bibs, records from Z3950
Comment 50 Nick Clemens 2021-08-31 13:01:04 UTC
(In reply to Marcel de Rooy from comment #45)
> (In reply to Marcel de Rooy from comment #42)
> > Commit promises:
> > The option "biblionumber" will set field 001 to the biblionumber when you
> > create a new record     or edit an existing record
> > 
> > Create works, modify does not.
> 
> And it contradicts the test plan:
>     14 - Edit a record with an existing 001 that is not the biblionumber
>     15 - Save and confirm 001 is not updated

Updated the commit message
Comment 51 Nick Clemens 2021-08-31 13:01:30 UTC
(In reply to David Cook from comment #46)
> Thanks for working on this, Nick. This is a feature I've long wanted in Koha.
> 
> I've just taken a quick peek at the code, so just going to ask for some
> clarification.
> 
> At a glance, it looks like a 001 will be assigned for original cataloguing
> or copy cataloguing using the Cataloguing UI (even if it already has a 001),
> is that right? That sounds good to me.
> 
> I wonder about Staged MARC Imports though. At a glance, I think those will
> retain their original 001 and won't get updated. I think that might be
> problematic. 
> 
> Same goes for the APIs.

Updated importing and svc/import_bibs
Comment 52 Marcel de Rooy 2021-08-31 14:31:55 UTC
I think this needs to be Needs Signoff.
If someone signs off on the last patch, I probably could do the QA again ;)
Comment 53 Katrin Fischer 2021-12-01 10:28:32 UTC
Is the sequence of the attached patch files correct? (QA follow-up on top)
Comment 54 Nick Clemens 2021-12-03 14:58:21 UTC
Created attachment 128223 [details] [review]
Bug 27981: DB update

Current status: Needs Signoff
Comment 55 Nick Clemens 2021-12-03 14:58:25 UTC
Created attachment 128224 [details] [review]
Bug 27981: Add option to automatically set 001 to the biblionumber

This patch adds a new system preference:
autoControlNumber

The option "biblionumber" will set field 001 to the biblionumber when you create a new record
or edit an existing record and clear the 001 field.

If a value is present in 001 when saving the record it will be preserved.

If set to 'OFF' the 001 field wil not be touched

When duplicating a record the 001 will be removed if autoControlNumber is set

To test:
 1 - Apply patches and updatedatabase
 2 - Create a new record with no 001 field
 3 - Save and view the MAC, confirm there is no 001
 4 - Set the system preference to 'biblionumber'
 5 - Edit the record you created previously
 6 - Note the 001 is prepopulated with the biblionumber
 7 - Delete the field
 8 - Save the record
 9 - View the MARC, the 001 is filled with biblionumber
10 - Edit the record
11 - Set the 001 to a different value "Not the biblionumber"
12 - Save
13 - View the marc and confirm the value you entered is retained
14 - Edit a record with an existing 001 that is not the biblionumber
15 - Save and confirm 001 is not updated

To test duplication:
1 - Edit a record as duplicate when using the advanced editor
2 - Confirm the 001 does not load, but record saves correctly
3 - Edit the record
4 - Switch to 'basic editor'
5 - Save, then view record
6 - Edit as duplicate in basic editor
7 - Confirm the 001 is removed
8 - Confirm the 001 is added on save

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 56 Nick Clemens 2021-12-03 14:58:30 UTC
Created attachment 128225 [details] [review]
Bug 27981: Adjust imported records, svc/import_bibs, records from Z3950
Comment 57 Nick Clemens 2022-03-22 18:45:58 UTC
Created attachment 132034 [details] [review]
Bug 27981: DB update
Comment 58 Nick Clemens 2022-03-22 18:46:02 UTC
Created attachment 132035 [details] [review]
Bug 27981: Add option to automatically set 001 to the biblionumber

This patch adds a new system preference:
autoControlNumber

The option "biblionumber" will set field 001 to the biblionumber when you create a new record
or edit an existing record and clear the 001 field.

If a value is present in 001 when saving the record it will be preserved.

If set to 'OFF' the 001 field wil not be touched

When duplicating a record the 001 will be removed if autoControlNumber is set

To test:
 1 - Apply patches and updatedatabase
 2 - Create a new record with no 001 field
 3 - Save and view the MAC, confirm there is no 001
 4 - Set the system preference to 'biblionumber'
 5 - Edit the record you created previously
 6 - Note the 001 is prepopulated with the biblionumber
 7 - Delete the field
 8 - Save the record
 9 - View the MARC, the 001 is filled with biblionumber
10 - Edit the record
11 - Set the 001 to a different value "Not the biblionumber"
12 - Save
13 - View the marc and confirm the value you entered is retained
14 - Edit a record with an existing 001 that is not the biblionumber
15 - Save and confirm 001 is not updated

To test duplication:
1 - Edit a record as duplicate when using the advanced editor
2 - Confirm the 001 does not load, but record saves correctly
3 - Edit the record
4 - Switch to 'basic editor'
5 - Save, then view record
6 - Edit as duplicate in basic editor
7 - Confirm the 001 is removed
8 - Confirm the 001 is added on save

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 59 Nick Clemens 2022-03-22 18:46:06 UTC
Created attachment 132036 [details] [review]
Bug 27981: Adjust imported records, svc/import_bibs, records from Z3950
Comment 60 Thomas Klausner 2022-05-03 13:06:54 UTC
I tested this feature, everything works as expected / documented, but I (currently) don't have time to set up a proper sign-off workflow ...
Comment 61 Nick Clemens 2022-05-03 13:45:50 UTC
Created attachment 134536 [details] [review]
Bug 27981: DB update

Signed-off-by: Thomas Klausner <domm@plix.at>
Comment 62 Nick Clemens 2022-05-03 13:45:55 UTC
Created attachment 134537 [details] [review]
Bug 27981: Add option to automatically set 001 to the biblionumber

This patch adds a new system preference:
autoControlNumber

The option "biblionumber" will set field 001 to the biblionumber when you create a new record
or edit an existing record and clear the 001 field.

If a value is present in 001 when saving the record it will be preserved.

If set to 'OFF' the 001 field wil not be touched

When duplicating a record the 001 will be removed if autoControlNumber is set

To test:
 1 - Apply patches and updatedatabase
 2 - Create a new record with no 001 field
 3 - Save and view the MAC, confirm there is no 001
 4 - Set the system preference to 'biblionumber'
 5 - Edit the record you created previously
 6 - Note the 001 is prepopulated with the biblionumber
 7 - Delete the field
 8 - Save the record
 9 - View the MARC, the 001 is filled with biblionumber
10 - Edit the record
11 - Set the 001 to a different value "Not the biblionumber"
12 - Save
13 - View the marc and confirm the value you entered is retained
14 - Edit a record with an existing 001 that is not the biblionumber
15 - Save and confirm 001 is not updated

To test duplication:
1 - Edit a record as duplicate when using the advanced editor
2 - Confirm the 001 does not load, but record saves correctly
3 - Edit the record
4 - Switch to 'basic editor'
5 - Save, then view record
6 - Edit as duplicate in basic editor
7 - Confirm the 001 is removed
8 - Confirm the 001 is added on save

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Thomas Klausner <domm@plix.at>
Comment 63 Nick Clemens 2022-05-03 13:46:00 UTC
Created attachment 134538 [details] [review]
Bug 27981: Adjust imported records, svc/import_bibs, records from Z3950

Signed-off-by: Thomas Klausner <domm@plix.at>
Comment 64 Nick Clemens 2022-05-03 13:46:05 UTC
Created attachment 134539 [details] [review]
Bug 27981: (follow-up) Correct variable
Comment 65 Katrin Fischer 2022-05-03 15:53:51 UTC
Thx Nick for adding the sign-off lines :)
Comment 66 Katrin Fischer 2022-06-12 11:49:45 UTC
This is a really interesting dev I've been often asked about. But: Tests are currently failing:

prove t/db_dependent/Biblio.t
t/db_dependent/Biblio.t .. 8/15     # Looks like you planned 3 tests but ran 1.

#   Failed test 'autoControlNumber tests'
#   at t/db_dependent/Biblio.t line 846.
Can't use string ("1") as a HASH ref while "strict refs" in use at /kohadevbox/koha/C4/Biblio.pm line 362.
# Looks like your test exited with 255 just after 15.
t/db_dependent/Biblio.t .. Dubious, test returned 255 (wstat 65280, 0xff00)
Failed 1/15 subtests 

Test Summary Report
-------------------
t/db_dependent/Biblio.t (Wstat: 65280 Tests: 15 Failed: 1)
  Failed test:  15
  Non-zero exit status: 255
Comment 67 Nick Clemens 2022-06-13 12:23:24 UTC
Created attachment 136022 [details] [review]
Bug 27981: (follow-up) Fix tests
Comment 68 Katrin Fischer 2022-07-16 07:58:01 UTC
*** Bug 23832 has been marked as a duplicate of this bug. ***
Comment 69 Andrii Nugged 2022-09-02 09:43:50 UTC
Created attachment 140092 [details] [review]
Bug 27981: DB update

Signed-off-by: Thomas Klausner <domm@plix.at>

Signed-off-by: Andrew Nugged <nugged@gmail.com>
Comment 70 Andrii Nugged 2022-09-02 09:43:55 UTC
Created attachment 140093 [details] [review]
Bug 27981: Add option to automatically set 001 to the biblionumber

This patch adds a new system preference:
autoControlNumber

The option "biblionumber" will set field 001 to the biblionumber when you create a new record
or edit an existing record and clear the 001 field.

If a value is present in 001 when saving the record it will be preserved.

If set to 'OFF' the 001 field wil not be touched

When duplicating a record the 001 will be removed if autoControlNumber is set

To test:
 1 - Apply patches and updatedatabase
 2 - Create a new record with no 001 field
 3 - Save and view the MAC, confirm there is no 001
 4 - Set the system preference to 'biblionumber'
 5 - Edit the record you created previously
 6 - Note the 001 is prepopulated with the biblionumber
 7 - Delete the field
 8 - Save the record
 9 - View the MARC, the 001 is filled with biblionumber
10 - Edit the record
11 - Set the 001 to a different value "Not the biblionumber"
12 - Save
13 - View the marc and confirm the value you entered is retained
14 - Edit a record with an existing 001 that is not the biblionumber
15 - Save and confirm 001 is not updated

To test duplication:
1 - Edit a record as duplicate when using the advanced editor
2 - Confirm the 001 does not load, but record saves correctly
3 - Edit the record
4 - Switch to 'basic editor'
5 - Save, then view record
6 - Edit as duplicate in basic editor
7 - Confirm the 001 is removed
8 - Confirm the 001 is added on save

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Thomas Klausner <domm@plix.at>

Signed-off-by: Andrew Nugged <nugged@gmail.com>
Comment 71 Andrii Nugged 2022-09-02 09:44:00 UTC
Created attachment 140094 [details] [review]
Bug 27981: Adjust imported records, svc/import_bibs, records from Z3950

Signed-off-by: Thomas Klausner <domm@plix.at>

Signed-off-by: Andrew Nugged <nugged@gmail.com>
Comment 72 Andrii Nugged 2022-09-02 09:44:05 UTC
Created attachment 140095 [details] [review]
Bug 27981: (follow-up) Correct variable

Signed-off-by: Andrew Nugged <nugged@gmail.com>
Comment 73 Andrii Nugged 2022-09-02 09:44:10 UTC
Created attachment 140096 [details] [review]
Bug 27981: (follow-up) Fix tests

Signed-off-by: Andrew Nugged <nugged@gmail.com>
Comment 74 Andrii Nugged 2022-09-02 09:44:14 UTC
Created attachment 140097 [details] [review]
Bug 27981: (QA follow-up) Unify JavaScript to false/true from 0/1/""

Make remove_control_num var and JS KohaBackend.GetRecord calls to have
only false/true instead of mixed 0,1, and "" in different places.

Signed-off-by: Andrew Nugged <nugged@gmail.com>
Comment 75 Andrii Nugged 2022-09-02 09:49:42 UTC
Did QA & Rebased to the latest master.

Also added 1 final patch (small polishing step) to use true/false everywhere to unify JS because,

    KohaBackend.GetRecord( id, remove_control_num, callback );

called in one place with remove_control_num to be 0 or 1,
and in another place:

    KohaBackend.GetRecord( id, "", callback );

let's have it true/false, so the next developer will get from the context that this variable is only boolean.
Comment 76 Andrii Nugged 2022-09-02 09:56:43 UTC
This bugfix code (including my last patch) was already used in our productions for more than a month as a hotfix, and works well.
Comment 77 Andrii Nugged 2022-09-02 09:59:26 UTC
Future development note, but this is not in the scope of the CURRENT TICKET:

Our librarians also noted that there's a 003 field which might also be auto-filled with some hardcoded value for this autofilled 001. They also noted that "However, it should be the case that the combination of 001 and 003 is always unique.").
Comment 78 Tomás Cohen Arazi 2022-09-12 18:29:42 UTC
Created attachment 140502 [details] [review]
Bug 27981: (QA follow-up) GetMarcBiblio was removed from codebase

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 79 Tomás Cohen Arazi 2022-09-12 18:37:00 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 80 Katrin Fischer 2022-10-24 21:06:04 UTC
*** Bug 29694 has been marked as a duplicate of this bug. ***