Bug 6299 - Provide a list of authorized values for relator terms
Summary: Provide a list of authorized values for relator terms
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: master
Hardware: All All
: P3 enhancement (vote)
Assignee: Tomás Cohen Arazi
QA Contact: Bugs List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-05-04 15:25 UTC by Tomás Cohen Arazi
Modified: 2013-12-05 20:04 UTC (History)
8 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Initial patch (170.64 KB, patch)
2011-06-07 14:30 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Fix category name lenght (166.27 KB, patch)
2011-07-07 17:31 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Complete list, complete strings (171.12 KB, patch)
2011-08-04 20:51 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
advertise availability in updatedatabase.pl (1.15 KB, patch)
2011-08-04 20:53 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Screenshot of updatedb page (29.39 KB, image/png)
2011-08-05 08:49 UTC, Julian Maurice
Details
advertise availability in updatedatabase.pl (single line) (1.07 KB, patch)
2011-08-08 14:47 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Signed patch (complete strings) (170.89 KB, patch)
2011-08-10 13:49 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Signed patch (updatedatabase advertisement) (1.18 KB, patch)
2011-08-10 13:50 UTC, Marcel de Rooy
Details | Diff | Splinter Review
es-ES was missing (24.77 KB, patch)
2011-08-10 14:48 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Signed follow up patch for Spanish (24.89 KB, patch)
2011-08-11 07:09 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 6299 - add es-ES (24.89 KB, patch)
2011-10-06 12:56 UTC, Paul Poulain
Details | Diff | Splinter Review
Bug 6299 - Advertise this addition during upgrade (1.59 KB, patch)
2011-10-06 12:57 UTC, Paul Poulain
Details | Diff | Splinter Review
Bug 6299 - Provide a list of authorized values for Relator terms (24.89 KB, patch)
2012-01-28 15:44 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 6299 - Advertise this addition during upgrade (1.84 KB, patch)
2012-01-28 15:44 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 6299 - Provide a list of authorized values for Relator terms (24.89 KB, patch)
2012-01-28 15:46 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 6299 - Advertise this addition during upgrade (1.84 KB, patch)
2012-01-28 15:46 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 6299 - Advertise this addition during upgrade (1.79 KB, patch)
2012-01-29 13:41 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Relator codes in French (23.49 KB, patch)
2012-01-30 18:16 UTC, Frédéric Demians
Details | Diff | Splinter Review
Relator codes in French (23.50 KB, patch)
2012-02-01 11:17 UTC, Frédéric Demians
Details | Diff | Splinter Review
Relator codes in French (23.50 KB, patch)
2012-02-07 18:53 UTC, Frédéric Demians
Details | Diff | Splinter Review
Bug 6299 Add relator codes for fr-FR (24.68 KB, patch)
2012-02-08 12:02 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 6299 - add es-ES (25.67 KB, patch)
2012-02-08 14:20 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 6299 - add es-ES (26.85 KB, patch)
2012-02-08 17:07 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 6299 Add relator codes for fr-FR (23.77 KB, patch)
2012-02-14 16:55 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 6299 - add es-ES (25.91 KB, patch)
2012-02-14 16:56 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 6299 - Advertise this addition during upgrade (1.66 KB, patch)
2012-02-14 16:56 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi 2011-05-04 15:25:13 UTC
Provide the user an optional thesaurus that can be linked from "relator term" fields in cataloguing frameworks.

For MARC21 there exists a list:
MARC Code List for Relators
http://www.loc.gov/marc/relators/relaterm.html

Caveats:
- The default values description should be translatable. Perhaps On-the-fly translation, or as a fixed list of values in the templates. Something to discuss.
Comment 1 Tomás Cohen Arazi 2011-06-07 14:30:04 UTC Comment hidden (obsolete)
Comment 2 Nicole C. Engard 2011-07-06 16:23:15 UTC
can you include 'how to test' instructions here?
Comment 3 Tomás Cohen Arazi 2011-07-07 16:20:36 UTC
To test:
Make a normal install, choose 'MARC Code List for Relators, as of http://www.loc.gov/marc/relators/relaterm.html (marc21_relatorterms)"
in the Optional data section of the Choose frameworks step.

