|
Description
Nick Clemens (kidclamp)
2024-01-05 18:54:28 UTC
Created attachment 160576 [details] [review] Bug 35717: DB changes Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 160577 [details] [review] Bug 35717: DBIC schema changes Created attachment 160578 [details] [review] Bug 35717: Add link between suggestions and orders Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 160579 [details] [review] Bug 35717: Remove GetSuggestionInfoFromBiblionumber Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 160580 [details] [review] Bug 35717: Replace GetSuggestionFromBiblionumber Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 160581 [details] [review] Bug 35717: Remove GetSuggestionFromBiblionumber Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 160582 [details] [review] Bug 35717: Adjust other calls Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 160583 [details] [review] Bug 35717: Tidy Test plan: 1 - apply patch, reset_all 2 - create purchase suggestion 3 - accept purchase suggestion 4 - create and receive order from suggestion 5 - confirm that notices were generated as suggestion is accepted, ordered, received 6 - confirm that order number is present in suggestions data Anything else I need to check here? Test plan: 1 - apply patch, reset_all 2 - create purchase suggestion 3 - accept purchase suggestion 4 - create and receive order from suggestion 5 - confirm that notices were generated as suggestion is accepted, ordered, received 6 - confirm that order number is present in suggestions data 7 - submit a second suggestion, accept it, create order from it 8 - cancel order from suggestion, confirm suggestion status reverts from "Ordered" to "Accepted" and order number is removed from suggestion Hi these patches applied, I'm getting an error when attempting to receive an order tied to a suggestion. It looks like maybe an undefined basket number? (In reply to Andrew Fuerste-Henry from comment #11) > Hi these patches applied, I'm getting an error when attempting to receive an > order tied to a suggestion. It looks like maybe an undefined basket number? I don't recreate this. Did you updatedatabase after applying the patch? (In reply to Lucas Gass from comment #12) > (In reply to Andrew Fuerste-Henry from comment #11) > > Hi these patches applied, I'm getting an error when attempting to receive an > > order tied to a suggestion. It looks like maybe an undefined basket number? > > I don't recreate this. Did you updatedatabase after applying the patch? I'm getting this error on a sandbox that was provisioned with the patch applied from the start. My understanding is that inherently includes an updatedatabase, and I can see via SQL that the suggestions table *does* include the ordernumber field. (In reply to Andrew Fuerste-Henry from comment #11) > Hi these patches applied, I'm getting an error when attempting to receive an > order tied to a suggestion. It looks like maybe an undefined basket number? Was the suggestion created before or after applying the patch? (In reply to Katrin Fischer from comment #14) > (In reply to Andrew Fuerste-Henry from comment #11) > > Hi these patches applied, I'm getting an error when attempting to receive an > > order tied to a suggestion. It looks like maybe an undefined basket number? > > Was the suggestion created before or after applying the patch? After. I tested this on a sandbox with the patch applied from creation. I guess that was wishful thinking. I hope Nick will have time to have a look soon. Created attachment 164834 [details] [review] Bug 35717: DB changes Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 164835 [details] [review] Bug 35717: DBIC schema changes Created attachment 164836 [details] [review] Bug 35717: Add link between suggestions and orders Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 164837 [details] [review] Bug 35717: Remove GetSuggestionInfoFromBiblionumber Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 164838 [details] [review] Bug 35717: Replace GetSuggestionFromBiblionumber Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 164839 [details] [review] Bug 35717: Remove GetSuggestionFromBiblionumber Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 164840 [details] [review] Bug 35717: Adjust other calls Signed-off-by: Nick Clemens <nick@bywatersolutions.com> And bug 31632? Can we maybe all discuss and move toward a single direction at the same time?... I'm looking at this one. Had to recreate a patch and some patches are in the wrong order? Created attachment 172871 [details] [review] Bug 35717: DB changes Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 172872 [details] [review] Bug 35717: Add link between suggestions and orders Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 172873 [details] [review] Bug 35717: Remove GetSuggestionInfoFromBiblionumber Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 172874 [details] [review] Bug 35717: Replace GetSuggestionFromBiblionumber Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 172875 [details] [review] Bug 35717: Remove GetSuggestionFromBiblionumber Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 172876 [details] [review] Bug 35717: DBIC schema [DO NOT PUSH] Created attachment 172877 [details] [review] Bug 35717: Adjust other calls Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 172878 [details] [review] Bug 35717: (QA follow-up) ->suggestions() always returns a resultset This patch acknowledges the fact a one to many relationship always returns an iterator (even an empty one) so there's no point in having a codepath for it not returning a resultset. The tests are simplified for readability too. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> (In reply to Jonathan Druart from comment #24) > And bug 31632? Can we maybe all discuss and move toward a single direction > at the same time?... Aleisha and Nick are on the same page. I'll take a look at that one after this one. Created attachment 172899 [details] [review] Bug 35717: Fix typo in basket.tt This patch fixes a stray `%]` displaying in basket.pl when the order is linked to a suggestion. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 172900 [details] [review] Bug 35717: Fix rebase issue after bug 34478 Some code was introduced and the block setting the `ordernumber` for the suggestion when creating an order from a suggestion got located before the actual order is created, and as such the suggestion got is ordernumber set as NULL. Created attachment 172901 [details] [review] Bug 35717: JS fixes Bug 37018 added validation to queries and `{"in":...}` is no longer valid. `{"-in":...}` is so this patch tweaks that. The previous patch fixes actual linking orders to suggestions. But not having orders linked highlighted the code explode because an empty array evaluates to `true` in Javascript. This patch fixes that too. The links generated by this patchset also get their components correctly escaped. This was already signed. I have just started to review the code here. 1) Migration I notice that there is no migration script for existing suggestions. You remove GetSuggestionFromBiblionumber and replace it with a lookup by ordernumber, but this will be empty for all existing suggestions after update. I hope I didn't miss it, but there seems to be no fallback. For the users this means they won't receive an AVAILABLE notice once the suggestion is received. For the people working in ACQ they will no longer be able to tell which old orders have been ordered from suggestions. How do we deal with that? Joubu and I had discussed that at length on bug 28844. The patches don't include any test plans and Andrew's test plan is great, but missing something:
-1 - Create at least 2 suggestions
0 - Create order from one of the suggestions
> 1 - apply patch, reset_all
> 2 - create purchase suggestion
> 3 - accept purchase suggestion
> 4 - create and receive order from suggestion
> 5 - confirm that notices were generated as suggestion is accepted, ordered,
> received
> 6 - confirm that order number is present in suggestions data
7 - Confirm the old order from suggestion
* displays suggestor still and switches to suggestion status to AVAILABLE when received
|