Bug 11885 improved the MARC detail tab view, so it it easier to style with CSS, but the authority MARC tabs are still in a sorry state. We can unify the code into a single include file, and reuse that from both.
Created attachment 78214 [details] [review] Bug 21280: Unify MARC tab HTML generation Bug 11885 improved the MARC detail tab view, so it it easier to style with CSS, but the authority MARC tabs are still in a sorry state. We can unify the code into a single include file, and reuse that from both. Test plan: 1) Apply the patch 2) Look at the biblio MARC detail view, check the tabs look the same as before 3) Look at an authority MARC detail view, check the tabs look the same as the biblio view
Hi, I get error: Applying: Bug 21280: Unify MARC tab HTML generation Using index info to reconstruct a base tree... M authorities/detail.pl M koha-tmpl/intranet-tmpl/prog/en/modules/authorities/detail.tt M koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/MARCdetail.tt Falling back to patching base and 3-way merge... error: The following untracked working tree files would be overwritten by merge: koha-tmpl/intranet-tmpl/prog/en/includes/marc_tab_html.inc Please, rebase on master.
With paxed's approval I'm taking over and extending the scope to include unifying the underlying perl code.
Created attachment 85502 [details] [review] Bug 21280: Move MARC editor code to a common class Combines the MARC editor code from addbiblio.pl and authorities.pl to a common class.
Created attachment 85503 [details] [review] Bug 21280: Move MARC editor tabs to an include file Moves the tabs code from addbiblio.tt to marc_editor.inc and changes authorities.tt to use it too. Minor changes in the tabs code were needed to support both functions. Adds support for toggling docs and tags for authorities and updates default documentation links. Test plan: 1. Test adding and editing biblio records and authority records. 2. Make sure visibility of fields is correct initially and when values are added. 3. Make sure settings for tags and documentation links work and stick across edits, also for authorities.
Created attachment 85504 [details] [review] Bug 21280: Unify field types
Created attachment 85517 [details] [review] Bug 21280: Move MARC editor code to a common class Combines the MARC editor code from addbiblio.pl and authorities.pl to a common class.
Created attachment 85518 [details] [review] Bug 21280: Move MARC editor tabs to an include file Moves the tabs code from addbiblio.tt to marc_editor.inc and changes authorities.tt to use it too. Minor changes in the tabs code were needed to support both functions. Adds support for toggling docs and tags for authorities and updates default documentation links. Test plan: 1. Test adding and editing biblio records and authority records. 2. Make sure visibility of fields is correct initially and when values are added. 3. Make sure settings for tags and documentation links work and stick across edits, also for authorities.
Created attachment 85519 [details] [review] Bug 21280: Unify field types
Created attachment 85520 [details] [review] Bug 21280: Add pref for MARC authority documentation url Makes the documentation url for authorities separately configurable.
Comment on attachment 85517 [details] [review] Bug 21280: Move MARC editor code to a common class Review of attachment 85517 [details] [review]: ----------------------------------------------------------------- I just quickly go through code and found only small issues, great job Ere. ::: Koha/MarcEditor.pm @@ +22,5 @@ > +use Modern::Perl; > + > +use Readonly; > + > +use C4::ClassSource; use Koha::ClassSources instead @@ +31,5 @@ > +use base qw(Class::Accessor); > + > +# Constants to refer to the standard editor types > +Readonly our $EDITOR_BIBLIO => 'biblio'; > +Readonly our $EDITOR_AUTHORITY => 'authority'; use constant instead of Readonly. See https://wiki.koha-community.org/wiki/Coding_Guidelines#PERL25:_Read_only_variables @@ +347,5 @@ > + } > + elsif ( $self->{tags}->{$tag}->{$subfield}->{authorised_value} eq "cn_source" ) { > + push @authorised_values, ""; > + > + my $class_sources = GetClassSources(); Could be easily replaced by Koha::ClassSource->search @@ +377,5 @@ > + $query = "SELECT authorised_value,lib > + FROM authorised_values > + WHERE category=? ORDER BY lib"; > + } > + my $authorised_values_sth = C4::Context->dbh->prepare( $query ); It would be better to use Koha::AuthorizedValues module
There is conflict with master, probably because of bug 22399
*** Bug 22574 has been marked as a duplicate of this bug. ***
Created attachment 87311 [details] [review] Bug 21280: Move MARC editor code to a common class Combines the MARC editor code from addbiblio.pl and authorities.pl to a common class.
Created attachment 87312 [details] [review] Bug 21280: Move MARC editor tabs to an include file Moves the tabs code from addbiblio.tt to marc_editor.inc and changes authorities.tt to use it too. Minor changes in the tabs code were needed to support both functions. Adds support for toggling docs and tags for authorities and updates default documentation links. Test plan: 1. Test adding and editing biblio records and authority records. 2. Make sure visibility of fields is correct initially and when values are added. 3. Make sure settings for tags and documentation links work and stick across edits, also for authorities.
Created attachment 87313 [details] [review] Bug 21280: Unify field types
Created attachment 87314 [details] [review] Bug 21280: Add pref for MARC authority documentation url Makes the documentation url for authorities separately configurable.
Rebased.
Created attachment 87318 [details] [review] Bug 21280: Move MARC editor code to a common class Combines the MARC editor code from addbiblio.pl and authorities.pl to a common class. Signed-off-by: Michal Denar <black23@gmail.com>
Created attachment 87319 [details] [review] Bug 21280: Move MARC editor tabs to an include file Moves the tabs code from addbiblio.tt to marc_editor.inc and changes authorities.tt to use it too. Minor changes in the tabs code were needed to support both functions. Adds support for toggling docs and tags for authorities and updates default documentation links. Test plan: 1. Test adding and editing biblio records and authority records. 2. Make sure visibility of fields is correct initially and when values are added. 3. Make sure settings for tags and documentation links work and stick across edits, also for authorities. Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com>
Created attachment 87320 [details] [review] Bug 21280: Unify field types Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com>
Created attachment 87321 [details] [review] Bug 21280: Add pref for MARC authority documentation url Makes the documentation url for authorities separately configurable. Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com>
Created attachment 89407 [details] [review] Bug 21280: Move MARC editor code to a common class Combines the MARC editor code from addbiblio.pl and authorities.pl to a common class. Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 89408 [details] [review] Bug 21280: Move MARC editor tabs to an include file Moves the tabs code from addbiblio.tt to marc_editor.inc and changes authorities.tt to use it too. Minor changes in the tabs code were needed to support both functions. Adds support for toggling docs and tags for authorities and updates default documentation links. Test plan: 1. Test adding and editing biblio records and authority records. 2. Make sure visibility of fields is correct initially and when values are added. 3. Make sure settings for tags and documentation links work and stick across edits, also for authorities. Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 89409 [details] [review] Bug 21280: Unify field types Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 89410 [details] [review] Bug 21280: Add pref for MARC authority documentation url Makes the documentation url for authorities separately configurable. Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 89411 [details] [review] Bug 21280: (follow-up) Adapt bug 21172 to Koha::MarcEditor Test plan: As bug 21772 - read the code and confirm it makes sense Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 89412 [details] [review] Bug 21280: (QA follow-up) Do not use type parameter in script tag Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 89413 [details] [review] Bug 21280: (QA follow-up) Use constant instead of Readonly in MarcEditor Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 89414 [details] [review] Bug 21280: (follow-up) Refactor build_tabs to return array for better transparency Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Not fully ready yet, I will continue on that
Josef?