Bug 28844 - Suggestion from existing title can alert patron in error
Summary: Suggestion from existing title can alert patron in error
Status: In Discussion
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: Main
Hardware: All All
: P5 - low major
Assignee: Bugs List
QA Contact: Testopia
URL:
Keywords: RM_priority
: 17463 (view as bug list)
Depends on: 14963
Blocks:
  Show dependency treegraph
 
Reported: 2021-08-11 12:58 UTC by Andrew Fuerste-Henry
Modified: 2024-09-11 14:26 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 28844: DB changes (3.46 KB, patch)
2023-05-10 13:25 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 28844: DBIC schema changes (3.16 KB, patch)
2023-05-10 13:25 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 28844: DBIC specific (894 bytes, patch)
2023-05-10 13:25 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 28844: Add link between suggestions and orders (17.88 KB, patch)
2023-05-10 13:25 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 28844: Remove GetSuggestionInfoFromBiblionumber (4.81 KB, patch)
2023-05-10 13:25 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 28844: Replace GetSuggestionFromBiblionumber (1.62 KB, patch)
2023-05-10 13:25 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 28844: Remove GetSuggestionFromBiblionumber (3.24 KB, patch)
2023-05-10 13:25 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 28844: DB changes (3.48 KB, patch)
2023-07-07 11:37 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 28844: DBIC schema changes (3.16 KB, patch)
2023-07-07 11:37 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 28844: DBIC specific (934 bytes, patch)
2023-07-07 11:37 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 28844: Add link between suggestions and orders (21.24 KB, patch)
2023-07-07 11:37 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 28844: Remove GetSuggestionInfoFromBiblionumber (4.78 KB, patch)
2023-07-07 11:37 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 28844: Replace GetSuggestionFromBiblionumber (1.62 KB, patch)
2023-07-07 11:37 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 28844: Remove GetSuggestionFromBiblionumber (3.21 KB, patch)
2023-07-07 11:37 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 28844: Remove GetSuggestionInfoFromBiblionumber (5.33 KB, patch)
2023-07-07 12:43 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 28844: Replace GetSuggestionFromBiblionumber (1.62 KB, patch)
2023-07-07 12:43 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 28844: Remove GetSuggestionFromBiblionumber (3.21 KB, patch)
2023-07-07 12:43 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 28844: DB changes (3.48 KB, patch)
2023-11-15 14:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 28844: DBIC schema changes (3.16 KB, patch)
2023-11-15 14:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 28844: DBIC specific (934 bytes, patch)
2023-11-15 14:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 28844: Add link between suggestions and orders (21.19 KB, patch)
2023-11-15 14:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 28844: Remove GetSuggestionInfoFromBiblionumber (5.33 KB, patch)
2023-11-15 14:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 28844: Replace GetSuggestionFromBiblionumber (1.62 KB, patch)
2023-11-15 14:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 28844: Remove GetSuggestionFromBiblionumber (3.21 KB, patch)
2023-11-15 14:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 28844: Adjust other calls (3.86 KB, patch)
2023-11-15 14:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 28844: Adjust tests (851 bytes, patch)
2023-11-15 14:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 28844: DB changes (3.53 KB, patch)
2023-12-12 18:27 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 28844: DBIC schema changes (3.22 KB, patch)
2023-12-12 18:27 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 28844: DBIC specific (990 bytes, patch)
2023-12-12 18:27 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 28844: Add link between suggestions and orders (21.25 KB, patch)
2023-12-12 18:27 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 28844: Remove GetSuggestionInfoFromBiblionumber (5.39 KB, patch)
2023-12-12 18:27 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 28844: Replace GetSuggestionFromBiblionumber (1.68 KB, patch)
2023-12-12 18:27 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 28844: Remove GetSuggestionFromBiblionumber (3.26 KB, patch)
2023-12-12 18:27 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 28844: Adjust other calls (3.92 KB, patch)
2023-12-12 18:27 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 28844: Adjust tests (907 bytes, patch)
2023-12-12 18:27 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 28844: (follow-up) Adjust suggestion id comparison (998 bytes, patch)
2023-12-12 18:28 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Fuerste-Henry 2021-08-11 12:58:40 UTC
To recreate:
- in acquisitions, create an order
- on the OPAC, find the bib relating to your order and put in a purchase suggestion tied to that bib
- back in acquisitions, receive your order without interacting with the purchase suggestion at all
- the patron tied to the suggestion has received a notice that their suggestion is available

While the suggestion and the order tie to the same bib, we cannot assume the suggestion and the order are linked directly. Especially in a multi-library system, there's no guarantee that the received copy of this title will go to the patron that requested it. 

We should only send suggestion notices to the patron for receipt of orders created from a purchase suggestion directly.
Comment 1 Jonathan Druart 2021-08-19 11:27:41 UTC
Looks like we need an additional link between the 2 tables aqorders and suggestions to achieve that.

