Bug 16208

Summary: Can't delete a library EAN if the EAN value is empty
Product: Koha Reporter: Owen Leonard <oleonard>
Component: AcquisitionsAssignee: Kyle M Hall <kyle>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: brendan, chris, julian.maurice, katrin.fischer, kyle
Version: Main   
Hardware: All   
OS: All   
URL: /cgi-bin/koha/admin/edi_ean_accounts.pl
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 7736, 16206    
Bug Blocks: 16256    
Attachments: Bug 16208 - Can't delete a library EAN if the EAN value is empty
Bug 16208 - Can't delete a library EAN if the EAN value is empty
Bug 16208 - Can't delete a library EAN if the EAN value is empty
[PASSED QA] Bug 16208 - Can't delete a library EAN if the EAN value is empty
Bug 16208 [QA Followup] - Fix incorrect form parameter names
[PASSED QA] Bug 16208 - Can't delete a library EAN if the EAN value is empty
[PASSED QA] Bug 16208 [QA Followup] - Fix incorrect form parameter names

Description Owen Leonard 2016-04-05 17:00:24 UTC
In testing the new EDI pages I tried adding a "Library EAN" with no fields filled out (other than the pre-filled options). The record saved successfully with empty values for EAN and description.

When I went to test deletion of one of these records I got an error: 

Can't call method "delete" on an undefined value at /admin/edi_ean_accounts.pl line 112.

I notice the link to delete passes the ean value (which is empty in my test):

/admin/edi_ean_accounts.pl?op=delete_confirm&branchcode=APL&ean=

I assume this is the reason for the error.

If the edifact_ean table has a primary key (ee_id), why not use that for deletion?

Should the EAN field be required?
Comment 1 Kyle M Hall 2016-04-18 16:11:37 UTC Comment hidden (obsolete)
Comment 2 Kyle M Hall 2016-04-18 16:13:06 UTC Comment hidden (obsolete)
Comment 3 Owen Leonard 2016-04-18 17:49:50 UTC
Bug 16206, "Corrections to templates related new EDI feature" is Passed QA. Could this patch be rebased on top of that?
Comment 4 Chris Cormack 2016-04-19 00:19:59 UTC Comment hidden (obsolete)
Comment 5 Katrin Fischer 2016-04-19 05:46:48 UTC Comment hidden (obsolete)
Comment 6 Katrin Fischer 2016-04-19 05:52:08 UTC
I am sorry... but I just noticed: it didn't delete!
When confirming yes, nothing happens.

From the logs:
[Tue Apr 19 07:49:37.687937 2016] [cgi:error] [pid 17497] [client 127.0.0.1:37847] AH01215: [Tue Apr 19 07:49:37 2016] edi_ean_accounts.pl: DBIx::Class::ResultSet::_build_unique_cond(): NULL/undef values supplied for requested unique constraint 'primary' (NULL values in column(s): 'ee_id'). This is almost certainly not what you wanted, though you can set DBIC_NULLABLE_KEY_NOWARN to disable this warning. at /home/katrin/kohaclone/admin/edi_ean_accounts.pl line 140, referer: http://localhost:8080/cgi-bin/koha/admin/edi_ean_accounts.pl
Comment 7 Katrin Fischer 2016-04-19 05:55:13 UTC
Looking at the form for confirming deletion shows the hidden value for ean to be still empty:
<input type="hidden" name="ean" value="" />
Comment 8 Kyle M Hall 2016-04-19 10:28:57 UTC Comment hidden (obsolete)
Comment 9 Katrin Fischer 2016-04-19 22:23:20 UTC
Created attachment 50445 [details] [review]
[PASSED QA] Bug 16208 - Can't delete a library EAN if the EAN value is empty

Test Plan:
1) Create an empty EAN
2) Attempt to delete it, you should get an error
3) Apply this patch
4) Attempt to delete the EAN, you should now be able to

Signed-off-by: Chris Cormack <chrisc@catalyst.net.z>

This depends on bug 16206 being pushed, or you need to apply that patch
first

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

https://bugs.koha-community.org/show_bug.cgi?id=16256

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 10 Katrin Fischer 2016-04-19 22:23:56 UTC
Created attachment 50446 [details] [review]
[PASSED QA] Bug 16208 [QA Followup] - Fix incorrect form parameter names

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 11 Brendan Gallagher 2016-04-20 16:51:45 UTC
Pushed to Master - Should be in the May 2016 Release.  Thanks!
Comment 12 Julian Maurice 2016-04-21 07:29:40 UTC
Not needed in stable versions, status changed to RESOLVED