I think that I've found a regression caused by https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19673. I'm investigating at the moment. I have the default SHOW_BCODE attribute category set up, and when running with the Environmental Variable DEBUG=1 in Apache and using Chrome on Windows 10, I'm getting database errors like this: DBD::mysql::st execute failed: Cannot add or update a child row: a foreign key constraint fails (`kohadb`.`borrower_attributes`, CONSTRAINT `borrower_attributes_ibfk_2` FOREIGN KEY (`code`) REFERENCES `borrower_attribute_types` (`code`) ON DELETE CASCADE ON UPDATE CASCADE) [for Statement "INSERT INTO borrower_attributes SET attribute = ?, code = ?, borrowernumber = ?" with ParamValues: 0=undef, 1="", 2="1"] at /koha/lib/C4/Members/Attributes.pm line 294. Note this is a modified Koha so the line number may not be relevant. It points to the final line of this function: sub UpdateBorrowerAttribute { my ( $borrowernumber, $attribute ) = @_; DeleteBorrowerAttribute $borrowernumber, $attribute; my $dbh = C4::Context->dbh; my $query = "INSERT INTO borrower_attributes SET attribute = ?, code = ?, borrowernumber = ?"; my @params = ( $attribute->{attribute}, $attribute->{code}, $borrowernumber ); my $sth = $dbh->prepare( $query ); $sth->execute( @params ); }
I find it odd that no one else has encountered this over the last year, but I'm guessing it might have to do with most people not running with RaiseError turned on (by DEBUG=1). I'm not sure what the best fix would be. One option is to maybe change the Javascript in modborrowers.tt. Another would be to update UpdateBorrowerAttribute() to look up the borrower_attribute_type/code before trying to do an insert, although that could have performance impacts on batch operations.
(In reply to David Cook from comment #1) > I find it odd that no one else has encountered this over the last year, but > I'm guessing it might have to do with most people not running with > RaiseError turned on (by DEBUG=1). > I can confirm that turning off DEBUG=1 makes the errors disappear, and turning it back on makes the errors re-appear.
(In reply to David Cook from comment #1) > Another would be to update UpdateBorrowerAttribute() to look up the > borrower_attribute_type/code before trying to do an insert, although that > could have performance impacts on batch operations. Oh actually in tools/modborrower.pl, we're already doing the following: my $attr_type = C4::Members::AttributeTypes->fetch( $_ ); We should just do some better validations there. Oh... $attr_type should be null, but then we do the following: ++$i and next if $attr_type->{category_code} and $attr_type->{category_code} ne $borrower_categorycode; That doesn't make any errors, so I wonder if that's an autovivification issue... I'll take a quick look...
(In reply to David Cook from comment #3) > That doesn't make any errors, so I wonder if that's an autovivification > issue... I'll take a quick look... Yep. That's exactly it. So need to first check the value of $attr_type when returned by C4::Members::AttributeTypes->fetch(). Just need to make sure the counter is still incremented correctly...
So were so close to the patch!
(In reply to Jonathan Druart from comment #5) > So were so close to the patch! *You* were so close to the patch!
Created attachment 91131 [details] [review] Bug 23217: Remove SQL error when batch patron mod and not attribute selected On the batch patron modification tool, if no patron attribute is selected then an UPDATE query will be executed anyway. Indeed the form will send a an empty "patron_attributes" parameter. We need to handle it. Test plan: Go to the patron modification tool Enter a cardnumber Change a field (like city) Do not change anything in the patron's attributes Save => Without this patch a SQL error is generated: DBD::mysql::st execute failed: Cannot add or update a child row: a foreign key constraint fails (`koha_kohadev`.`borrower_attributes`, CONSTRAINT `borrower_attributes_ibfk_2` FOREIGN KEY (`code`) REFERENCES `borrower_attribute_types` (`code`) ON DELETE CASCADE ON UPDATE CASCADE) [for Statement "INSERT INTO borrower_attributes SET attribute = ?, code = ?, borrowernumber = ?" with ParamValues: 0=undef, 1="", 2="51"] at /home/vagrant/kohaclone/C4/Members/Attributes.pm line 287. => With this patch applied you will not see it You should also test that you can modify patron's attributes ;)
Thanks, Jonathan. I'm working from home with a sick child today, but I'll take a look at signing off your patch when I'm back in the office (with a full size keyboard instead of a tiny netbook keyboard).
Created attachment 91785 [details] [review] Bug 23217: Remove SQL error when batch patron mod and not attribute selected On the batch patron modification tool, if no patron attribute is selected then an UPDATE query will be executed anyway. Indeed the form will send a an empty "patron_attributes" parameter. We need to handle it. Test plan: Go to the patron modification tool Enter a cardnumber Change a field (like city) Do not change anything in the patron's attributes Save => Without this patch a SQL error is generated: DBD::mysql::st execute failed: Cannot add or update a child row: a foreign key constraint fails (`koha_kohadev`.`borrower_attributes`, CONSTRAINT `borrower_attributes_ibfk_2` FOREIGN KEY (`code`) REFERENCES `borrower_attribute_types` (`code`) ON DELETE CASCADE ON UPDATE CASCADE) [for Statement "INSERT INTO borrower_attributes SET attribute = ?, code = ?, borrowernumber = ?" with ParamValues: 0=undef, 1="", 2="51"] at /home/vagrant/kohaclone/C4/Members/Attributes.pm line 287. => With this patch applied you will not see it You should also test that you can modify patron's attributes ;) Signed-off-by: Hayley Mapley <hayleymapley@catalyst.net.nz>
Created attachment 91808 [details] [review] Bug 23217: Remove SQL error when batch patron mod and not attribute selected On the batch patron modification tool, if no patron attribute is selected then an UPDATE query will be executed anyway. Indeed the form will send a an empty "patron_attributes" parameter. We need to handle it. Test plan: Go to the patron modification tool Enter a cardnumber Change a field (like city) Do not change anything in the patron's attributes Save => Without this patch a SQL error is generated: DBD::mysql::st execute failed: Cannot add or update a child row: a foreign key constraint fails (`koha_kohadev`.`borrower_attributes`, CONSTRAINT `borrower_attributes_ibfk_2` FOREIGN KEY (`code`) REFERENCES `borrower_attribute_types` (`code`) ON DELETE CASCADE ON UPDATE CASCADE) [for Statement "INSERT INTO borrower_attributes SET attribute = ?, code = ?, borrowernumber = ?" with ParamValues: 0=undef, 1="", 2="51"] at /home/vagrant/kohaclone/C4/Members/Attributes.pm line 287. => With this patch applied you will not see it You should also test that you can modify patron's attributes ;) Signed-off-by: Hayley Mapley <hayleymapley@catalyst.net.nz> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Nice work! Pushed to master for 19.11.00
Pushed to 19.05.x for 19.05.03