That would make sense, especially when looking at such code:

         SELECT suggestions.*,
            U1.surname          AS surnamesuggestedby,
            U1.firstname        AS firstnamesuggestedby,
            U1.borrowernumber   AS borrnumsuggestedby
        FROM suggestions
            LEFT JOIN borrowers AS U1 ON suggestedby=U1.borrowernumber
        WHERE biblionumber=?
        LIMIT 1

in GetSuggestionInfoFromBiblionumber. It is wrong if there are several suggestions on the same bib.

Hard to know it won't break existing workflows however.
Comment 2 Katrin Fischer 2021-08-20 10:40:39 UTC
(In reply to Jonathan Druart from comment #1)
> Looks like we need an additional link between the 2 tables aqorders and
> suggestions to achieve that.
> 
> That would make sense, especially when looking at such code:
> 
>          SELECT suggestions.*,
>             U1.surname          AS surnamesuggestedby,
>             U1.firstname        AS firstnamesuggestedby,
>             U1.borrowernumber   AS borrnumsuggestedby
>         FROM suggestions
>             LEFT JOIN borrowers AS U1 ON suggestedby=U1.borrowernumber
>         WHERE biblionumber=?
>         LIMIT 1
> 
> in GetSuggestionInfoFromBiblionumber. It is wrong if there are several
> suggestions on the same bib.
> 
> Hard to know it won't break existing workflows however.

I agree - we should have a real link and not be guessing.
Comment 3 Jonathan Druart 2023-05-10 13:25:22 UTC
Created attachment 151001 [details] [review]
Bug 28844: DB changes
Comment 4 Jonathan Druart 2023-05-10 13:25:24 UTC
Created attachment 151002 [details] [review]
Bug 28844: DBIC schema changes
Comment 5 Jonathan Druart 2023-05-10 13:25:27 UTC
Created attachment 151003 [details] [review]
Bug 28844: DBIC specific
Comment 6 Jonathan Druart 2023-05-10 13:25:30 UTC
Created attachment 151004 [details] [review]
Bug 28844: Add link between suggestions and orders
Comment 7 Jonathan Druart 2023-05-10 13:25:32 UTC
Created attachment 151005 [details] [review]
Bug 28844: Remove GetSuggestionInfoFromBiblionumber
Comment 8 Jonathan Druart 2023-05-10 13:25:35 UTC
Created attachment 151006 [details] [review]
Bug 28844: Replace GetSuggestionFromBiblionumber
Comment 9 Jonathan Druart 2023-05-10 13:25:38 UTC
Created attachment 151007 [details] [review]
Bug 28844: Remove GetSuggestionFromBiblionumber
Comment 10 Jonathan Druart 2023-05-10 13:26:28 UTC
Well, no test plan here. You need to test the whole suggestion/acquisition workflow and search for bugs!
Link should be displayed correctly as before, and even better in cause of multiple suggestions on the same record.
Comment 11 Jonathan Druart 2023-05-10 13:27:42 UTC
Please do not backport this.
Comment 12 Jonathan Druart 2023-06-09 13:19:34 UTC
Andrew, Katrin, can I get your opinion on these patches please? :)
Comment 13 Katrin Fischer 2023-06-25 13:58:06 UTC
*** Bug 17463 has been marked as a duplicate of this bug. ***
Comment 14 Katrin Fischer 2023-07-07 10:12:02 UTC
Sorry, patch doesn't apply. Please reabase!
Comment 15 Jonathan Druart 2023-07-07 11:37:09 UTC
Created attachment 153157 [details] [review]
Bug 28844: DB changes
Comment 16 Jonathan Druart 2023-07-07 11:37:13 UTC
Created attachment 153158 [details] [review]
Bug 28844: DBIC schema changes
Comment 17 Jonathan Druart 2023-07-07 11:37:16 UTC
Created attachment 153159 [details] [review]
Bug 28844: DBIC specific
Comment 18 Jonathan Druart 2023-07-07 11:37:20 UTC
Created attachment 153160 [details] [review]
Bug 28844: Add link between suggestions and orders
Comment 19 Jonathan Druart 2023-07-07 11:37:23 UTC
Created attachment 153161 [details] [review]
Bug 28844: Remove GetSuggestionInfoFromBiblionumber
Comment 20 Jonathan Druart 2023-07-07 11:37:27 UTC
Created attachment 153162 [details] [review]
Bug 28844: Replace GetSuggestionFromBiblionumber
Comment 21 Jonathan Druart 2023-07-07 11:37:30 UTC
Created attachment 153163 [details] [review]
Bug 28844: Remove GetSuggestionFromBiblionumber
Comment 22 Jonathan Druart 2023-07-07 12:43:17 UTC
Created attachment 153174 [details] [review]
Bug 28844: Remove GetSuggestionInfoFromBiblionumber
Comment 23 Jonathan Druart 2023-07-07 12:43:20 UTC
Created attachment 153175 [details] [review]
Bug 28844: Replace GetSuggestionFromBiblionumber
Comment 24 Jonathan Druart 2023-07-07 12:43:23 UTC
Created attachment 153176 [details] [review]
Bug 28844: Remove GetSuggestionFromBiblionumber
Comment 25 Katrin Fischer 2023-07-07 14:37:41 UTC
I'd really love to see this long standing issue fixed :)