Then edit a MARC Framework and choose to link 700$e to the list of authorized values MARCRELATOR

Success should be:
- You can choose to install MaRC relator terms data
- You can link to that list in your frameworks
Comment 4 Tomás Cohen Arazi 2011-07-07 17:31:35 UTC Comment hidden (obsolete)
Comment 5 Nicole C. Engard 2011-07-09 12:09:03 UTC
Okay, and how do we test this if we're a library upgrading - not a new install?  There should be an option for those who are on Koha already and just upgrading from an older version (as I am on my test machine).

Nicole
Comment 6 Tomás Cohen Arazi 2011-07-18 17:46:31 UTC
Prompting the user for loading 'new in koha' sample data is not a feature Koha provides right now. Maybe there could be a new bug filled for that.
As of this bug, it should be signed-off if it happens to work on new installs, as any other optional data's SQL files.
Comment 7 Julian Maurice 2011-07-22 09:55:50 UTC
Not tested on a new install, but with

    $ mysql -u user -p db_name < installer/data/mysql/en/optional/marc21_relatorterms.sql

and it works fine.
Waiting for someone testing on a new install.
Comment 8 Marcel de Rooy 2011-08-04 10:32:45 UTC
> There should be an option for those who are on Koha already and just upgrading
from an older version (as I am on my test machine).

I think this patch provides an excellent addition of sample MARC data. The install script offers the option as advertised. (Did not run it though.)
But I agree with Nicole that only this patch will not help existing installs. An experienced user may run mysql from the command-line, but that seems not to be enough. (How would he know at first?) An informational warning on the updatedatabase screen could be helpful as well as additional information in the INSTALL files. (But even these two things may be overlooked easily.) 

