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.
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
Created attachment 6199 [details] [review] Patch for bug 6893 (suggestions list)
Created attachment 6200 [details] [review] PATCH for bug 6893 (suggestions list) Sorry, forgot to check the "patch" box.
Created attachment 6201 [details] [review] Patch for bug 6893 (suggestions list) Well... I'm learning Bugzilla :P
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 6247 [details] [review] Follow-up patch (removed ORDERED status filtering)
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 !