1) Migration of data

At the moment this change would break processing the existing suggestions. As the suggestion_id for the existing orders is empty, the information would not show up on the orders in acq, and I am also not sure that the patrons would be informed by email.

My suggestion: 

We only compare entries with the same biblionumber:

1 suggestion with status in (ORDERED, AVAILABLE) and 1 order => link them
1 suggestion with status (ORDERED) and 1 order with orderstatus in (ordered, partial) => link them
1 suggestion with status (AVAILABLE) and 1 order with orderstatus (complete) => link them

For the rest:
Mimick what we have done until now, use the first match (LIMIT 1, ->single):

For every order line that has no suggestion_id yet
that has a matching biblionumber in suggestions
link to the suggestion_id with the lowest/smallest suggestion_id where status in (ORDERED, AVAILABLE)

So it might not be correct, but I think it's what would be displayed on the order line now and where the email would be sent to, so that might be the best we can do?

2) ModSuggestion 

ModSuggestion is called to update the status of the suggestion and to send the email, but the calls haven't always been adjusted to look up the correct suggestion.

Orders.pm cancel()
my $suggestion = Koha::Suggestions->find({ biblionumber => $self->biblionumber, status => "ORDERED" });
if ( $suggestion and $suggestion->id ) {

Wouldn't this fail if there is more than one suggestion for biblionumber with status ORDERED?

addorder.pl = seems OK, because using the suggestion_id passed, but double checking would be good

EDI.pm
                    my $s = $schema->resultset('Suggestion')->search(
                        {
                            biblionumber => $order->biblionumber->biblionumber,
                        }
                    )->single;
                    if ($s) {
                        ModSuggestion(
                            {
                                suggestionid => $s->suggestionid,
                                STATUS       => 'AVAILABLE',
                            }
                        );
                    }

And there are more calls I haven't checked that need to be verified.
Comment 26 Jonathan Druart 2023-11-15 14:08:57 UTC
Created attachment 158982 [details] [review]
Bug 28844: DB changes
Comment 27 Jonathan Druart 2023-11-15 14:09:00 UTC
Created attachment 158983 [details] [review]
Bug 28844: DBIC schema changes
Comment 28 Jonathan Druart 2023-11-15 14:09:04 UTC
Created attachment 158984 [details] [review]
Bug 28844: DBIC specific
Comment 29 Jonathan Druart 2023-11-15 14:09:08 UTC
Created attachment 158985 [details] [review]
Bug 28844: Add link between suggestions and orders
Comment 30 Jonathan Druart 2023-11-15 14:09:11 UTC
Created attachment 158986 [details] [review]
Bug 28844: Remove GetSuggestionInfoFromBiblionumber
Comment 31 Jonathan Druart 2023-11-15 14:09:14 UTC
Created attachment 158987 [details] [review]
Bug 28844: Replace GetSuggestionFromBiblionumber
Comment 32 Jonathan Druart 2023-11-15 14:09:17 UTC
Created attachment 158988 [details] [review]
Bug 28844: Remove GetSuggestionFromBiblionumber
Comment 33 Jonathan Druart 2023-11-15 14:09:21 UTC
Created attachment 158989 [details] [review]
Bug 28844: Adjust other calls
Comment 34 Jonathan Druart 2023-11-15 14:09:24 UTC
Created attachment 158990 [details] [review]
Bug 28844: Adjust tests
Comment 35 Jonathan Druart 2023-11-15 14:09:33 UTC
(In reply to Katrin Fischer from comment #25)
> I'd really love to see this long standing issue fixed :)
> 
> 
> 1) Migration of data
> So it might not be correct, but I think it's what would be displayed on the
> order line now and where the email would be sent to, so that might be the
> best we can do?

It was left on purpose. I don't think we should copy an information if we are not sure it is correct.
I am not confident in migrating something here.

> 2) ModSuggestion 
> 
> ModSuggestion is called to update the status of the suggestion and to send
> the email, but the calls haven't always been adjusted to look up the correct
> suggestion.

Correct, good catch. Should be fixed.

> And there are more calls I haven't checked that need to be verified.

Other calls look good.

