[Koha-bugs] [Bug 20446] QUOTES processing broken by run time error
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20446 Martin Renvoize changed: What|Removed |Added Status|Pushed to Stable|RESOLVED Resolution|--- |FIXED --- Comment #35 from Martin Renvoize --- Already in 17.11.x, marking as RESOLVED FIXED. -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20446] QUOTES processing broken by run time error
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20446 Martin Renvoize changed: What|Removed |Added CC||martin.renvoize@ptfs-europe ||.com Status|Pushed to Master|Pushed to Stable -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20446] QUOTES processing broken by run time error
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20446 Jonathan Druart changed: What|Removed |Added Status|Passed QA |Pushed to Master --- Comment #34 from Jonathan Druart --- Pushed to master for 18.05, thanks to everybody involved! -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20446] QUOTES processing broken by run time error
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20446 --- Comment #33 from Jonathan Druart --- (In reply to Colin Campbell from comment #31) > Any finds in quote processing are redundant > > logic is > > Create basket > if return is ok [; > create orderline > create orderline > ]; > > The find was introduced on a misconception. Yes, for now. But the logic could change later. Nowadays I have more concerns about consistency/maintainability rather than performance. Do not read this wrong: we are talking about a select on a primary key, so very limited impact. -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20446] QUOTES processing broken by run time error
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20446 Marcel de Rooy changed: What|Removed |Added CC||m.de.r...@rijksmuseum.nl QA Contact|testo...@bugs.koha-communit |m.de.r...@rijksmuseum.nl |y.org | Assignee|colin.campbell@ptfs-europe. |jonathan.dru...@bugs.koha-c |com |ommunity.org -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20446] QUOTES processing broken by run time error
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20446 Marcel de Rooy changed: What|Removed |Added Attachment #73736|0 |1 is obsolete|| --- Comment #32 from Marcel de Rooy --- Created attachment 73756 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=73756&action=edit Bug 20446: Fix Edifact quotes processing Caused by commit 04aea91de0f2fe1103e4021f880d135da1fd11a9 Bug 15685: (QA follow-up) Address QA issues ->find is called on Koha::Object instead of the set class (Koha::Objects) and raises the following error: Can't use string ("Koha::Acquisition::Basket") as a HASH ref while "strict refs" in use at /usr/share/koha/lib/Koha/Object.pm line 275. This patch also makes sure $basketno refers to an existing basket in DB I cannot provide a test plan, I have no idea how this code is used. Signed-off-by: Josef Moravec Signed-off-by: Marcel de Rooy -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20446] QUOTES processing broken by run time error
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20446 Marcel de Rooy changed: What|Removed |Added Status|Signed Off |Passed QA Patch complexity|--- |Trivial patch -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20446] QUOTES processing broken by run time error
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20446 --- Comment #31 from Colin Campbell --- Any finds in quote processing are redundant logic is Create basket if return is ok [; create orderline create orderline ]; The find was introduced on a misconception. -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20446] QUOTES processing broken by run time error
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20446 Josef Moravec changed: What|Removed |Added CC||josef.mora...@gmail.com Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20446] QUOTES processing broken by run time error
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20446 Josef Moravec changed: What|Removed |Added Attachment #73424|0 |1 is obsolete|| --- Comment #30 from Josef Moravec --- Created attachment 73736 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=73736&action=edit Bug 20446: Fix Edifact quotes processing Caused by commit 04aea91de0f2fe1103e4021f880d135da1fd11a9 Bug 15685: (QA follow-up) Address QA issues ->find is called on Koha::Object instead of the set class (Koha::Objects) and raises the following error: Can't use string ("Koha::Acquisition::Basket") as a HASH ref while "strict refs" in use at /usr/share/koha/lib/Koha/Object.pm line 275. This patch also makes sure $basketno refers to an existing basket in DB I cannot provide a test plan, I have no idea how this code is used. Signed-off-by: Josef Moravec -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20446] QUOTES processing broken by run time error
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20446 --- Comment #29 from Colin Campbell --- Its not sensible it adds a lot of code to test something we already have the result of. The only basket info we want is the basketno, returned on creation which is passed as a parameter Add unnecessary code is surely potentially buggy -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20446] QUOTES processing broken by run time error
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20446 --- Comment #28 from Jonathan Druart --- I have attached an alternative patch for this issue. Could you please test and confirm it makes sense? I have tried but failed writing tests for this change. -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20446] QUOTES processing broken by run time error
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20446 Jonathan Druart changed: What|Removed |Added Attachment #73156|0 |1 is obsolete|| --- Comment #27 from Jonathan Druart --- Created attachment 73424 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=73424&action=edit Bug 20446: Fix Edifact quotes processing Caused by commit 04aea91de0f2fe1103e4021f880d135da1fd11a9 Bug 15685: (QA follow-up) Address QA issues ->find is called on Koha::Object instead of the set class (Koha::Objects) and raises the following error: Can't use string ("Koha::Acquisition::Basket") as a HASH ref while "strict refs" in use at /usr/share/koha/lib/Koha/Object.pm line 275. This patch also makes sure $basketno refers to an existing basket in DB I cannot provide a test plan, I have no idea how this code is used. -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20446] QUOTES processing broken by run time error
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20446 Jonathan Druart changed: What|Removed |Added Status|Passed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20446] QUOTES processing broken by run time error
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20446 --- Comment #26 from Tomás Cohen Arazi --- (In reply to Tomás Cohen Arazi from comment #25) > (In reply to Jonathan Druart from comment #22) > > It's more about consistency: AcqCreateItem pref's value should not be > > retrieved directly, the logic should be stay in Koha::Acquisition::Basket > > Both solutions do the job, but given the fact we have an encapsulated > business-logic behind 'effective_create_item', the best way to do it is to > use the encapsulated one, because any changes on that business logic or > (new?) pref combinations wouldn't have an impact on the EDI code itself. Its a pity I'm not seeing regression tests for this btw. -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20446] QUOTES processing broken by run time error
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20446 Tomás Cohen Arazi changed: What|Removed |Added CC||tomasco...@gmail.com --- Comment #25 from Tomás Cohen Arazi --- (In reply to Jonathan Druart from comment #22) > It's more about consistency: AcqCreateItem pref's value should not be > retrieved directly, the logic should be stay in Koha::Acquisition::Basket Both solutions do the job, but given the fact we have an encapsulated business-logic behind 'effective_create_item', the best way to do it is to use the encapsulated one, because any changes on that business logic or (new?) pref combinations wouldn't have an impact on the EDI code itself. -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20446] QUOTES processing broken by run time error
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20446 Bug 20446 depends on bug 15685, which changed state. Bug 15685 Summary: Allow creation of items (AcqCreateItem) to be customizable per-basket https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15685 What|Removed |Added Status|Pushed to Master|RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20446] QUOTES processing broken by run time error
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20446 Fridolin SOMERS changed: What|Removed |Added CC||fridolin.som...@biblibre.co ||m --- Comment #24 from Fridolin SOMERS --- Bug 15685 not in 17.05.x -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20446] QUOTES processing broken by run time error
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20446 --- Comment #23 from Nick Clemens --- Picking this for 17.11.04 to restore functionality - will watch here for final resolution and update as needed -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20446] QUOTES processing broken by run time error
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20446 --- Comment #22 from Jonathan Druart --- It's more about consistency: AcqCreateItem pref's value should not be retrieved directly, the logic should be stay in Koha::Acquisition::Basket -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20446] QUOTES processing broken by run time error
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20446 --- Comment #21 from Colin Campbell --- (In reply to Jonathan Druart from comment #20) > The original problem is that we are calling ->find on Koha::Object instead > of the set class (Koha::Objects) > > -my $basket = Koha::Acquisition::Basket->find( $basketno ); > +my $basket = Koha::Acquisition::Baskets->find( $basketno ); > > Sounds like the correct fix to me. Could someone confirm? Read the above comments. There is no need to retrieve the basket which you have just created. It was being retrieved to find a parameter that was set based on the systempreference. It introduce3s extra processing for no purpose -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20446] QUOTES processing broken by run time error
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20446 --- Comment #20 from Jonathan Druart --- The original problem is that we are calling ->find on Koha::Object instead of the set class (Koha::Objects) -my $basket = Koha::Acquisition::Basket->find( $basketno ); +my $basket = Koha::Acquisition::Baskets->find( $basketno ); Sounds like the correct fix to me. Could someone confirm? -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20446] QUOTES processing broken by run time error
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20446 Jonathan Druart changed: What|Removed |Added Depends on||15685 CC||jonathan.dru...@bugs.koha-c ||ommunity.org See Also|https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=15685 | --- Comment #19 from Jonathan Druart --- Caused by commit 04aea91de0f2fe1103e4021f880d135da1fd11a9 Bug 15685: (QA follow-up) Address QA issues Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15685 [Bug 15685] Allow creation of items (AcqCreateItem) to be customizable per-basket -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20446] QUOTES processing broken by run time error
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20446 Kyle M Hall changed: What|Removed |Added Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20446] QUOTES processing broken by run time error
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20446 Kyle M Hall changed: What|Removed |Added Attachment #73104|0 |1 is obsolete|| --- Comment #18 from Kyle M Hall --- Created attachment 73156 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=73156&action=edit Bug 20446 Revert to using syspref for item creation Using a per-basket setting is nonsensical when baskets are created automatically by quote processing a few lines before this attempts to read it. cron job aborts with a runtime error leaving basket for quote half-created. This reverts to the previous code using the global setting. (Basket itself will have been set to the global setting by default) Signed-off-by: Kyle M Hall -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20446] QUOTES processing broken by run time error
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20446 --- Comment #17 from Katrin Fischer --- Kyle, can you QA please? -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20446] QUOTES processing broken by run time error
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20446 --- Comment #16 from Kyle M Hall --- (In reply to Colin Campbell from comment #7) > Kyle your patch is illogical the baskets in question CANNOT override the > syspreference, because we have just created them in accordance with that > syspreference microseconmds before. > Baskets can override the systempreference if there is user interaction to > make that choice, the whole point of electronic ordering is that there is no > such interaction and the basket is generated from the QUOTES message > directly with no user interaction. > reinstanting the original patch I see you are correct, I hadn't read the calling code where the basket was created. My bad! -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20446] QUOTES processing broken by run time error
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20446 M. Tompsett changed: What|Removed |Added Status|Failed QA |Signed Off --- Comment #15 from M. Tompsett --- Katrin makes a valid point. Sign off now, tests later. I didn't officially sign off, because I just shut down my work computer where I was signing off. But I'll set to Signed Off, because it truly is an identical reversion. -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20446] QUOTES processing broken by run time error
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20446 --- Comment #14 from Katrin Fischer --- We can ask why no tests was submitted initially OR when the new feature was added, but this doesn't change that this is a critical bug and we broke an important acquisitions workflow. Tomorrow releases might happen already. If we want to have a chance to get this included, we need to move fast. Can someone verify the change now is correct and sign off? I suggest documenting the need for tests on a separate bug for now. Even better of course, if someone provides them. -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20446] QUOTES processing broken by run time error
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20446 --- Comment #13 from Colin Campbell --- (In reply to M. Tompsett from comment #12) > My mistake. Colin is correct. misc/cronjobs/edi_cron.pl calls > process_quote() in Koha::EDI, which calls quote_item in Koha::EDI. As far as > I can tell there is no other way to trigger quote_item. And so, I believe > the original patch was more correct, in that it is simpler. So apologies for > any frustrations. > > *IF* in the future, someone wanted to manually integrate EDI quote > processing -- it would be useful to see debugging feedback step by step -- > Kyle's patch makes sense. > > However, I still believe there should be test coverage. There is no test > coverage of Koha::EDI. This needs to be solved. It is a serious hole. Test > coverage for quote_item and/or process_quote would be good, leaving Failed > QA. patch reinstates a call to C4::Context - do we need another test to show that works? QA failed not for this patch, but for more global concerns, I'd argur thats counterproductive - or shall we just remove this functionality from Koha -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20446] QUOTES processing broken by run time error
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20446 --- Comment #12 from M. Tompsett --- My mistake. Colin is correct. misc/cronjobs/edi_cron.pl calls process_quote() in Koha::EDI, which calls quote_item in Koha::EDI. As far as I can tell there is no other way to trigger quote_item. And so, I believe the original patch was more correct, in that it is simpler. So apologies for any frustrations. *IF* in the future, someone wanted to manually integrate EDI quote processing -- it would be useful to see debugging feedback step by step -- Kyle's patch makes sense. However, I still believe there should be test coverage. There is no test coverage of Koha::EDI. This needs to be solved. It is a serious hole. Test coverage for quote_item and/or process_quote would be good, leaving Failed QA. -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20446] QUOTES processing broken by run time error
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20446 --- Comment #11 from Colin Campbell --- (In reply to Colin Campbell from comment #10) > There is and cannot be a user interaction - you bare processing a file there > is no user satr at a terminal. This is an offline process. -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20446] QUOTES processing broken by run time error
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20446 --- Comment #10 from Colin Campbell --- There is and cannot be a user interaction - you bare processing a file there is no user satr at a terminal. -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20446] QUOTES processing broken by run time error
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20446 M. Tompsett changed: What|Removed |Added Status|Needs Signoff |Failed QA --- Comment #9 from M. Tompsett --- (In reply to Colin Campbell from comment #7) > Kyle your patch is illogical the baskets in question CANNOT override the > syspreference, because we have just created them in accordance with that > syspreference microseconmds before. > Baskets can override the systempreference if there is user interaction to > make that choice, the whole point of electronic ordering is that there is no > such interaction and the basket is generated from the QUOTES message > directly with no user interaction. > reinstanting the original patch But his change allows for the case where there IS user interaction. And if it was created microseconds before, the values should be identical. If it is user interaction later, why should a system preference change after the order creation change the behaviour? Once the order is created, should you not change the behaviour on the order, not forcefully via system preference? This is why tests are very much needed, because then we'd know how it is supposed to behave, because the tests would demonstrate trigger cases. Marking Failed QA until I see tests. -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20446] QUOTES processing broken by run time error
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20446 Colin Campbell changed: What|Removed |Added Attachment #73103|0 |1 is obsolete|| --- Comment #8 from Colin Campbell --- Created attachment 73104 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=73104&action=edit proposed patch to allow users to use acquisitions via EDI -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20446] QUOTES processing broken by run time error
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20446 --- Comment #7 from Colin Campbell --- Kyle your patch is illogical the baskets in question CANNOT override the syspreference, because we have just created them in accordance with that syspreference microseconmds before. Baskets can override the systempreference if there is user interaction to make that choice, the whole point of electronic ordering is that there is no such interaction and the basket is generated from the QUOTES message directly with no user interaction. reinstanting the original patch -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20446] QUOTES processing broken by run time error
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20446 --- Comment #6 from Kyle M Hall --- (In reply to Colin Campbell from comment #4) > Where was the test when this code was introduced to break EDI? A lack of a test does not obviate our responsibility to add regression tests where possible and reasonable. Adding unit tests while fixing regressions in Koha is a very common occurrence. I do understand the frustration as I've been in this situation dozens of times ; ) I notice that there unit test files for invoices and orders, but not quotes. Is there any reason quotes weren't unit tested when introduced? I'll leave it to cait of mtompsett to decide if unit tests are necessary. > Basically people cant run Edifact EDI with the addition of this code > It undoes a change that should never have been made Your patch seems to introduce a regression of its own. Baskets in Koha can now override AcqCreateItem with their own alternative setting. The patch I've submitted should take care of the issue and keep support for the new feature! -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20446] QUOTES processing broken by run time error
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20446 Kyle M Hall changed: What|Removed |Added Attachment #73090|0 |1 is obsolete|| --- Comment #5 from Kyle M Hall --- Created attachment 73103 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=73103&action=edit Bug 20446: QUOTES processing broken by run time error -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20446] QUOTES processing broken by run time error
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20446 Mason James changed: What|Removed |Added See Also||https://bugs.koha-community ||.org/bugzilla3/show_bug.cgi ||?id=15685 CC||m...@kohaaloha.com -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20446] QUOTES processing broken by run time error
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20446 Katrin Fischer changed: What|Removed |Added CC||katrin.fisc...@bsz-bw.de, ||k...@bywatersolutions.com, ||n...@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20446] QUOTES processing broken by run time error
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20446 --- Comment #4 from Colin Campbell --- Where was the test when this code was introduced to break EDI? Basically people cant run Edifact EDI with the addition of this code It undoes a change that should never have been made -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20446] QUOTES processing broken by run time error
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20446 M. Tompsett changed: What|Removed |Added CC||mtomp...@hotmail.com --- Comment #3 from M. Tompsett --- While this change is simple enough: 1) Where's the test plan? (as if I have no clue what Edifact quotes are and how to trigger them -- which, sadly, I do.) 2) Why is there no use Koha::Acquisition::Basket? 3) Is it supposed to be Basket or Baskets? 4) As such, is this the correct way to fix the problem? -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20446] QUOTES processing broken by run time error
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20446 Colin Campbell changed: What|Removed |Added Assignee|koha-b...@lists.koha-commun |colin.campbell@ptfs-europe. |ity.org |com Status|NEW |Needs Signoff --- Comment #2 from Colin Campbell --- Created attachment 73090 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=73090&action=edit proposed patch restoring working code Restores the existing working code that used the system preference. The per-basket setting has been set based on that systempreference when loading the quote which this orderline is part, so the checking the value was redundant at this point -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20446] QUOTES processing broken by run time error
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20446 Colin Campbell changed: What|Removed |Added Severity|enhancement |critical --- Comment #1 from Colin Campbell --- NB Basket will have been created with value defaulting to the syspref -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/