Additionally, minor point: Should you better test on the existence of RELTERMS in the table before inserting? I already have RELTERM in my data; could easily have been RELTERMS too. If RELTERMS should exist somewhere already, you would probably want to insert them under a different name.
Comment 9 Tomás Cohen Arazi 2011-08-04 12:08:01 UTC
Marcel, (In reply to comment #8)
> > There should be an option for those who are on Koha already and just upgrading
> from an older version (as I am on my test machine).

As stated before, I asked on this on IRC and I was told there is no way to handle this properly (i.e. prompt the user to make desitions on this). As I see  it, this is not different to sample patron types and such.

If you think I should add an advice on updatedatabase.pl just let me know.
If you think we should have a method for prompting the user to make a desition just let me know, and we can fill a new bug to work on that.

Regards
To+
Comment 10 Marcel de Rooy 2011-08-04 13:01:55 UTC
(In reply to comment #9)
> As I see it, this is not different to sample patron types and such.

I think you are right about that. I would however add at least a statement to the upgrading user via the updatedatabase script telling him that he could add relator terms now (if you mention that you are adding under RELTERMS, you partially cover my other remark about the name too). So, I would be willing to sign off. I now tested the patch on an empty db. That worked okay.

But unfortunately I saw then that several of the descriptions are somehow truncated in your list (sound should be sound designer, supporting should be supporting host, etc.) It seems that a lot of the two-or-more-words descriptions only contain one word. (You have 80 chars there.)

Therefore I suggest to let somebody correct this list first and add a line to updatedatabase.
Comment 11 Tomás Cohen Arazi 2011-08-04 20:51:56 UTC Comment hidden (obsolete)
Comment 12 Tomás Cohen Arazi 2011-08-04 20:53:38 UTC Comment hidden (obsolete)
Comment 13 Julian Maurice 2011-08-05 08:49:17 UTC
Created attachment 4855 [details]
Screenshot of updatedb page

Printing a '\n' in updatedatabase.pl cause lines to be treated as html list items (see screenshot). I think you must keep only the last line feed (just esthetical issue however).
Comment 14 Tomás Cohen Arazi 2011-08-08 14:47:51 UTC Comment hidden (obsolete)
Comment 15 Tomás Cohen Arazi 2011-08-08 14:53:16 UTC
If anyone is about to sign this please wait until I provide a es-ES script, now that language is on master.
Comment 16 Marcel de Rooy 2011-08-10 13:44:33 UTC
(In reply to comment #15)
> If anyone is about to sign this please wait until I provide a es-ES script, now
> that language is on master.
Tomas, I am signing off now on both patches. Please send a small followup fix for Spanish.
Comment 17 Marcel de Rooy 2011-08-10 13:49:10 UTC Comment hidden (obsolete)
Comment 18 Marcel de Rooy 2011-08-10 13:50:50 UTC Comment hidden (obsolete)
Comment 19 Tomás Cohen Arazi 2011-08-10 14:48:01 UTC Comment hidden (obsolete)
Comment 20 Marcel de Rooy 2011-08-11 07:09:17 UTC Comment hidden (obsolete)
Comment 21 Paul Poulain 2011-10-06 12:56:49 UTC Comment hidden (obsolete)
Comment 22 Paul Poulain 2011-10-06 12:57:02 UTC Comment hidden (obsolete)
Comment 23 Paul Poulain 2011-10-06 13:10:25 UTC
QA comment:
* installer only patch
* add some datas in authorised values, nothing "dangerous"
My only concern is that fr, de, ... versions are also in english. But that can be addessed in another patch, and should not delay passed QA imo

so, marking "passed QA", but chris, I wouldn't complain if you set "failed QA" because of the language comment I made
Comment 24 Ian Walls 2011-10-06 14:56:53 UTC
updatedatabase.pl patch has <<<<<<<<< HEAD relic.

I recommend that we remove any of the SQL files for languages that do not have their translations in place.  It's extra work for a system administrator to remove those untranslated values after they're added, and the lack of support for certain languages is not advertised.  The sysadmin will still have the choice to upload an existing language.

In addition to removing these untranslated SQL files, I also recommend altering the updatedatabase.pl script to indicate exactly which languages are currently supported.

Marking as Failed QA until the git merge relic is removed, and the recommendations for handling untranslated files is addressed.
Comment 25 Paul Poulain 2011-10-06 15:08:55 UTC
yikes ! sorry for the <<<< relic, it's me !!!
OK for the translation proposal, that's a good option.
Comment 26 Jared Camins-Esakov 2012-01-28 15:44:34 UTC Comment hidden (obsolete)
Comment 27 Jared Camins-Esakov 2012-01-28 15:44:48 UTC Comment hidden (obsolete)
Comment 28 Jared Camins-Esakov 2012-01-28 15:46:00 UTC
Created attachment 7373 [details] [review]
Bug 6299 - Provide a list of authorized values for Relator terms

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
August 10,2011: Tested install and upgrade.

Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
January 28, 2012: Removed untranslated non-en SQL relator term files.
Comment 29 Jared Camins-Esakov 2012-01-28 15:46:10 UTC Comment hidden (obsolete)
Comment 30 Jared Camins-Esakov 2012-01-28 15:47:19 UTC
Made the changes requested by Ian, and returning the status to signed off. Note that the "add es-ES" patch should *not* be applied, as it has not been translated yet. tcohen's comment indicates that a translation is in the works.
Comment 31 Jared Camins-Esakov 2012-01-29 13:41:31 UTC Comment hidden (obsolete)
Comment 32 Jared Camins-Esakov 2012-01-29 13:41:57 UTC
Oops. I didn't actually change the status.
Comment 33 Frédéric Demians 2012-01-30 17:09:35 UTC
The MARC Code List for Relators is here:

  http://www.loc.gov/marc/relators/relacode.html

The translation of this table in French is available here:

  http://www.marc21.ca/040010-220-f.html
Comment 34 Frédéric Demians 2012-01-30 18:16:49 UTC Comment hidden (obsolete)
Comment 35 Frédéric Demians 2012-01-30 18:17:45 UTC
Additional sign-off required for 'Relator codes in Frenche' patch.
Comment 36 Frédéric Demians 2012-02-01 11:17:19 UTC Comment hidden (obsolete)
Comment 37 Tomás Cohen Arazi 2012-02-01 13:07:41 UTC
I've been out on vacations. Next week I'll provide a new patch for the spanish translation which I have at the office.
Comment 38 Frédéric Demians 2012-02-07 18:53:29 UTC Comment hidden (obsolete)
Comment 39 Jared Camins-Esakov 2012-02-08 12:02:11 UTC Comment hidden (obsolete)
Comment 40 Tomás Cohen Arazi 2012-02-08 14:20:51 UTC Comment hidden (obsolete)
Comment 41 Tomás Cohen Arazi 2012-02-08 14:23:37 UTC
Spanish translation added, no signoff yet.
Comment 42 Jared Camins-Esakov 2012-02-08 17:07:30 UTC Comment hidden (obsolete)
Comment 43 Paul Poulain 2012-02-13 21:25:32 UTC
QA comment:

Those patches look OK, however, when trying to apply the FR & ES translations, I get a nasty:

fatal: sha1 information is lacking or useless (installer/data/mysql/updatedatabase.pl).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 Bug 6299 - add es-ES
When you have resolved this problem run "git am --resolved".
If you would prefer to skip this patch, instead run "git am --skip".
To restore the original branch and stop patching run "git am --abort".

(same for FR)

Any idea why Jared ?
Comment 44 Tomás Cohen Arazi 2012-02-14 13:28:48 UTC
(In reply to comment #43)
> QA comment:
> 
> Those patches look OK, however, when trying to apply the FR & ES translations,
> I get a nasty:
> 
> fatal: sha1 information is lacking or useless
> (installer/data/mysql/updatedatabase.pl).
> Repository lacks necessary blobs to fall back on 3-way merge.
> Cannot fall back to three-way merge.
> Patch failed at 0001 Bug 6299 - add es-ES
> When you have resolved this problem run "git am --resolved".
> If you would prefer to skip this patch, instead run "git am --skip".
> To restore the original branch and stop patching run "git am --abort".
> 
> (same for FR)
> 
> Any idea why Jared ?

There must have been an addition to updatedatabase.pl that makes patches don't apply
Comment 45 Jared Camins-Esakov 2012-02-14 16:55:50 UTC
Created attachment 7637 [details] [review]
Bug 6299 Add relator codes for fr-FR

Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Confirmed that the RELTERMS authorised values category is populated. Not
knowing French I can't comment on the translations, but any errors can be
corrected at a later date.
Comment 46 Jared Camins-Esakov 2012-02-14 16:56:04 UTC
Created attachment 7638 [details] [review]
Bug 6299 - add es-ES

Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Confirmed that the RELTERMS authorised values category is populated. Not
knowing Spanish I can't comment on the translations, but any errors can be
corrected at a later date.
Comment 47 Jared Camins-Esakov 2012-02-14 16:56:16 UTC
Created attachment 7639 [details] [review]
Bug 6299 - Advertise this addition during upgrade

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
August 10, 2011: Fixed some small typos.

Signed-off-by: Paul Poulain <paul.poulain@biblibre.com>
Oct 7th, 2011: rebased for master

Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Comment 48 Jared Camins-Esakov 2012-02-14 16:57:01 UTC
Rebased to fix the problem that Paul encountered. Reordered the patches to simplify later rebasings. Returning to Passed QA status.
Comment 49 Paul Poulain 2012-02-14 20:35:54 UTC
perfect, this time, worked well !
Comment 50 Jared Camins-Esakov 2012-12-31 00:37:09 UTC
There have been no further reports of problems so I am marking this bug resolved.