Bug 21280 - Unify MARC editor in biblios and authorities
Summary: Unify MARC editor in biblios and authorities
Status: BLOCKED
Alias: None
Product: Koha
Classification: Unclassified
Component: Staff interface (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Ere Maijala
QA Contact: Josef Moravec
URL:
Keywords:
: 22574 (view as bug list)
Depends on:
Blocks:
 
Reported: 2018-08-28 11:15 UTC by paxed
Modified: 2020-01-23 11:22 UTC (History)
4 users (show)

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


Attachments
Bug 21280: Unify MARC tab HTML generation (10.36 KB, patch)
2018-08-28 11:18 UTC, paxed
Details | Diff | Splinter Review
Bug 21280: Move MARC editor code to a common class (67.90 KB, patch)
2019-02-22 11:05 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 21280: Move MARC editor tabs to an include file (59.44 KB, patch)
2019-02-22 11:05 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 21280: Unify field types (3.18 KB, patch)
2019-02-22 11:05 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 21280: Move MARC editor code to a common class (67.90 KB, patch)
2019-02-22 11:46 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 21280: Move MARC editor tabs to an include file (59.44 KB, patch)
2019-02-22 11:46 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 21280: Unify field types (3.18 KB, patch)
2019-02-22 11:46 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 21280: Add pref for MARC authority documentation url (8.37 KB, patch)
2019-02-22 11:46 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 21280: Move MARC editor code to a common class (67.90 KB, patch)
2019-04-02 09:38 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 21280: Move MARC editor tabs to an include file (59.70 KB, patch)
2019-04-02 09:38 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 21280: Unify field types (3.26 KB, patch)
2019-04-02 09:38 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 21280: Add pref for MARC authority documentation url (8.37 KB, patch)
2019-04-02 09:38 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 21280: Move MARC editor code to a common class (68.02 KB, patch)
2019-04-02 11:54 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 21280: Move MARC editor tabs to an include file (59.77 KB, patch)
2019-04-02 11:55 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 21280: Unify field types (3.28 KB, patch)
2019-04-02 11:55 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 21280: Add pref for MARC authority documentation url (8.51 KB, patch)
2019-04-02 11:55 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 21280: Move MARC editor code to a common class (68.12 KB, patch)
2019-05-07 06:28 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 21280: Move MARC editor tabs to an include file (59.81 KB, patch)
2019-05-07 06:28 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 21280: Unify field types (3.32 KB, patch)
2019-05-07 06:28 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 21280: Add pref for MARC authority documentation url (8.53 KB, patch)
2019-05-07 06:28 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 21280: (follow-up) Adapt bug 21172 to Koha::MarcEditor (1.48 KB, patch)
2019-05-07 06:28 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 21280: (QA follow-up) Do not use type parameter in script tag (827 bytes, patch)
2019-05-07 06:28 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 21280: (QA follow-up) Use constant instead of Readonly in MarcEditor (7.32 KB, patch)
2019-05-07 06:29 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 21280: (follow-up) Refactor build_tabs to return array for better transparency (3.68 KB, patch)
2019-05-07 06:29 UTC, Josef Moravec
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description paxed 2018-08-28 11:15:04 UTC
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.
Comment 1 paxed 2018-08-28 11:18:26 UTC
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
Comment 2 Michal Denar 2018-10-04 19:55:37 UTC
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.
Comment 3 Ere Maijala 2019-02-22 09:06:06 UTC
With paxed's approval I'm taking over and extending the scope to include unifying the underlying perl code.
Comment 4 Ere Maijala 2019-02-22 11:05:10 UTC
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.
Comment 5 Ere Maijala 2019-02-22 11:05:14 UTC
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.
Comment 6 Ere Maijala 2019-02-22 11:05:17 UTC
Created attachment 85504 [details] [review]
Bug 21280: Unify field types
Comment 7 Ere Maijala 2019-02-22 11:46:06 UTC
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.
Comment 8 Ere Maijala 2019-02-22 11:46:12 UTC
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.
Comment 9 Ere Maijala 2019-02-22 11:46:16 UTC
Created attachment 85519 [details] [review]
Bug 21280: Unify field types
Comment 10 Ere Maijala 2019-02-22 11:46:20 UTC
Created attachment 85520 [details] [review]
Bug 21280: Add pref for MARC authority documentation url

Makes the documentation url for authorities separately configurable.
Comment 11 Josef Moravec 2019-03-26 11:34:12 UTC
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
Comment 12 Josef Moravec 2019-03-26 11:44:08 UTC
There is conflict with master, probably because of bug 22399
Comment 13 Josef Moravec 2019-03-26 11:44:53 UTC
*** Bug 22574 has been marked as a duplicate of this bug. ***
Comment 14 Ere Maijala 2019-04-02 09:38:33 UTC
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.
Comment 15 Ere Maijala 2019-04-02 09:38:37 UTC
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.
Comment 16 Ere Maijala 2019-04-02 09:38:40 UTC
Created attachment 87313 [details] [review]
Bug 21280: Unify field types
Comment 17 Ere Maijala 2019-04-02 09:38:43 UTC
Created attachment 87314 [details] [review]
Bug 21280: Add pref for MARC authority documentation url

Makes the documentation url for authorities separately configurable.
Comment 18 Ere Maijala 2019-04-02 09:39:11 UTC
Rebased.
Comment 19 Michal Denar 2019-04-02 11:54:58 UTC
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>
Comment 20 Michal Denar 2019-04-02 11:55:03 UTC
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>
Comment 21 Michal Denar 2019-04-02 11:55:07 UTC
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>
Comment 22 Michal Denar 2019-04-02 11:55:10 UTC
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>
Comment 23 Josef Moravec 2019-05-07 06:28:37 UTC
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>
Comment 24 Josef Moravec 2019-05-07 06:28:42 UTC
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>
Comment 25 Josef Moravec 2019-05-07 06:28:46 UTC
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>
Comment 26 Josef Moravec 2019-05-07 06:28:50 UTC
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>
Comment 27 Josef Moravec 2019-05-07 06:28:55 UTC
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>
Comment 28 Josef Moravec 2019-05-07 06:28:59 UTC
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>
Comment 29 Josef Moravec 2019-05-07 06:29:06 UTC
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>
Comment 30 Josef Moravec 2019-05-07 06:29:11 UTC
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>
Comment 31 Josef Moravec 2019-05-07 06:29:34 UTC
Not fully ready yet, I will continue on that
Comment 32 Ere Maijala 2020-01-23 11:22:00 UTC
Josef?