Koha-community/Koha
13
7
Fork 0
Code Issues Releases Wiki Activity

Bug 29001: Fix framework edition when subfields are reordered

Browse source 
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

Signed-off-by: Thibault Kero <thibault.keromnes@univ-paris8.fr>

Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
This commit is contained in:
Jonathan Druart 2022-07-19 17:00:29 +02:00 committed by Tomas Cohen Arazi
parent 8b957a69d4
commit 16a919d919
Signed by: tomascohen
GPG key ID: 0A272EA1B2F3C15F
2 changed files with 48 additions and 55 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

56
admin/marc_subfields_structure.pl
View file

@ -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,31 @@ 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');
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) {

47
koha-tmpl/intranet-tmpl/prog/en/modules/admin/marc_subfields_structure.tt
View file

@ -161,48 +161,51 @@
[% 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" />
<input type="checkbox" id="repeatable[% loo.row | html %]" name="repeatable_[% loo.row | html %]" value="1" />
[% END %]
</li>
<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>