Bug 16208 - Can't delete a library EAN if the EAN value is empty
Summary: Can't delete a library EAN if the EAN value is empty
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Kyle M Hall
QA Contact: Testopia
URL: /cgi-bin/koha/admin/edi_ean_accounts.pl
Keywords:
Depends on: 7736 16206
Blocks: 16256
  Show dependency treegraph
 
Reported: 2016-04-05 17:00 UTC by Owen Leonard
Modified: 2016-12-05 21:23 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 16208 - Can't delete a library EAN if the EAN value is empty (3.41 KB, patch)
2016-04-18 16:11 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 16208 - Can't delete a library EAN if the EAN value is empty (3.40 KB, patch)
2016-04-18 16:13 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 16208 - Can't delete a library EAN if the EAN value is empty (3.71 KB, patch)
2016-04-19 00:19 UTC, Chris Cormack
Details | Diff | Splinter Review
[PASSED QA] Bug 16208 - Can't delete a library EAN if the EAN value is empty (3.83 KB, patch)
2016-04-19 05:46 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 16208 [QA Followup] - Fix incorrect form parameter names (1.84 KB, patch)
2016-04-19 10:28 UTC, Kyle M Hall
Details | Diff | Splinter Review
[PASSED QA] Bug 16208 - Can't delete a library EAN if the EAN value is empty (3.89 KB, patch)
2016-04-19 22:23 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 16208 [QA Followup] - Fix incorrect form parameter names (1.91 KB, patch)
2016-04-19 22:23 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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