Bug 6893

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!

 Bug applicable in 3.6.0
Does not depend on AcqCreateItem syspref = i've tested with "ordering" and it's also here

Works nicely in current mater/3.6.

Wrong tab... this bug still exists.

Adrien will work on it

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 ?

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.

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.

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... :^(

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).

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!

QA comment: Large patch, but a lot of description, only a few lines !

Patch pushed, please test

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.

(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.

(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 !