Attachment 171363 Details for Bug 36197 – Bug 36197: Update how form params are validated

[patch] Bug 36197: Update how form params are validated

Bug-36197-Update-how-form-params-are-validated.patch (text/plain), 5.58 KB, created by Pedro Amorim on 2024-09-12 09:34:51 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Pedro Amorim

Created: 2024-09-12 09:34:51 UTC

Size: 5.58 KB

patch

obsolete

>From 377f9a52ad7c2c44096693edbc4d9ed02b90de52 Mon Sep 17 00:00:00 2001
>From: Pedro Amorim <pedro.amorim@ptfs-europe.com>
>Date: Tue, 5 Mar 2024 17:03:06 +0000
>Subject: [PATCH] Bug 36197: Update how form params are validated
>
>Validating an unauth request form requires checking for unauth info
>fields.
>Validating a regular authenticated request requires checking for
>borrower information.
>
>Signed-off-by: David Nind <david@davidnind.com>
>---
> Koha/ILL/Backend/Standard.pm | 104 ++++++++++++++++------
> Koha/ILL/Backend/opac-includes/create.inc | 2 +
> 2 files changed, 79 insertions(+), 27 deletions(-)
>
>diff --git a/Koha/ILL/Backend/Standard.pm b/Koha/ILL/Backend/Standard.pm
>index 87b7ac5ff6..dadc7d8169 100644
>--- a/Koha/ILL/Backend/Standard.pm
>+++ b/Koha/ILL/Backend/Standard.pm
>@@ -275,9 +275,6 @@ sub create {
> }
> 
> # Received completed details of form. Validate and create request.
>- ## Validate
>- my ( $brw_count, $brw ) =
>- _validate_borrower( $other->{'cardnumber'} );
> my $result = {
> cwd => dirname(__FILE__),
> status => "",
>@@ -289,31 +286,36 @@ sub create {
> core => $core_fields
> };
> my $failed = 0;
>- if ( !$other->{'type'} ) {
>- $result->{status} = "missing_type";
>- $result->{value} = $params;
>- $failed = 1;
>- } elsif ( !$other->{'branchcode'} ) {
>- $result->{status} = "missing_branch";
>- $result->{value} = $params;
>- $failed = 1;
>- } elsif ( !Koha::Libraries->find( $other->{'branchcode'} ) ) {
>- $result->{status} = "invalid_branch";
>- $result->{value} = $params;
>- $failed = 1;
>- } elsif ( $brw_count == 0 ) {
>- $result->{status} = "invalid_borrower";
>- $result->{value} = $params;
>- $failed = 1;
>- } elsif ( $brw_count > 1 ) {
>-
>- # We must select a specific borrower out of our options.
>- $params->{brw} = $brw;
>- $result->{value} = $params;
>- $result->{stage} = "borrowers";
>- $result->{error} = 0;
>- $failed = 1;
>+
>+ my $unauthenticated_request =
>+ C4::Context->preference("ILLOpacUnauthenticatedRequest") && !$other->{'cardnumber'};
>+ if($unauthenticated_request){
>+ ( $failed, $result ) = _validate_form_params( $other, $result, $params);
>+ if ( !_unauth_request_data_check($other) ) {
>+ $result->{status} = "missing_unauth_data";
>+ $result->{value} = $params;
>+ $failed = 1;
>+ }
>+ }else{
>+ ( $failed, $result ) = _validate_form_params( $other, $result, $params );
>+
>+ my ( $brw_count, $brw ) =
>+ _validate_borrower( $other->{'cardnumber'} );
>+
>+ if ( $brw_count == 0 ) {
>+ $result->{status} = "invalid_borrower";
>+ $result->{value} = $params;
>+ $failed = 1;
>+ } elsif ( $brw_count > 1 ) {
>+ # We must select a specific borrower out of our options.
>+ $params->{brw} = $brw;
>+ $result->{value} = $params;
>+ $result->{stage} = "borrowers";
>+ $result->{error} = 0;
>+ $failed = 1;
>+ }
> }
>+
> return $result if $failed;
> 
> $self->add_request( { request => $params->{request}, other => $other } );
>@@ -1215,6 +1217,54 @@ sub _set_suppression {
> return 1;
> }
> 
>+=head3 _unauth_request_data_check
>+
>+ _unauth_request_data_check($other);
>+
>+Checks if unauthenticated request data is present
>+
>+=cut
>+
>+sub _unauth_request_data_check {
>+ my ($other) = @_;
>+
>+ return 1 unless C4::Context->preference("ILLOpacUnauthenticatedRequest");
>+
>+ return
>+ $other->{unauthenticated_first_name}
>+ && $other->{unauthenticated_last_name}
>+ && $other->{unauthenticated_email};
>+}
>+
>+=head3 _validate_form_params
>+
>+ _validate_form_params( $other, $result, $params );
>+
>+Validate form parameters and return the validation result
>+
>+=cut
>+
>+sub _validate_form_params {
>+ my ( $other, $result, $params ) = @_;
>+
>+ my $failed = 0;
>+ if ( !$other->{'type'} ) {
>+ $result->{status} = "missing_type";
>+ $result->{value} = $params;
>+ $failed = 1;
>+ } elsif ( !$other->{'branchcode'} ) {
>+ $result->{status} = "missing_branch";
>+ $result->{value} = $params;
>+ $failed = 1;
>+ } elsif ( !Koha::Libraries->find( $other->{'branchcode'} ) ) {
>+ $result->{status} = "invalid_branch";
>+ $result->{value} = $params;
>+ $failed = 1;
>+ }
>+
>+ return ( $failed, $result );
>+}
>+
> =head1 AUTHORS
> 
> Alex Sassmannshausen <alex.sassmannshausen@ptfs-europe.com>
>diff --git a/Koha/ILL/Backend/opac-includes/create.inc b/Koha/ILL/Backend/opac-includes/create.inc
>index 50025fe3ab..b6b7dc5e61 100644
>--- a/Koha/ILL/Backend/opac-includes/create.inc
>+++ b/Koha/ILL/Backend/opac-includes/create.inc
>@@ -7,6 +7,8 @@
> Please Note: Mandatory field Identifier is missing.
> [% ELSIF whole.status == 'missing_type' %]
> Please Note: Type is a mandatory field.
>+[% ELSIF whole.status == 'missing_unauth_data' %]
>+Please Note: Patron data (first name, last name and e-mail) are mandatory fields.
> [% ELSIF whole.status == 'missing_branch' %]
> Please Note: Branch is a mandatory field.
> [% ELSIF whole.status == 'invalid_borrower' %]
>-- 
>2.39.2

Actions: View | Diff | Splinter Review

Attachments on bug 36197: 171358 | 171359 | 171360 | 171361 | 171362 | 171363 | 171364 | 171365 | 171366 | 171367 | 171368 | 171369 | 171370 | 171371 | 174495 | 174496 | 174497 | 174498 | 174499 | 174500 | 174501 | 174502 | 174503 | 174504 | 174505 | 174506 | 174507 | 174508