|
Description
Vitor Fernandes
2020-02-20 12:44:28 UTC
Created attachment 118186 [details] [review] Bug 24698: Add plugin and template files for unimarc authority leader In Unimarc installations when adding/editing an authority record there is no Tag Editor for the leader field ( 000 ). This patch fixes that. Test plan: 1) Try to catalogue a new authoritry and check if on field 000 you can find the Tag Editor. 2) Apply the patch. 3) In the authorities framework view the subfields of field 000 and select for plugin option the value unimarc_leader_authorities.pl. 4) Restart memcached / plack if needed. 5) Repeat step 1 and notice if the Tag Editor is visible and click it to open new window in order to manage the field contents and press OK. 6) Check the value of the generated leader. Created attachment 124437 [details] [review] Bug 24698: Add plugin and template files for unimarc authority leader In Unimarc installations when adding/editing an authority record there is no Tag Editor for the leader field ( 000 ). This patch fixes that. Test plan: 1) Try to catalogue a new authoritry and check if on field 000 you can find the Tag Editor. 2) Apply the patch. 3) In the authorities framework view the subfields of field 000 and select for plugin option the value unimarc_leader_authorities.pl. 4) Restart memcached / plack if needed. 5) Repeat step 1 and notice if the Tag Editor is visible and click it to open new window in order to manage the field contents and press OK. 6) Check the value of the generated leader. Signed-off-by: Esther <estherm@sodapsringsid.com> Created attachment 125669 [details] [review] Bug 24698: Add plugin and template files for unimarc authority leader In Unimarc installations when adding/editing an authority record there is no Tag Editor for the leader field ( 000 ). This patch fixes that. Test plan: 1) Try to catalogue a new authoritry and check if on field 000 you can find the Tag Editor. 2) Apply the patch. 3) In the authorities framework view the subfields of field 000 and select for plugin option the value unimarc_leader_authorities.pl. 4) Restart memcached / plack if needed. 5) Repeat step 1 and notice if the Tag Editor is visible and click it to open new window in order to manage the field contents and press OK. 6) Check the value of the generated leader. Signed-off-by: Esther <estherm@sodapsringsid.com> Created attachment 125670 [details] [review] Bug 24698: (QA follow-up) Fix errors caused by more strict exports being introduced The original patch "Bug 24698: Add plugin and template files for unimarc authority leader" doesn't work without explictly importing the methods from C4::Auth and C4::Output due to the default exports being changed in those modules after the patch was made. Created attachment 125671 [details] [review] Bug 24698: (QA follow-up) Remove debug info being outputted to the template This is only needed during development. Created attachment 125672 [details] [review] Bug 24698: (QA follow-up) Align variables with whitespace Created attachment 125673 [details] [review] Bug 24698: (QA follow-up) Remove dead code These were left off from the copy&paste taken from unimarc_field_100_authorities.pl Created attachment 125674 [details] [review] Bug 24698: (QA follow-up) Fix formatting and typos Created attachment 125675 [details] [review] Bug 24698: (QA follow-up) Remove passing of authnotrequired parameter It should only be used when we want to override the default of auth being required. Created attachment 125676 [details] [review] Bug 24698: (QA follow-up) Make plugin script executable Created attachment 125677 [details] [review] Bug 24698: (QA follow-up) Use spaces instead of tabs This makes the QA script happy. Created attachment 125678 [details] [review] Bug 24698: (QA follow-up) Filter untrusted input to prevent XSS Created attachment 125679 [details] [review] Bug 24698: (QA follow-up) Don't specify select element's size if it is 1 This is to make the QA script happy, in order to not to break Bootstrap CSS rules, see Bug 28066 for more info Created attachment 125680 [details] [review] Bug 24698: (QA follow-up) Don't specify script type explictly It's makes the QA script complain because the option doesn't have any effect nowadays: https://www.w3.org/TR/2017/REC-html52-20171214/obsolete.html#warnings-for-obsolete-but-conforming-features Don't mind the follow-ups ;D I'm willing to Pass QA after checking one thing that is unclear to me: The spec from that ifla.org website only defines as possible values for "9 Type of entity" the letters a-l but here this adds the option for "Series" with an blank value (space). Is this supposed to be allowed? Where is it mentioned as being a possible value? No it is not allowed base ifla instructions you are right. I forgot to delete it. (In reply to George Veranis from comment #16) > No it is not allowed base ifla instructions you are right. I forgot to > delete it. Could you please send a follow-up for that and sign-off the ones I did? (even though they are pretty trivial – but many in count). Created attachment 125929 [details] [review] Bug 24698: (QA follow-up) Fix errors caused by more strict exports being introduced The original patch "Bug 24698: Add plugin and template files for unimarc authority leader" doesn't work without explictly importing the methods from C4::Auth and C4::Output due to the default exports being changed in those modules after the patch was made. Signed-off-by: George Veranis <gveranis@dataly.gr> Created attachment 125930 [details] [review] Bug 24698: (QA follow-up) Remove debug info being outputted to the template This is only needed during development. Signed-off-by: George Veranis <gveranis@dataly.gr> Created attachment 125931 [details] [review] Bug 24698: (QA follow-up) Align variables with whitespace Signed-off-by: George Veranis <gveranis@dataly.gr> Created attachment 125932 [details] [review] Bug 24698: (QA follow-up) Remove dead code These were left off from the copy&paste taken from unimarc_field_100_authorities.pl Signed-off-by: George Veranis <gveranis@dataly.gr> Created attachment 125933 [details] [review] Bug 24698: (QA follow-up) Fix formatting and typos Signed-off-by: George Veranis <gveranis@dataly.gr> Created attachment 125934 [details] [review] Bug 24698: (QA follow-up) Remove passing of authnotrequired parameter It should only be used when we want to override the default of auth being required. Signed-off-by: George Veranis <gveranis@dataly.gr> Created attachment 125935 [details] [review] Bug 24698: (QA follow-up) Make plugin script executable Signed-off-by: George Veranis <gveranis@dataly.gr> Created attachment 125936 [details] [review] Bug 24698: (QA follow-up) Use spaces instead of tabs This makes the QA script happy. Signed-off-by: George Veranis <gveranis@dataly.gr> Created attachment 125937 [details] [review] Bug 24698: (QA follow-up) Filter untrusted input to prevent XSS Signed-off-by: George Veranis <gveranis@dataly.gr> Created attachment 125938 [details] [review] Bug 24698: (QA follow-up) Don't specify select element's size if it is 1 This is to make the QA script happy, in order to not to break Bootstrap CSS rules, see Bug 28066 for more info Signed-off-by: George Veranis <gveranis@dataly.gr> Created attachment 125939 [details] [review] Bug 24698: (QA follow-up) Don't specify script type explictly It's makes the QA script complain because the option doesn't have any effect nowadays: https://www.w3.org/TR/2017/REC-html52-20171214/obsolete.html#warnings-for-obsolete-but-conforming-features Signed-off-by: George Veranis <gveranis@dataly.gr> Created attachment 125940 [details] [review] Bug 24698: (follow-up) remove HTML select option for Series The IFLA authorities format specification defines only the letters a-l as possible values for authority leader 000 character position 9 ("Type of entity"). Therefore, the first option from the HTML select dropdown menu (" - Series") must be removed. Mentored-by: Andreas Roussos <a.roussos@dataly.gr> (In reply to Joonas Kylmälä from comment #17) > (In reply to George Veranis from comment #16) > > No it is not allowed base ifla instructions you are right. I forgot to > > delete it. > > Could you please send a follow-up for that and sign-off the ones I did? > (even though they are pretty trivial – but many in count). Hi Joonas, I did what you asked for , please check-it and tell me if you need something more . Thanks for your QA ! Created attachment 125998 [details] [review] Bug 24698: Add plugin and template files for unimarc authority leader In Unimarc installations when adding/editing an authority record there is no Tag Editor for the leader field ( 000 ). This patch fixes that. Test plan: 1) Try to catalogue a new authoritry and check if on field 000 you can find the Tag Editor. 2) Apply the patch. 3) In the authorities framework view the subfields of field 000 and select for plugin option the value unimarc_leader_authorities.pl. 4) Restart memcached / plack if needed. 5) Repeat step 1 and notice if the Tag Editor is visible and click it to open new window in order to manage the field contents and press OK. 6) Check the value of the generated leader. Signed-off-by: Esther <estherm@sodapsringsid.com> Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi> Created attachment 125999 [details] [review] Bug 24698: (QA follow-up) Fix errors caused by more strict exports being introduced The original patch "Bug 24698: Add plugin and template files for unimarc authority leader" doesn't work without explictly importing the methods from C4::Auth and C4::Output due to the default exports being changed in those modules after the patch was made. Signed-off-by: George Veranis <gveranis@dataly.gr> Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi> Created attachment 126000 [details] [review] Bug 24698: (QA follow-up) Remove debug info being outputted to the template This is only needed during development. Signed-off-by: George Veranis <gveranis@dataly.gr> Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi> Created attachment 126001 [details] [review] Bug 24698: (QA follow-up) Align variables with whitespace Signed-off-by: George Veranis <gveranis@dataly.gr> Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi> Created attachment 126002 [details] [review] Bug 24698: (QA follow-up) Remove dead code These were left off from the copy&paste taken from unimarc_field_100_authorities.pl Signed-off-by: George Veranis <gveranis@dataly.gr> Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi> Created attachment 126003 [details] [review] Bug 24698: (QA follow-up) Fix formatting and typos Signed-off-by: George Veranis <gveranis@dataly.gr> Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi> Created attachment 126004 [details] [review] Bug 24698: (QA follow-up) Remove passing of authnotrequired parameter It should only be used when we want to override the default of auth being required. Signed-off-by: George Veranis <gveranis@dataly.gr> Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi> Created attachment 126005 [details] [review] Bug 24698: (QA follow-up) Make plugin script executable Signed-off-by: George Veranis <gveranis@dataly.gr> Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi> Created attachment 126006 [details] [review] Bug 24698: (QA follow-up) Use spaces instead of tabs This makes the QA script happy. Signed-off-by: George Veranis <gveranis@dataly.gr> Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi> Created attachment 126007 [details] [review] Bug 24698: (QA follow-up) Filter untrusted input to prevent XSS Signed-off-by: George Veranis <gveranis@dataly.gr> Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi> Created attachment 126008 [details] [review] Bug 24698: (QA follow-up) Don't specify select element's size if it is 1 This is to make the QA script happy, in order to not to break Bootstrap CSS rules, see Bug 28066 for more info Signed-off-by: George Veranis <gveranis@dataly.gr> Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi> Created attachment 126009 [details] [review] Bug 24698: (QA follow-up) Don't specify script type explictly It's makes the QA script complain because the option doesn't have any effect nowadays: https://www.w3.org/TR/2017/REC-html52-20171214/obsolete.html#warnings-for-obsolete-but-conforming-features Signed-off-by: George Veranis <gveranis@dataly.gr> Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi> Created attachment 126010 [details] [review] Bug 24698: (follow-up) remove HTML select option for Series The IFLA authorities format specification defines only the letters a-l as possible values for authority leader 000 character position 9 ("Type of entity"). Therefore, the first option from the HTML select dropdown menu (" - Series") must be removed. Mentored-by: Andreas Roussos <a.roussos@dataly.gr> Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi> (In reply to George Veranis from comment #30) > Hi Joonas, I did what you asked for , please check-it and tell me if you > need something more . Thanks for your QA ! Looks good now. Passing QA. Follow-up patches will be squashed when pushed. Pushed to master for 21.11, thanks to everybody involved! Pushed to 21.05.x for 21.05.05 Pushed to 20.11.x for 20.11.11 Not backported to oldoldstable (20.05.x). Feel free to ask if it's needed. |