Bug 6893

Summary: Order from suggestion does not remove suggestion from 'accepted' list
Product: Koha Reporter: Katrin Fischer <katrin.fischer>
Component: AcquisitionsAssignee: Adrien SAURAT <adrien.saurat>
Status: CLOSED FIXED QA Contact: Bugs List <koha-bugs>
Severity: critical    
Priority: PATCH-Sent (DO NOT USE) CC: francois.charbonnier, katrin.fischer, paul.poulain, semarie, sophie.meynieux
Version: 3.6   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Patch for bug 6893 (suggestions list)
PATCH for bug 6893 (suggestions list)
Patch for bug 6893 (suggestions list)
Follow-up patch (removed ORDERED status filtering)
[SIGNED-OFF] Bug 6893 : Updates suggestions list when adding orders

Description Katrin Fischer 2011-09-21 06:13:07 UTC
AcqCreateItem set to 'in cataloguing'

To reproduce:

1) create new suggestion
2) accept suggestion
3) create new basket
4) create order line from suggestion
5) create another order line from suggestions

Problem: In 5) you will still see the already ordered suggestion in the list of suggestions.
Comment 1 Katrin Fischer 2011-10-18 06:12:41 UTC
Updating severity, as this bug breaks an important step in the suggestion management process. There is no way to tell which suggestions have already been ordered and the list will grow longer and longer!
Comment 2 Paul Poulain 2011-10-26 15:17:38 UTC
 Bug applicable in 3.6.0
Does not depend on AcqCreateItem syspref = i've tested with "ordering" and it's also here
Comment 3 Katrin Fischer 2011-10-26 15:47:40 UTC
Works nicely in current mater/3.6.
Comment 4 Katrin Fischer 2011-10-26 15:48:06 UTC
Wrong tab... this bug still exists.
Comment 5 Sophie MEYNIEUX 2011-10-27 08:45:25 UTC
Adrien will work on it
Comment 6 Adrien SAURAT 2011-11-04 13:36:45 UTC Comment hidden (obsolete)
Comment 7 Adrien SAURAT 2011-11-04 13:43:44 UTC Comment hidden (obsolete)
Comment 8 Adrien SAURAT 2011-11-04 13:44:35 UTC Comment hidden (obsolete)
Comment 9 Adrien SAURAT 2011-11-04 14:03:42 UTC
Two changes :
1) acqui/addorder.pl
line 215, if we have to change status we call ModSuggestion.
But the field name was called in lower-case, and expected later as upper-case.
Thus the 'ORDERED' state, for suggestions being ordered, was never attained.

This was the main problem. The suggestions remained in the ACCEPTED state, thus being still displayed in the suggestions list, and probably causing problems later with the e-mail sending (didn't test this part but this should rely on the ORDERED state).


2) C4/Suggestions.pm
found there a SQL clause "WHERE STATUS NOT IN ('CLAIMED')"
but except here, CLAIMED was nowhere to be found in the code.
Seems logical to think that it was supposed to be NOT IN ('ORDERED').

===

For this bug, the 1st modification was enough to repair, as the suggestions list called in newordersuggestion.pl already asks for the 'ACCEPTED' status (so it's not necessary to exclude the ORDERED).
But the 2nd modification could (should) help in other parts. Were the suggestions in OPAC correctly displayed ? Are the ordered suggestions supposed to appear there ?
Comment 10 Sophie MEYNIEUX 2011-11-04 16:13:45 UTC
I have tested this patch as described by Katrin and ordered books are well removed from suggestions list.

But in OPAC, suggestions that are ordered disappear from patron suggestions list. Shouldn't they be displaid with status Ordered ? 
I don't know if I should sign-off then.
Comment 11 Katrin Fischer 2011-11-04 16:19:35 UTC
Hi Sophie, thx for testing! Do they show up before the patch? I have not checked the OPAC display yet. If they don't, we might put that on a separate bug.
Comment 12 Francois Charbonnier 2011-11-06 11:34:14 UTC
I just tested this patch and i noticed a couple of things that not working well. These different elements seem to related to each other since a part of it works with the master branch but not with the patch once applied...

1/
If you are in the "pending" tab and click on "Submit" without checking any pending suggestion, the suggestion disappears from the table until you click on the "manage suggestion" link again on the acquisition home page.

2/ 
once you accepted the suggestion, you can order from this suggestion and then, it's no longer available from the newordersuggestion.pl page. 

But then, I noticed that the "ordered" tab doesn't appear and, at the opac level, the suggestion stays at the "accepted status". 

When you click, once again, on the "manage suggestion" link, on the acquisition home page, the ordered tab shows up but it's empty. And, if there is an "ordered" tab in the staff interface, the information at the opac for this suggestion should be "ordered" as well.

(I just noticed that, if I check out the master branch, the ordered tab is ok. There is one line inside corresponding to the order i did during my tests...) 

3/ Now, that I've tested all that, I did a new suggestion at the opac that I can't accept on the staff interface... It stays at the pending status no matter what i'm doing... :^(
Comment 13 Adrien SAURAT 2011-11-07 09:11:22 UTC
Ok, I expect the OPAC side to be affected, but thought it could be in a good way as this useless CLAIMED status could have broken something.

In fact, that's my 2nd modification (replacing CLAIMED with ORDERED) which broke something, and I see what's happening so most of these problems will disappear : I'll soon make another version of the patch with the OPAC ordered suggestions appearing (with status "Ordered by the library"), and Admin ordered suggestions appearing in the Ordered tab.

If we are sure that the CLAIMED status for a suggestion does no exist (I didn't find any other occurence in the whole code), then I will just suppress it to make the Suggestions.pm file clearer.



I didn't encounter any of the 1st and 3rd bugs mentionned by François, they probably appear in some perticuliar cases, and this is probably something different requiring another BZ bug (they don't look closely related to the ORDERED status change).
Comment 14 Adrien SAURAT 2011-11-07 10:21:00 UTC Comment hidden (obsolete)
Comment 15 Katrin Fischer 2011-12-01 07:36:52 UTC
Created attachment 6483 [details] [review]
[SIGNED-OFF] Bug 6893 : Updates suggestions list when adding orders

A suggestion added through addorder.pl now changes
to the ORDERED status as initially intended (but so
far it stayed as ACCEPTED).

This fixes the list of potential suggestions to order,
the ORDERED ones don't appear anymore.

Bug 6893 : Follow-up: Disabled WHERE clause excluding ORDERED status

In SearchSuggestions, a WHERE clause was excluding the ORDERED
suggestions from the results. Now replaced with 1=1.

(before 6893 correction, the WHERE clause was excluding the
STATUS='CLAIMED' suggestions, but this status cannot be found anywhere
else in the code... so I changed to STATUS='ORDERED' which
seemed more accurate, but it had impacts on OPAC and some Admin tabs)

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Both patches applied cleanly, sqashed them together after testing.

Tested:
1) Creating new suggestions
- from OPAC, shows up in patron account as 'requested', pending in staff
- from staff, shows up in list of pending suggestions
Status in database is ASKED

