[Koha-bugs] [Bug 20446] QUOTES processing broken by run time error

[bugzilla-daemon]

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

[bugzilla-daemon]

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

[bugzilla-daemon]

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

[bugzilla-daemon]

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

[bugzilla-daemon]

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

[bugzilla-daemon]

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=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

[bugzilla-daemon]

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

[bugzilla-daemon]

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

[bugzilla-daemon]

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

[bugzilla-daemon]

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=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

[bugzilla-daemon]

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

[bugzilla-daemon]

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

[bugzilla-daemon]

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=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

[bugzilla-daemon]

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

[bugzilla-daemon]

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

[bugzilla-daemon]

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

[bugzilla-daemon]

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

[bugzilla-daemon]

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

[bugzilla-daemon]

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

[bugzilla-daemon]

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

[bugzilla-daemon]

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

[bugzilla-daemon]

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

[bugzilla-daemon]

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

[bugzilla-daemon]

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

[bugzilla-daemon]

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=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

[bugzilla-daemon]

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

[bugzilla-daemon]

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

[bugzilla-daemon]

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

[bugzilla-daemon]

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

[bugzilla-daemon]

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

[bugzilla-daemon]

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

[bugzilla-daemon]

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

[bugzilla-daemon]

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

[bugzilla-daemon]

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

[bugzilla-daemon]

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=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

[bugzilla-daemon]

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

[bugzilla-daemon]

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

[bugzilla-daemon]

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=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

[bugzilla-daemon]

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

[bugzilla-daemon]

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

[bugzilla-daemon]

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

[bugzilla-daemon]

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

[bugzilla-daemon]

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=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

[bugzilla-daemon]

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/