Bug 19349 - Allow to store biblio record's creator and last modifier in MARC
Summary: Allow to store biblio record's creator and last modifier in MARC
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Julian Maurice
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks: 24906
  Show dependency treegraph
 
Reported: 2017-09-20 13:17 UTC by Julian Maurice
Modified: 2020-05-23 11:01 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 19349: Store record's creator and last modifier in record (9.91 KB, patch)
2017-09-20 13:20 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 19349: Store record's creator and last modifier in record (9.96 KB, patch)
2017-10-20 12:43 UTC, Simon Pouchol
Details | Diff | Splinter Review
Bug 19349: Store record's creator and last modifier in record (11.69 KB, patch)
2018-09-14 08:58 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 19349: Store record's creator and last modifier in record (11.77 KB, patch)
2018-11-02 10:21 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 19349: (QA follow-up) Cosmetic changes (2.74 KB, patch)
2018-11-02 10:21 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Julian Maurice 2017-09-20 13:17:15 UTC
This will allow to store borrowernumber and name of biblio record's creator and last modifier in the MARC record.
Comment 1 Julian Maurice 2017-09-20 13:20:03 UTC
Created attachment 67233 [details] [review]
Bug 19349: Store record's creator and last modifier in record

Test plan:
1. Run updatedatabase.pl
2. Set sysprefs MarcFieldForCreatorId, MarcFieldForCreatorName,
   MarcFieldForModifierId, MarcFieldForModifierName
3. Create a new biblio
4. Verify that the fields are correctly filled
5. Logout and login as another user
6. Modify the same biblio
7. Verify that only the fields for last modifier have been modified
Comment 2 Alex Buckley 2017-10-03 20:33:57 UTC
Hi Julian

