[Bug 16739] New: generate edifact showing on basket groups
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16739 Bug ID: 16739 Summary: generate edifact showing on basket groups Change sponsored?: --- Product: Koha Version: master Hardware: All URL: cgi-bin/koha/acqui/basketgroup.pl?booksellerid=2&close d=1 OS: All Status: Needs Signoff Severity: normal Priority: P5 - low Component: Acquisitions Assignee: koha-bugs@lists.koha-community.org Reporter: nengard@gmail.com QA Contact: testopia@bugs.koha-community.org CC: katrin.fischer@bsz-bw.de, veron@veron.ch CC: veron@veron.ch Summary: generate edifact showing on basket groups See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi ?id=6740 See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi ?id=16740 CC: katrin.fischer@bsz-bw.de Status: Needs Signoff When looking at basketgroups the 'generate edifact order' button shows even if edi isn't set up for that vendor and leads to a 500 error page --- Comment #1 from Marc Véron <veron@veron.ch> --- Duplicate of 6740? --- Comment #2 from Marc Véron <veron@veron.ch> --- Sorry, I mean Bug 16740 - EDIFACT on Basket Groups leads to 500 error ( --- Comment #3 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- *** Bug 16740 has been marked as a duplicate of this bug. *** --- Comment #4 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Still valid. --- Comment #5 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Still valid. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16739 --- Comment #6 from Jon Knight <J.P.Knight@lboro.ac.uk> --- Created attachment 75173 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=75173&action=edit Bug 16739: generate edifact showing on basket groups When a book seller (vendor) does not have EDI account information configured the basketgroup display still shows a button to generate EDIFACT output which results in 500 error when clicked. This patch fixes two aspects of this: a) it stops the button being displayed in the first place, replacing it with a message that explains there is not EDIFACT configuration for the vendor. b) if, somehow, an edifact print operation is passed to the basketgroup script it detects the lack of an EAN and redirects back to the basket group display page with a warning message. To test: 1) Create a vendor with no EDI details. 2) Create a basket with some items in, then close it and add it to a basket group for this vendor. 3) Go to that vendor's basket groups, click on the "Closed" tab and then click on the 'generate edifact order' button. You should get a 500 error. 4) Apply this patch. 5) Repeat 3, except this time you should find that the 'generate edifact order' button has been replaced with a note that there is "No EDIFACT configuration for <vendor>". -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16739 Jon Knight <J.P.Knight@lboro.ac.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |J.P.Knight@lboro.ac.uk Assignee|koha-bugs@lists.koha-commun |J.P.Knight@lboro.ac.uk |ity.org | -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16739 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #7 from Owen Leonard <oleonard@myacpl.org> --- This looks like it's working correctly, but the QA tool has several complaints about tab indentation. Indentation should always be done with 4 spaces. There is one part of the template which I don't understand, starting at line 149: [% IF (ediaccount) %] <div class="btn-group"><a href="[% script_name %]?op=ediprint&basketgroupid=[% basketgroupid %]&booksellerid=[% booksellerid %]" class="btn btn-default btn-sm" id="printbutton"><i class="fa fa-download"></i> Generate EDIFACT order</a></div> [% ELSE %] <div class="btn-group"><a href="[% script_name %]?op=ediprint&basketgroupid=[% basketgroupid %]&booksellerid=[% booksellerid %]" class="btn btn-default btn-sm" id="printbutton"></div> [% END %] What is the purpose of the [% ELSE %] section? The <a> tag is incomplete, so I would expect this to break HTML rendering somehow. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16739 --- Comment #8 from Jon Knight <J.P.Knight@lboro.ac.uk> --- (In reply to Owen Leonard from comment #7)
This looks like it's working correctly, but the QA tool has several complaints about tab indentation. Indentation should always be done with 4 spaces.
There is one part of the template which I don't understand, starting at line 149:
[% IF (ediaccount) %] <div class="btn-group"><a href="[% script_name %]?op=ediprint&basketgroupid=[% basketgroupid %]&booksellerid=[% booksellerid %]" class="btn btn-default btn-sm" id="printbutton"><i class="fa fa-download"></i> Generate EDIFACT order</a></div> [% ELSE %] <div class="btn-group"><a href="[% script_name %]?op=ediprint&basketgroupid=[% basketgroupid %]&booksellerid=[% booksellerid %]" class="btn btn-default btn-sm" id="printbutton"></div> [% END %]
What is the purpose of the [% ELSE %] section? The <a> tag is incomplete, so I would expect this to break HTML rendering somehow.
Ah, good spot - I was putting in other text whilst testing the patch and left the rump in there (but it didn't display on the browser so I didn't notice it). I'll get rid of the else clause (and see if I can munge the indentation into something more acceptable!). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16739 Jon Knight <J.P.Knight@lboro.ac.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #75173|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16739 Jon Knight <J.P.Knight@lboro.ac.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16739 --- Comment #9 from Jon Knight <J.P.Knight@lboro.ac.uk> --- Created attachment 75316 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=75316&action=edit Bug 16739: generate edifact showing on basket groups When a book seller (vendor) does not have EDI account information configured the basketgroup display still shows a button to generate EDIFACT output which results in 500 error when clicked. This patch fixes two aspects of this: a) it stops the button being displayed in the first place, replacing it with a message that explains there is not EDIFACT configuration for the vendor. b) if, somehow, an edifact print operation is passed to the basketgroup script it detects the lack of an EAN and redirects back to the basket group display page with a warning message. To test: 1) Create a vendor with no EDI details. 2) Create a basket with some items in, then close it and add it to a basket group for this vendor. 3) Go to that vendor's basket groups, click on the "Closed" tab and then click on the 'generate edifact order' button. You should get a 500 error. 4) Apply this patch. 5) Repeat 3, except this time you should find that the 'generate edifact order' button has been replaced with a note that there is "No EDIFACT configuration for <vendor>". -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16739 Spencer <ssmith3@mckinneytexas.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |ssmith3@mckinneytexas.org --- Comment #10 from Spencer <ssmith3@mckinneytexas.org> --- Created attachment 79174 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=79174&action=edit screencaptrure It appears this patch does not apply. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16739 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |colin.campbell@ptfs-europe. |y.org |com CC| |martin.renvoize@ptfs-europe | |.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16739 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #75316|0 |1 is obsolete| | --- Comment #11 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 79466 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=79466&action=edit Bug 16739: generate edifact showing on basket groups When a book seller (vendor) does not have EDI account information configured the basketgroup display still shows a button to generate EDIFACT output which results in 500 error when clicked. This patch fixes two aspects of this: a) it stops the button being displayed in the first place, replacing it with a message that explains there is not EDIFACT configuration for the vendor. b) if, somehow, an edifact print operation is passed to the basketgroup script it detects the lack of an EAN and redirects back to the basket group display page with a warning message. To test: 1) Create a vendor with no EDI details. 2) Create a basket with some items in, then close it and add it to a basket group for this vendor. 3) Go to that vendor's basket groups, click on the "Closed" tab and then click on the 'generate edifact order' button. You should get a 500 error. 4) Apply this patch. 5) Repeat 3, except this time you should find that the 'generate edifact order' button has been replaced with a note that there is "No EDIFACT configuration for <vendor>". Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16739 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off --- Comment #12 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Patch rescued (just needed the filters adding for the xss fixes), and signed off. Thanks Jon -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16739 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|generate edifact showing on |Generate EDIFACT on basket |basket groups |groups falsely showing when | |configuration is incomplete -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16739 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16739 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #79466|0 |1 is obsolete| | --- Comment #13 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 79484 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=79484&action=edit Bug 16739: Show note instead of 500 error for 'Generate EDIFACT' when no EDI account is configured When a book seller (vendor) does not have EDI account information configured the basketgroup display still shows a button to generate EDIFACT output which results in 500 error when clicked. This patch fixes two aspects of this: a) it stops the button being displayed in the first place, replacing it with a message that explains there is not EDIFACT configuration for the vendor. b) if, somehow, an edifact print operation is passed to the basketgroup script it detects the lack of an EAN and redirects back to the basket group display page with a warning message. To test: 1) Create a vendor with no EDI details. 2) Create a basket with some items in, then close it and add it to a basket group for this vendor. 3) Go to that vendor's basket groups, click on the "Closed" tab and then click on the 'generate edifact order' button. You should get a 500 error. 4) Apply this patch. 5) Repeat 3, except this time you should find that the 'generate edifact order' button has been replaced with a note that there is "No EDIFACT configuration for <vendor>". Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16739 --- Comment #14 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- This is a big improvement over the 500 error, so PQA! While testing I was wondering - maybe we should not show this info at all, when there is no configuration assuming the library is not using EDIFACT at all? Might be a broader discussion to be had - see also bug 16258 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16739 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master CC| |nick@bywatersolutions.com --- Comment #15 from Nick Clemens <nick@bywatersolutions.com> --- Awesome work all! Pushed to master for 18.11 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16739 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |Pushed to Stable --- Comment #16 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Pushed to 18.05.x for 18.05.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16739 Fridolin SOMERS <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m --- Comment #17 from Fridolin SOMERS <fridolin.somers@biblibre.com> --- Pushed to 17.11.x for 17.11.11 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16739 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED Status|Pushed to Stable |RESOLVED -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org