Note: suggestions made from staff for the same patron have suggestedby = 0
in the database, so are not showing up in the patron account. Is this by
intention? (if not, I think this is a separate bug)

2) Accepting suggestions
- from list
- from edit page
Status in database is ACCEPTED
Patron account shows 'Accepted by the library (Bestseller)'

Note: after changing the status from the list, bug 4068 kicks in and
some tabs and even suggestions are hidden. Going back to the acq start
page and going back to the suggestions page from there helps.

3) Rejecting suggestions
- from list
- from edit page
Status in database is REJECTED
Patron account in OPAC shows 'Suggestion declined'

Note: bug 4068 again.

4) Ordering from suggestions
- new basket
- ordered 2 accepted suggestions
SUCCESS! Great improvement:
- Status in database is now ORDERED
- Already ordered suggestions no longer show up on list of
  suggestions to order from.
- Patron account shows 'Ordered by the library (Bestseller)'
- Suggestion management page shows a new tab 'Ordered'
- For the suggestion with a valid 'suggestedby' borrowernumber,
  a mail was generated. (bug 7094)

5) Receive ordered suggestions
- received my ordered suggestions
This is is not in the scope of this bug, but for documentation:
- No message was generated to inform the patron
- Status stays ORDERED
- Patron account still shows item as ordered, no change to 4)

Looking at the suggestion.tt I think there should be another status
AVAILABLE.
216     [% ELSIF (suggestion.suggestiontypelabel == "Available") %]Available

Signing off, fixes bug reported. Danke!
Comment 16 Paul Poulain 2011-12-06 19:33:22 UTC
QA comment: Large patch, but a lot of description, only a few lines !

Patch pushed, please test
Comment 17 Sébastien Marie 2011-12-07 05:38:23 UTC
Comment on attachment 6483 [details] [review]
[SIGNED-OFF] Bug 6893 : Updates suggestions list when adding orders

>-    WHERE STATUS NOT IN ('CLAIMED')
>+    WHERE 1=1

I don't understand the need of this "WHERE" clause ("WHERE 1=1").
If the previous was wrong and not need, should it be removed (and not only replaced by a "always match"-clause) ?

But perhaps I miss something... please explain.
Comment 18 Sébastien Marie 2011-12-07 07:52:25 UTC
(In reply to comment #17)
> Comment on attachment 6483 [details] [review]
> [SIGNED-OFF] Bug 6893 : Updates suggestions list when adding orders
> 
> >-    WHERE STATUS NOT IN ('CLAIMED')
> >+    WHERE 1=1
> 
> I don't understand the need of this "WHERE" clause ("WHERE 1=1").
> If the previous was wrong and not need, should it be removed (and not only
> replaced by a "always match"-clause) ?
> 
> But perhaps I miss something... please explain.

ok, seen why... there is add of "and xxx" after.
sorry for noise.
Comment 19 Paul Poulain 2011-12-07 08:39:20 UTC
(In reply to comment #18)
> > But perhaps I miss something... please explain.
> 
> ok, seen why... there is add of "and xxx" after.
> sorry for noise.

Very common trick to dynamically build a query. And you're right to ask, don't worry about that !