|
Description
Fridolin Somers
2012-12-03 13:07:20 UTC
Created attachment 13840 [details] [review] Proposed patch I field does not exits, create a new one with subfield (or value for a control field). 8844 Created attachment 14281 [details] [review] Bug 9192: UNIMARC_sync_date_created_with_marc_biblio.pl field creation Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Created attachment 14939 [details] [review] Bug 9192: UNIMARC_sync_date_created_with_marc_biblio.pl field creation Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Signed-off-by: Mason James <mtj@kohaaloha.com> (In reply to comment #4) > Created attachment 14939 [details] [review] > Bug 9192: UNIMARC_sync_date_created_with_marc_biblio.pl field creation > > Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> > Signed-off-by: Mason James <mtj@kohaaloha.com> testing 1 commit(s) (applied to f8686e9 'Bug 9116: Remove some useless tmp var') passing QA.. patch looks good There is a problem with this patch. This patch does not check whether the specified field is a control field or variable field. Instead, to decide which field creation routine to call it checks whether the user has supplied a subfield marker. Please provide a follow-up that either identifies control fields based on the fixed fields defined in the UNIMARC standard (field number < 010, I think) or by using the is_controlfield_tag routine from MARC::Record. Created attachment 15327 [details] [review] Proposed patch (follow-up 1) Follow up to test control fields Is this what you meant Jared ? What must be done to continue ? Created attachment 20866 [details] [review] [SIGNED-OFF] Bug 9192: UNIMARC_sync_date_created_with_marc_biblio.pl field creation (followup) Add tests : if field < 10 it must not have a subfield if field > 9 is must have a subfield Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Both patches applied, no koha-qa errors. Created attachment 20867 [details] [review] Bug 9192: followup fix typo Fix typo on usage message. Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Can I please get a test plan for this? Test plan : - Launch UNIMARC_sync_date_created_with_marc_biblio.pl : - with -c 512 => Script dies with message : "date-created-marc field is greated that 009, it should have a subfield." - with -c 006a => Script dies with message : "die "date-created-marc field is lower that 010, it should not have a subfield." - with -m 512 => Script dies with message : "date-modified-marc field is greated that 009, it should have a subfield." - with -m 006a => Script dies with message : "die "date-modified-marc field is lower that 010, it should not have a subfield." QA Comment : * UNIMARC specific * no visible side effect, maintenance script * passes koha-qa.pl * tested by a French & an Argentina librarian, 2 different UNIMARC libraries passed QA Created attachment 22173 [details] [review] [PASSED QA]Bug 9192: UNIMARC_sync_date_created_with_marc_biblio.pl field creation Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Signed-off-by: Mason James <mtj@kohaaloha.com> Bug 9192: UNIMARC_sync_date_created_with_marc_biblio.pl field creation (followup) Add tests : if field < 10 it must not have a subfield if field > 9 is must have a subfield Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Both patches applied, no koha-qa errors. Bug 9192: followup fix typo Fix typo on usage message. Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Signed-off-by: Paul Poulain <paul.poulain@biblibre.com> It is strange to have a cronjob for doing that in UNIMARC. Do you know how it is done in MARC21? If it is a better way, maybe we could do a ENH for making unimarc behave the same way as MARC21? Mathieu (In reply to mathieu saby from comment #16) > It is strange to have a cronjob for doing that in UNIMARC. > Do you know how it is done in MARC21? If it is a better way, maybe we could > do a ENH for making unimarc behave the same way as MARC21? MARC21 users tend to use the 005 field, which per bug 5374 gets updated automatically whenever a bib is saved (it does that for UNIMARC as well, actually). From my POV, it would be a plausible enhancement to have the UNIMARC date add/modified fields be updated by ModBiblio(Marc). Is that plausible from the POV of UNIMARC users? I've pushed the current patch to master, BTW. Thanks, Fridolyn! (In reply to Galen Charlton from comment #18) > I've pushed the current patch to master, BTW. Thanks, Fridolyn! (In reply to Galen Charlton from comment #17) > (In reply to mathieu saby from comment #16) > > It is strange to have a cronjob for doing that in UNIMARC. > > Do you know how it is done in MARC21? If it is a better way, maybe we could > > do a ENH for making unimarc behave the same way as MARC21? > > MARC21 users tend to use the 005 field, which per bug 5374 gets updated > automatically whenever a bib is saved (it does that for UNIMARC as well, > actually). > > From my POV, it would be a plausible enhancement to have the UNIMARC date > add/modified fields be updated by ModBiblio(Marc). Is that plausible from > the POV of UNIMARC users? 005 could probably be used in unimarc, as it is made for that ;-) We tested a long time ago, but I think I remember that the plugin was just updating 005 when some value was already in 005, and not filling it at first when we create the record. (In reply to Galen Charlton from comment #18) > I've pushed the current patch to master, BTW. Thanks, Fridolyn! Thanks a lot. I know that using this script with cron once a day and not when record is created/modified is to speed up those actions. It was even more usefull when items where in MARCXML. This patch has been pushed to 3.12.x, will be in 3.12.8. Thanks Fridolin! |