I let you decide what the status is.
Comment 36 Nick Clemens (kidclamp) 2023-12-12 18:27:38 UTC
Created attachment 159759 [details] [review]
Bug 28844: DB changes

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 37 Nick Clemens (kidclamp) 2023-12-12 18:27:41 UTC
Created attachment 159760 [details] [review]
Bug 28844: DBIC schema changes

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 38 Nick Clemens (kidclamp) 2023-12-12 18:27:43 UTC
Created attachment 159761 [details] [review]
Bug 28844: DBIC specific

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 39 Nick Clemens (kidclamp) 2023-12-12 18:27:46 UTC
Created attachment 159762 [details] [review]
Bug 28844: Add link between suggestions and orders

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 40 Nick Clemens (kidclamp) 2023-12-12 18:27:49 UTC
Created attachment 159763 [details] [review]
Bug 28844: Remove GetSuggestionInfoFromBiblionumber

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 41 Nick Clemens (kidclamp) 2023-12-12 18:27:51 UTC
Created attachment 159764 [details] [review]
Bug 28844: Replace GetSuggestionFromBiblionumber

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 42 Nick Clemens (kidclamp) 2023-12-12 18:27:54 UTC
Created attachment 159765 [details] [review]
Bug 28844: Remove GetSuggestionFromBiblionumber

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 43 Nick Clemens (kidclamp) 2023-12-12 18:27:56 UTC
Created attachment 159766 [details] [review]
Bug 28844: Adjust other calls

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 44 Nick Clemens (kidclamp) 2023-12-12 18:27:59 UTC
Created attachment 159767 [details] [review]
Bug 28844: Adjust tests

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 45 Nick Clemens (kidclamp) 2023-12-12 18:28:01 UTC
Created attachment 159768 [details] [review]
Bug 28844: (follow-up) Adjust suggestion id comparison

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 46 Nick Clemens (kidclamp) 2023-12-12 18:30:59 UTC
Added a minor follow-up, otherwise it seemed to work as expected, QA tool does note some untidiness

I think  the link here is wrong though. I believe the suggestions need to store the ordernumber - this allows for multiple suggestions on a single order. The current patches limit an order being tied to a single suggestion.

If a library has two patrons suggest the same title, they may only want to order a single copy, or place a single order for multiple copies. Could it be done with the order number stored in suggestions?
Comment 47 Jonathan Druart 2023-12-13 09:07:08 UTC
I think I understand what you mean, Nick, but the UI is not allowing it for now (to have several suggestions linked to an order).

Should we really add complexity (at least rewrite everything here) in case maybe one day we will implement it?

It will still be possible later to move the data.
Comment 48 Katrin Fischer 2023-12-23 12:40:58 UTC
Maybe we could extend the relationship later. At the moment we create an order from a suggestion, so a 1:1 makes sense.

When I tested this my main concern was to ensure that the suggestions existing previously to this fix would still work, patrons receiving emails etc. Did you take a look at that aspect?
Comment 49 Jonathan Druart 2024-01-03 14:51:20 UTC
(In reply to Katrin Fischer from comment #48)
> Maybe we could extend the relationship later. At the moment we create an
> order from a suggestion, so a 1:1 makes sense.
> 
> When I tested this my main concern was to ensure that the suggestions
> existing previously to this fix would still work, patrons receiving emails
> etc. Did you take a look at that aspect?

Yes, but didn't fix it. Seen comment 35?
Comment 50 Katrin Fischer 2024-01-03 22:40:49 UTC
(In reply to Jonathan Druart from comment #49)
> (In reply to Katrin Fischer from comment #48)
> > Maybe we could extend the relationship later. At the moment we create an
> > order from a suggestion, so a 1:1 makes sense.
> > 
> > When I tested this my main concern was to ensure that the suggestions
> > existing previously to this fix would still work, patrons receiving emails
> > etc. Did you take a look at that aspect?
> 
> Yes, but didn't fix it. Seen comment 35?

Yes, but ... 

I think we need to fix this, but I also think we can't break existing suggestions especially since it involves a service the library provides to their patrons and they can't really fix it in the GUI either.

Too late today, but we should give this some more thought.
Comment 51 Jonathan Druart 2024-01-04 11:08:03 UTC
I am not willing to write the DB rev to adjust the existing entries. I would like help here please.
Comment 52 Katrin Fischer 2024-01-04 12:53:02 UTC
OK, I am not sure I could write it either, but the first step is to make a plan.
Comment 53 Nick Clemens (kidclamp) 2024-01-05 19:00:00 UTC
I reworked this with the relationship built the other way on bug 35717

I can attempt the update there, however, I think a better path is to allow a GUI way for the suggestions to be correctly linked, or to attempt to update and provide a way to correct if needed.


I will be working on 19358 and 31632 this cycle which will be part of the GUI.

Please let me know if we can agree to continue this work on 35717