I have set the four sysprefs with the values 942$w, 942$x, 942$y, and 942$z however when I go to create a new biblio these four fields are not displayed in the '9' tab of the new biblio page
Comment 3 Julian Maurice 2017-10-04 07:25:51 UTC
(In reply to Alex Buckley from comment #2)
> Hi Julian
> 
> I have set the four sysprefs with the values 942$w, 942$x, 942$y, and 942$z
> however when I go to create a new biblio these four fields are not displayed
> in the '9' tab of the new biblio page

Hi Alex,

you need to create these subfields in the MARC framework you are using, or make them visible if they already exist.

Note that the subfields are filled after you click on 'Save', so their values are not visible in the MARC editor when you create a new biblio.
Comment 4 Simon Pouchol 2017-10-20 12:43:47 UTC
Created attachment 68317 [details] [review]
Bug 19349: Store record's creator and last modifier in record

Test plan:
1. Run updatedatabase.pl
2. Set sysprefs MarcFieldForCreatorId, MarcFieldForCreatorName,
   MarcFieldForModifierId, MarcFieldForModifierName
3. Create a new biblio
4. Verify that the fields are correctly filled
5. Logout and login as another user
6. Modify the same biblio
7. Verify that only the fields for last modifier have been modified

Works perfectly.

Signed-off-by: Simon Pouchol <simon.pouchol@biblibre.com>
Comment 5 Marcel de Rooy 2017-10-23 13:16:57 UTC
Julian, just a few preliminary comments:

-    # FIXME To replace with ->find_or_create?
-    if ( my $m_rs = Koha::Biblio::Metadatas->find($metadata) ) {
-        $m_rs->metadata( $record->as_xml_record($encoding) );
-        $m_rs->store;
-    } else {
-        my $m_rs = Koha::Biblio::Metadata->new($metadata);
-        $m_rs->metadata( $record->as_xml_record($encoding) );
-        $m_rs->store;
+
+    my $m_rs = Koha::Biblio::Metadatas->find($metadata);
+    unless ($m_rs) {
+        $m_rs = Koha::Biblio::Metadata->new($metadata);
+    }
Why did you not apply what is in the FIXME ?

You add a new sub in C4/Biblio for MARC manipulation. Actually something like create or update. Should we not be looking now how to do such things in the Koha namespace instead of extending C4?

You add four prefs. Would it be handy to combine them. And test on its value before saving?
Comment 6 Marcel de Rooy 2017-11-03 09:04:54 UTC
Any feedback please ?
Comment 7 Julian Maurice 2018-09-14 08:58:51 UTC
Created attachment 78639 [details] [review]
Bug 19349: Store record's creator and last modifier in record

Test plan:
1. Run updatedatabase.pl
2. Set sysprefs MarcFieldForCreatorId, MarcFieldForCreatorName,
   MarcFieldForModifierId, MarcFieldForModifierName
3. Create a new biblio
4. Verify that the fields are correctly filled
5. Logout and login as another user
6. Modify the same biblio
7. Verify that only the fields for last modifier have been modified

Works perfectly.

Signed-off-by: Simon Pouchol <simon.pouchol@biblibre.com>
Comment 8 Julian Maurice 2018-09-14 09:02:52 UTC
(In reply to Marcel de Rooy from comment #5)
> Why did you not apply what is in the FIXME ?
Because find_or_create is not what I want. find_or_new would be nice but it's
not implemented in Koha::Objects

> You add a new sub in C4/Biblio for MARC manipulation. Actually something
> like create or update. Should we not be looking now how to do such things in
> the Koha namespace instead of extending C4?
You're right. I moved set_koha_field to Koha::Util::MARC

> You add four prefs. Would it be handy to combine them.
How do you suggest to combine them ?

> And test on its value before saving?
Is it possible to check sysprefs value before saving ?
Comment 9 paxed 2018-09-28 06:11:25 UTC
Wouldn't the Koha to MARC mapping fields work for these, instead of creating separate sysprefs for each?
Comment 10 Katrin Fischer 2018-09-28 06:20:26 UTC
It won't right now because there are no columns added to any of the tables.(In reply to paxed from comment #9)
> Wouldn't the Koha to MARC mapping fields work for these, instead of creating
> separate sysprefs for each?

Not right now as there are no matching columns in the tables.

Although we have something similar already, some placeholders that can be used in the default values of the frameworks (https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7045). Maybe instead of adding new prefs this could be extended? Then the configuration could happen in the frameworks. But needs better documentation.
Comment 11 Marcel de Rooy 2018-11-02 09:42:40 UTC
QA: Looking here
Comment 12 Marcel de Rooy 2018-11-02 09:49:37 UTC
Put 990a in the field. (Yeah should have been 990$a but is not listed on the preferences form..) Internal server error.
Tag "990a" is not a valid tag. at /usr/share/koha/devclone/Koha/Util/MARC.pm line 218.
Comment 13 Marcel de Rooy 2018-11-02 09:55:47 UTC
$record->append_fields($field);

Should we actually append or insert_ordered ?
Comment 14 Marcel de Rooy 2018-11-02 10:21:38 UTC
Created attachment 81873 [details] [review]
Bug 19349: Store record's creator and last modifier in record

Test plan:
1. Run updatedatabase.pl
2. Set sysprefs MarcFieldForCreatorId, MarcFieldForCreatorName,
   MarcFieldForModifierId, MarcFieldForModifierName
3. Create a new biblio
4. Verify that the fields are correctly filled
5. Logout and login as another user
6. Modify the same biblio
7. Verify that only the fields for last modifier have been modified

Works perfectly.

Signed-off-by: Simon Pouchol <simon.pouchol@biblibre.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 15 Marcel de Rooy 2018-11-02 10:21:43 UTC
Created attachment 81874 [details] [review]
Bug 19349: (QA follow-up) Cosmetic changes

[1] Construction with a // b instead of a; unless( a ) b;
[2] Error checking on subfieldCode
[3] Add explanation how to fill preference

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 16 Marcel de Rooy 2018-11-02 10:23:23 UTC
Passed QA
If you dont want this in your record, simply do not add something in these prefs. No harm done.
Comment 17 Nick Clemens 2018-11-08 21:10:05 UTC
Awesome work all!

Pushed to master for 18.11
Comment 18 Martin Renvoize 2018-11-16 14:03:10 UTC
Enhancement, will not be backported to 18.05.x series.