Bugzilla – Attachment 86017 Details for
Bug 21761
Koha::Object supports passing through 'update' which means we can side step 'set' + 'store'
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 21761: Try to overload Koha::Object->update to handle incorrect values
Bug-21761-Try-to-overload-KohaObject-update-to-han.patch (text/plain), 6.75 KB, created by
Jonathan Druart
on 2019-03-04 17:00:15 UTC
(
hide
)
Description:
Bug 21761: Try to overload Koha::Object->update to handle incorrect values
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2019-03-04 17:00:15 UTC
Size:
6.75 KB
patch
obsolete
>From da7880144efc0b6636e839263abebb5ee578584e Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >Date: Sun, 3 Mar 2019 23:41:38 -0300 >Subject: [PATCH] Bug 21761: Try to overload Koha::Object->update to handle > incorrect values > >And use default values instead. > >This is a terrible patch, but publish it to help others having their >try. >--- > Koha/Object.pm | 123 ++++++++++++++++++++++++++++++++++--------- > t/db_dependent/Koha/Object.t | 3 ++ > 2 files changed, 101 insertions(+), 25 deletions(-) > >diff --git a/Koha/Object.pm b/Koha/Object.pm >index 1c382f26d3..b6a4118970 100644 >--- a/Koha/Object.pm >+++ b/Koha/Object.pm >@@ -118,37 +118,110 @@ Returns: > > =cut > >-sub store { >- my ($self) = @_; >+sub _fix_default_value { >+ my ($self, $params) = @_; >+ >+ my $column_info = $params->{column_info}; >+ my $col = $params->{col}; >+ my $val = exists $params->{val} ? $params->{val} : $self->$col; >+ >+ return unless $column_info; # the column does not exist >+ >+ my $reset; >+ # Integers >+ if ( _numeric_column_type( $column_info->{data_type} ) ) { >+ # Has been passed but not a number, usually an empty string >+ if ( defined $val and not looks_like_number( $val ) ) { >+ if ( $column_info->{is_nullable} ) { >+ # If nullable, default to null >+ $val = undef; >+ $reset = 1; >+ } else { >+ # If cannot be null, get the default value >+ # What if cannot be null and does not have a default value? Possible? >+ $val = $column_info->{default_value}; >+ $reset = 1; >+ } >+ } >+ } >+ elsif ( _date_or_datetime_column_type( $column_info->{data_type} ) ) { >+ # Set to null if an empty string (or == 0 but should not happen) >+ if ( defined $val and not $val ) { >+ if ( $column_info->{is_nullable} ) { >+ $val = undef; >+ $reset = 1; >+ } else { >+ $val = $column_info->{default_value}; >+ $reset = 1; >+ } >+ } >+ } >+ $self->$col($val) if $reset; >+ return $reset; >+} > >- my $columns_info = $self->_result->result_source->columns_info; >+sub update { >+ my ($self, $values) = @_; > >- # Handle not null and default values for integers and dates >- foreach my $col ( keys %{$columns_info} ) { >- # Integers >- if ( _numeric_column_type( $columns_info->{$col}->{data_type} ) ) { >- # Has been passed but not a number, usually an empty string >- if ( defined $self->$col and not looks_like_number( $self->$col ) ) { >- if ( $columns_info->{$col}->{is_nullable} ) { >- # If nullable, default to null >- $self->$col(undef); >- } else { >- # If cannot be null, get the default value >- # What if cannot be null and does not have a default value? Possible? >- $self->$col($columns_info->{$col}->{default_value}); >+ my $columns_info = $self->_result->result_source->columns_info; >+ my $new_values; >+ for my $col ( keys %$values ) { >+ if ( >+ $self->_fix_default_value( >+ { >+ col => $col, >+ column_info => $columns_info->{$col}, >+ val => $values->{$col} > } >- } >+ ) >+ ) >+ { >+ $values->{$col} = $self->$col; > } >- elsif ( _date_or_datetime_column_type( $columns_info->{$col}->{data_type} ) ) { >- # Set to null if an empty string (or == 0 but should not happen) >- if ( defined $self->$col and not $self->$col ) { >- if ( $columns_info->{$col}->{is_nullable} ) { >- $self->$col(undef); >- } else { >- $self->$col($columns_info->{$col}->{default_value}); >+ } >+ try { >+ return $self->_result->update($values) ? $self : undef; >+ } >+ catch { >+ # FIXME this is copied from ->store >+ >+ # Catch problems and raise relevant exceptions >+ if (ref($_) eq 'DBIx::Class::Exception') { >+ if ( $_->{msg} =~ /Cannot add or update a child row: a foreign key constraint fails/ ) { >+ # FK constraints >+ # FIXME: MySQL error, if we support more DB engines we should implement this for each >+ if ( $_->{msg} =~ /FOREIGN KEY \(`(?<column>.*?)`\)/ ) { >+ Koha::Exceptions::Object::FKConstraint->throw( >+ error => 'Broken FK constraint', >+ broken_fk => $+{column} >+ ); > } > } >+ elsif( $_->{msg} =~ /Duplicate entry '(.*?)' for key '(?<key>.*?)'/ ) { >+ Koha::Exceptions::Object::DuplicateID->throw( >+ error => 'Duplicate ID', >+ duplicate_id => $+{key} >+ ); >+ } >+ elsif( $_->{msg} =~ /Incorrect (?<type>\w+) value: '(?<value>.*)' for column '(?<property>\w+)'/ ) { >+ Koha::Exceptions::Object::BadValue->throw( >+ type => $+{type}, >+ value => $+{value}, >+ property => $+{property} >+ ); >+ } > } >+ Koha::Exceptions::Object->throw( ref($self) . "::update generated this error: " . $@ ); >+ } >+} >+ >+sub store { >+ my ($self) = @_; >+ >+ my $columns_info = $self->_result->result_source->columns_info; >+ # Handle not null and default values for integers and dates >+ foreach my $col ( keys %{$columns_info} ) { >+ $self->_fix_default_value({ col => $col, column_info => $columns_info->{$col} }); > } > > try { >@@ -452,7 +525,7 @@ sub AUTOLOAD { > } > } > >- my @known_methods = qw( is_changed id in_storage get_column discard_changes update make_column_dirty ); >+ my @known_methods = qw( is_changed id in_storage get_column discard_changes make_column_dirty ); > > Koha::Exceptions::Object::MethodNotCoveredByTests->throw( > error => sprintf("The method %s->%s is not covered by tests!", ref($self), $method), >diff --git a/t/db_dependent/Koha/Object.t b/t/db_dependent/Koha/Object.t >index fb57218afa..f1a2d600af 100755 >--- a/t/db_dependent/Koha/Object.t >+++ b/t/db_dependent/Koha/Object.t >@@ -345,6 +345,9 @@ subtest 'store() tests' => sub { > is( $itemtype->notforloan, undef, 'int DEFAULT NULL should default to null'); > is( $itemtype->hideinopac, 0, 'int NOT NULL DEFAULT 0 should default to 0'); > >+ $itemtype = $builder->build_object( { class => 'Koha::ItemTypes' }); >+ $itemtype->update({ notforloan => "" }); >+ > subtest 'Bad value tests' => sub { > > plan tests => 1; >-- >2.11.0
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 21761
:
86017
|
90914
|
95947
|
96136
|
96154
|
96155