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

Bug 30482: Potential for bad string concatenation in cataloging validation error message

Browse source 
In validating the basic MARC editor form before submission, we run
AreFieldsNotOk() twice (once with the parameter "false") and concatenate
the result for output. This creates the potential for the error string
to be appended with "0" if AreFieldsNotOk() returns false.

This patch improves the logic around building the error string.

To test, apply the patch and make sure one of your MARC frameworks
contains at least one mandatory field and at least one important field.

- Edit or create a MARC record in the basic MARC editor.
- Submit the form in various states of completion:
  - If a mandatory and an important field are empty, you should see two
    error messages at the top. "The following mandatory subfields aren't
    filled" and "The following important subfields aren’t filled."
  - If a mandatory OR an important field is empty, you should see a
    single message.
    - If it's the important field which is empty, a confirmation will
      ask if you want to save the record anyway. Test that both answers
      to this confirmation work correctly.
  - If no mandatory or important fields are empty the form should
    submit.

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
This commit is contained in:
Owen Leonard 2022-04-07 09:36:00 +00:00 committed by Fridolin Somers
parent 763bfaa55b
commit 545922e4cb
1 changed files with 26 additions and 23 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

49
koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/addbiblio.tt
View file

@ -540,7 +540,7 @@ function PopupMARCFieldDoc(field) {
var subfields = new Array();
var tab = new Array();
var label = new Array();
var flag=0;
var flag = false;
var tabflag= new Array();
var StrAlert = "<div id='form-errors' class='dialog alert list'>";
var notFilledClass = "subfield_not_filled";
@ -609,7 +609,7 @@ function PopupMARCFieldDoc(field) {
var tag=tagsubfieldid.substr(0,3);
var subfield=tagsubfieldid.substr(3,1);
StrAlert += "<li>"+_("Tag %s subfield %s %s in tab %s").format(tag, subfield, formatFieldName( tabflag[tagsubfieldid][1] ), tabflag[tagsubfieldid][2]) + ' <a class="linkfield btn btn-link" href="#" data-tab="' + tabflag[tagsubfieldid][2] + '" data-field="' + tabflag[tagsubfieldid][3] + '"><i class="fa fa-arrow-right" aria-hidden="true"></i> ' + _("Go to field") + '</a></li>';
flag=1;
flag = true;
}
}
StrAlert += "</ul>";
@ -663,7 +663,7 @@ function PopupMARCFieldDoc(field) {
}
if(isempty){
flag = 1;
flag = true;
if (mandatory) {
mandatoryFields[ arr[0] ] = {
importance: "mandatory",
@ -694,7 +694,7 @@ function PopupMARCFieldDoc(field) {
StrAlert += "</ul>";
}
StrAlert += "</div>";
if (flag) {
if ( flag ) {
$("#show-errors").html('<button type="button" class="btn btn-danger show-errors"><i class="fa fa-warning"></i> ' + _("Errors") + '</span>');
return StrAlert;
} else {
@ -703,33 +703,36 @@ function PopupMARCFieldDoc(field) {
}
/**
*
*
* Run checks for mandatory and important fields
* Output errors if necessary, or submit the form
*/
function Check(){
var StrAlert = AreFieldsNotOk();
if( ! StrAlert ){
var StrWarning = AreFieldsNotOk(false);
if (StrWarning){
var StrWarning = AreFieldsNotOk( false );
if( !StrAlert && StrWarning ){
// Check important fields
$("#check_errors").html( StrWarning );
$('html, body').animate({ scrollTop: 0 }, 'fast');
// Check important fields
$("#check_errors").html( AreFieldsNotOk(false) );
$('html, body').animate({ scrollTop: 0 }, 'fast');
var r=confirm(_("Important fields(s) are not filled. Are you sure you want to save?"));
if (! r){
return false;
}
var r=confirm( _("Important fields(s) are not filled. Are you sure you want to save?" ) );
if (! r){
return false;
} else {
document.f.submit();
return true;
}
document.f.submit();
return true;
} else {
// Call AreFieldsNotOk() twice to check both mandatory and important fields
$("#check_errors").html( AreFieldsNotOk() + AreFieldsNotOk(false) );
} else if( StrAlert ){
var strAll = StrAlert;
if( StrWarning ){
strAll += StrWarning;
}
$("#check_errors").html( strAll );
$('html, body').animate({ scrollTop: 0 }, 'fast');
Sticky.hcSticky('refresh');
return false;
} else if( !StrAlert && !StrWarning ){
document.f.submit();
return true;
}
}