Bugzilla – Attachment 137874 Details for
Bug 29001
Subfields attributes are not preserved when order is changed in framework
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 29001: Fix framework edition when subfields are reordered
Bug-29001-Fix-framework-edition-when-subfields-are.patch (text/plain), 21.06 KB, created by
Jonathan Druart
on 2022-07-19 15:15:22 UTC
(
hide
)
Description:
Bug 29001: Fix framework edition when subfields are reordered
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2022-07-19 15:15:22 UTC
Size:
21.06 KB
patch
obsolete
>From 25eb023b7921eb0717104ed219f6a36637607de9 Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >Date: Tue, 19 Jul 2022 17:00:29 +0200 >Subject: [PATCH] Bug 29001: Fix framework edition when subfields are reordered > >There is a flaw when subfields are ordered, the inputs are not retrieved >correctly. >We should not rely on the order but use an id instead. >Test plan: >Create, edit subfields >Modify values from the different subfields >Confirm that values are correctly saved >--- > admin/marc_subfields_structure.pl | 57 ++++++++----------- > .../modules/admin/marc_subfields_structure.tt | 45 ++++++++------- > 2 files changed, 48 insertions(+), 54 deletions(-) > >diff --git a/admin/marc_subfields_structure.pl b/admin/marc_subfields_structure.pl >index a492e5c568f..63bd41902e9 100755 >--- a/admin/marc_subfields_structure.pl >+++ b/admin/marc_subfields_structure.pl >@@ -18,6 +18,7 @@ > # along with Koha; if not, see <http://www.gnu.org/licenses>. > > use Modern::Perl; >+use Encode qw( encode_utf8 ); > use C4::Output qw( output_html_with_http_headers ); > use C4::Auth qw( get_template_and_user ); > use CGI qw ( -utf8 ); >@@ -211,42 +212,32 @@ if ( $op eq 'add_form' ) { > elsif ( $op eq 'add_validate' ) { > my $dbh = C4::Context->dbh; > $template->param( tagfield => "$input->param('tagfield')" ); >- my @tagsubfield = $input->multi_param('tagsubfield'); >- my @liblibrarian = $input->multi_param('liblibrarian'); >- my @libopac = $input->multi_param('libopac'); >- my @kohafield = $input->multi_param('kohafield'); >- my @tab = $input->multi_param('tab'); >- my @seealso = $input->multi_param('seealso'); >- my @hidden = $input->multi_param('hidden'); >- my @authorised_values = $input->multi_param('authorised_value'); >- my @authtypecodes = $input->multi_param('authtypecode'); >- my @value_builder = $input->multi_param('value_builder'); >- my @link = $input->multi_param('link'); >- my @defaultvalue = $input->multi_param('defaultvalue'); >- my @maxlength = $input->multi_param('maxlength'); >+ my $tagfield = $input->param('tagfield'); >+ my @tagsubfield = $input->multi_param('tagsubfield'); >+ use Data::Printer colored => 1; warn p @tagsubfield; >+ my @tab_ids = $input->multi_param('tab_id'); > > my $display_order; >- for ( my $i = 0 ; $i <= $#tagsubfield ; $i++ ) { >- my $tagfield = $input->param('tagfield'); >- my $tagsubfield = $tagsubfield[$i]; >+ for my $tagsubfield ( @tagsubfield ) { > $tagsubfield = "@" unless $tagsubfield ne ''; >- my $liblibrarian = $liblibrarian[$i]; >- my $libopac = $libopac[$i]; >- my $repeatable = $input->param("repeatable$i") ? 1 : 0; >- my $mandatory = $input->param("mandatory$i") ? 1 : 0; >- my $important = $input->param("important$i") ? 1 : 0; >- my $kohafield = $kohafield[$i]; >- my $tab = $tab[$i]; >- my $seealso = $seealso[$i]; >- my $authorised_value = $authorised_values[$i]; >- my $authtypecode = $authtypecodes[$i]; >- my $value_builder = $value_builder[$i]; >- my $hidden = $hidden[$i]; #input->param("hidden$i"); >- my $isurl = $input->param("isurl$i") ? 1 : 0; >- my $link = $link[$i]; >- my $defaultvalue = $defaultvalue[$i]; >- my $maxlength = $maxlength[$i] ? $maxlength[$i] : 9999; >- >+ my $id = shift @tab_ids; >+ my $liblibrarian = $input->param("liblibrarian_$id"); >+ my $libopac = $input->param("libopac_$id"); >+ my $repeatable = $input->param("repeatable_$id") ? 1 : 0; >+ my $mandatory = $input->param("mandatory_$id") ? 1 : 0; >+ my $important = $input->param("important_$id") ? 1 : 0; >+ my $kohafield = $input->param("kohafield_$id"); >+ my $tab = $input->param("tab_$id"); >+ my $seealso = $input->param("seealso_$id"); >+ my $authorised_value = $input->param("authorised_values_$id"); >+ my $authtypecode = $input->param("authtypecodes_$id"); >+ my $value_builder = $input->param("value_builder_$id"); >+ my $hidden = $input->param("hidden_$id"); >+ my $isurl = $input->param("isurl_$id") ? 1 : 0; >+ my $link = $input->param("link_$id"); >+ my $defaultvalue = $input->param("defaultvalue_$id"); >+ my $maxlength = $input->param("maxlength_$id") || 9999; >+ > if (defined($liblibrarian) && $liblibrarian ne "") { > my $mss = Koha::MarcSubfieldStructures->find({tagfield => $tagfield, tagsubfield => $tagsubfield, frameworkcode => $frameworkcode }); > if ($mss) { >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/marc_subfields_structure.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/marc_subfields_structure.tt >index 0ee1b7d199f..24482fa7798 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/marc_subfields_structure.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/marc_subfields_structure.tt >@@ -161,26 +161,29 @@ > [% IF ( subfieldcode == 0 || subfieldcode ) %] > <li> > <span class="label">Subfield code:</span> >- [% loo.subfieldcode | html %] <input type="hidden" name="tagsubfield" value="[% loo.subfieldcode | html %]" /> >+ [% loo.subfieldcode | html %] >+ <input type="hidden" name="tagsubfield" value="[% loo.subfieldcode | html %]" /> >+ <input type="hidden" name="tab_id" value="[% loo.row | html %]" /> > </li> > [% ELSE %] > <li> > <label for="tagsubfield[% loo.row | html %]">Subfield code:</label> > <input type="text" id="tagsubfield[% loo.row | html %]" name="tagsubfield" value="[% loo.subfieldcode | html %]" /> >+ <input type="hidden" name="tab_id" value="[% loo.row | html %]" /> > </li> > [% END %] > <li> > <label for="liblibrarian[% loo.row | html %]">Text for librarian: </label> >- <input id="liblibrarian[% loo.row | html %]" type="text" name="liblibrarian" value="[% loo.liblibrarian | html_entity %]" size="40" maxlength="80" /> >+ <input id="liblibrarian[% loo.row | html %]" type="text" name="liblibrarian_[% loo.row | html %]" value="[% loo.liblibrarian | html_entity %]" size="40" maxlength="80" /> > </li> > <li> > <label for="libopac[% loo.row | html %]">Text for OPAC: </label> >- <input type="text" id="libopac[% loo.row | html %]" name="libopac" value="[% loo.libopac | html_entity %]" size="40" maxlength="80" /> >+ <input type="text" id="libopac[% loo.row | html %]" name="libopac_[% loo.row | html %]" value="[% loo.libopac | html_entity %]" size="40" maxlength="80" /> > </li> > <li> > <label for="repeatable[% loo.row | html %]">Repeatable: </label> > [% IF loo.repeatable %] >- <input type="checkbox" id="repeatable[% loo.row | html %]" name="repeatable[% loo.row | html %]" checked="checked" value="1" /> >+ <input type="checkbox" id="repeatable[% loo.row | html %]" name="repeatable_[% loo.row | html %]" checked="checked" value="1" /> > [% ELSE %] > <input type="checkbox" id="repeatable[% loo.row | html %]" name="repeatable[% loo.row | html %]" value="1" /> > [% END %] >@@ -188,21 +191,21 @@ > <li> > <label for="mandatory[% loo.row | html %]">Mandatory: </label> > [% IF loo.mandatory %] >- <input type="checkbox" id="mandatory[% loo.row | html %]" name="mandatory[% loo.row | html %]" checked="checked" value="1" /> >+ <input type="checkbox" id="mandatory[% loo.row | html %]" name="mandatory_[% loo.row | html %]" checked="checked" value="1" /> > [% ELSE %] >- <input type="checkbox" id="mandatory[% loo.row | html %]" name="mandatory[% loo.row | html %]" value="1" /> >+ <input type="checkbox" id="mandatory[% loo.row | html %]" name="mandatory_[% loo.row | html %]" value="1" /> > [% END %] > </li> > <li> > <label for="important[% loo.row | html %]">Important: </label> > [% IF loo.important %] >- <input type="checkbox" id="important[% loo.row | html %]" name="important[% loo.row | html %]" checked="checked" value="1" /> >+ <input type="checkbox" id="important[% loo.row | html %]" name="important_[% loo.row | html %]" checked="checked" value="1" /> > [% ELSE %] >- <input type="checkbox" id="important[% loo.row | html %]" name="important[% loo.row | html %]" value="1" /> >+ <input type="checkbox" id="important[% loo.row | html %]" name="important_[% loo.row | html %]" value="1" /> > [% END %] > </li> > <li><label for="tab[% loo.row | html %]">Managed in tab: </label> >- <select name="tab" tabindex="" id="tab[% loo.row | html %]"> >+ <select name="tab_[% loo.row | html %]" tabindex="" id="tab[% loo.row | html %]"> > [%- IF ( loo.tab == -1 ) -%] > <option value="-1" selected="selected">ignore</option> > [%- ELSE -%] >@@ -233,17 +236,17 @@ > <ol> > <li> > <label for="defaultvalue[% loo.row | html %]">Default value:</label> >- <input type="text" name="defaultvalue" id="defaultvalue[% loo.row | html %]" value="[% loo.defaultvalue | html %]" /> >+ <input type="text" name="defaultvalue_[% loo.row | html %]" id="defaultvalue[% loo.row | html %]" value="[% loo.defaultvalue | html %]" /> > </li> > <li> > <label for="maxlength[% loo.row | html %]">Max length:</label> >- <input type="text" id="maxlength[% loo.row | html %]" name="maxlength" value="[% loo.maxlength | html %]" size="4" /> >+ <input type="text" id="maxlength[% loo.row | html %]" name="maxlength_[% loo.row | html %]" value="[% loo.maxlength | html %]" size="4" /> > </li> > <li> > [% IF loo.hidden_protected %] >- <input type="hidden" id="hidden-[% loo.row | html %]" name="hidden" value="[% loo.hidden | html %]" data-koha-protected="1" /> >+ <input type="hidden" id="hidden-[% loo.row | html %]" name="hidden_[% loo.row | html %]" value="[% loo.hidden | html %]" data-koha-protected="1" /> > [% ELSE %] >- <input type="hidden" id="hidden-[% loo.row | html %]" name="hidden" value="[% loo.hidden | html %]" /> >+ <input type="hidden" id="hidden-[% loo.row | html %]" name="hidden_[% loo.row | html %]" value="[% loo.hidden | html %]" /> > [% END %] > <label for="hidden[% loo.row | html %]">Visibility: </label> > <input type="checkbox" id="hidden_opac_[% loo.row | html %]" class="inclusive_[% loo.row | html %]" name="hidden_opac_[% loo.row | html %]"/> >@@ -260,21 +263,21 @@ > <li> > <label for="isurl[% loo.row | html %]">Is a URL:</label> > [% IF loo.isurl %] >- <input type="checkbox" id="isurl[% loo.row | html %]" name="isurl[% loo.row | html %]" checked="checked" value="1" /> >+ <input type="checkbox" id="isurl[% loo.row | html %]" name="isurl_[% loo.row | html %]" checked="checked" value="1" /> > [% ELSE %] >- <input type="checkbox" id="isurl[% loo.row | html %]" name="isurl[% loo.row | html %]" value="1" /> >+ <input type="checkbox" id="isurl[% loo.row | html %]" name="isurl_[% loo.row | html %]" value="1" /> > [% END %] > <span class="hint">If checked, it means that the subfield is a URL and can be clicked</span> > </li> > <li> > <label for="link[% loo.row | html %]">Link:</label> >- <input type="text" id="link[% loo.row | html %]" name="link" value="[% loo.link | html %]" size="10" maxlength="80" /> >+ <input type="text" id="link[% loo.row | html %]" name="link_[% loo.row | html %]" value="[% loo.link | html %]" size="10" maxlength="80" /> > <div class="hint">An index name, e.g. title or Local-Number</div> > </li> > <li> > <label for="kohafield[% loo.row | html %]">Koha link:</label> > <!-- This select should be DISABLED; value is submitted by the following hidden input --> >- <select name="kohafield" id="kohafield[% loo.row | html %]" disabled> >+ <select name="kohafield_[% loo.row | html %]" id="kohafield[% loo.row | html %]" disabled> > [% FOREACH value IN loo.kohafields %] > [% IF ( value == loo.kohafield ) %] > <option value="[% value | html %]" selected="selected">[% value | html %]</option> >@@ -284,7 +287,7 @@ > [% END %] > </select> > <!-- Do NOT remove this next hidden input! We need it to save kohafield. --> >- <input type="hidden" name="kohafield" value="[% loo.kohafield | html %]"/> >+ <input type="hidden" name="kohafield_[% loo.row | html %]" value="[% loo.kohafield | html %]"/> > </li> > </ol> > </fieldset> <!-- /.rows --> >@@ -296,7 +299,7 @@ > <ol> > <li> > <label for="authorised_value[% loo.row | html %]">Authorized value:</label> >- <select name="authorised_value" id="authorised_value[% loo.row | html %]"> >+ <select name="authorised_value_[% loo.row | html %]" id="authorised_value[% loo.row | html %]"> > <option value=""></option> > [% FOREACH value IN loo.authorised_values %] > [% IF ( value == loo.authorised_value ) %] >@@ -309,7 +312,7 @@ > </li> > <li> > <label for="authtypecode[% loo.row | html %]">Thesaurus:</label> >- <select name="authtypecode" id="authtypecode[% loo.row | html %]"> >+ <select name="authtypecode_[% loo.row | html %]" id="authtypecode[% loo.row | html %]"> > [% FOREACH value IN loo.authtypes %] > [% IF ( value == loo.authtypecode ) %] > <option value="[% value | html %]" selected="selected">[% value | html %]</option> >@@ -321,7 +324,7 @@ > </li> > <li> > <label for="value_builder[% loo.row | html %]">Plugin:</label> >- <select name="value_builder" id="value_builder[% loo.row | html %]"> >+ <select name="value_builder_[% loo.row | html %]" id="value_builder[% loo.row | html %]"> > [% FOREACH value IN loo.value_builders %] > [% IF ( value == loo.value_builder ) %] > <option value="[% value | html %]" selected="selected">[% value | html %]</option> >-- >2.25.1
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=137874">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 29001
:
137874
|
137893
|
137904
|
137991
|
137992