Bug 24830 - dbic_merge_prefetch is not handling recursive cases correctly
Summary: dbic_merge_prefetch is not handling recursive cases correctly
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Agustín Moyano
QA Contact: Testopia
URL:
Keywords:
Depends on: 24356
Blocks: 20212
  Show dependency treegraph
 
Reported: 2020-03-09 13:49 UTC by Tomás Cohen Arazi (tcohen)
Modified: 2020-11-30 21:48 UTC (History)
3 users (show)

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


Attachments
Bug 24830: Fix parse_prefetch recursion and +count cases (2.53 KB, patch)
2020-03-09 16:10 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 24830: Add tests (2.76 KB, patch)
2020-03-12 21:49 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 24830: Fix parse_prefetch recursion and +count cases (2.53 KB, patch)
2020-03-12 21:49 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 24830: Fix parse_prefetch recursion and +count cases (2.73 KB, patch)
2020-03-12 21:55 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 24830: Add tests (2.82 KB, patch)
2020-03-13 18:05 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 24830: Fix parse_prefetch recursion and +count cases (2.78 KB, patch)
2020-03-13 18:05 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 24830: Add tests (2.89 KB, patch)
2020-03-20 12:18 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 24830: Fix parse_prefetch recursion and +count cases (2.85 KB, patch)
2020-03-20 12:18 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi (tcohen) 2020-03-09 13:49:28 UTC
In the context of bug 20212, I noticed that requesting to embed basket and basket.basket_group yields no basket is prefetch.
Comment 1 Tomás Cohen Arazi (tcohen) 2020-03-09 13:59:27 UTC
I also noticed the relation+count case is not being handled correctly (i.e. it won't be prefetch).
Comment 2 Agustín Moyano 2020-03-09 16:10:50 UTC
Created attachment 100393 [details] [review]
Bug 24830: Fix parse_prefetch recursion and +count cases

When a child of a prefetched element wasn't found on element's
prefetch_whitelist, no prefetch element was returned.. including element
it self. Now we return element's name if no child was found.

Also, embeded keys that ended with +count where taken literaly. Now we
search the correct key by stripping _count of embedded element if it was
declared as "is_count"
Comment 3 Martin Renvoize (ashimema) 2020-03-10 20:40:27 UTC
Should this be needs signoff?
Comment 4 Agustín Moyano 2020-03-10 21:09:48 UTC
(In reply to Martin Renvoize from comment #3)
> Should this be needs signoff?

Hi Martin, not yet.. I still have to design how to test.
Comment 5 Agustín Moyano 2020-03-12 21:49:08 UTC
Created attachment 100664 [details] [review]
Bug 24830: Add tests
Comment 6 Agustín Moyano 2020-03-12 21:49:11 UTC
Created attachment 100665 [details] [review]
Bug 24830: Fix parse_prefetch recursion and +count cases

When a child of a prefetched element wasn't found on element's
prefetch_whitelist, no prefetch element was returned.. including element
it self. Now we return element's name if no child was found.

Also, embeded keys that ended with +count where taken literaly. Now we
search the correct key by stripping _count of embedded element if it was
declared as "is_count"
Comment 7 Agustín Moyano 2020-03-12 21:55:21 UTC
Created attachment 100666 [details] [review]
Bug 24830: Fix parse_prefetch recursion and +count cases

When a child of a prefetched element wasn't found on element's
prefetch_whitelist, no prefetch element was returned.. including element
it self. Now we return element's name if no child was found.

Also, embeded keys that ended with +count where taken literaly. Now we
search the correct key by stripping _count of embedded element if it was
declared as "is_count"

To test:
1. Apply test patch but not this one
2. prove t/Koha/REST/Plugin/Query.t
CHECK => tests shoul fail
3. Apply this patch
4. prove t/Koha/REST/Plugin/Query.t
SUCCESS => tests pass
5. Sign off
Comment 8 Tomás Cohen Arazi (tcohen) 2020-03-13 18:05:44 UTC
Created attachment 100689 [details] [review]
Bug 24830: Add tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 9 Tomás Cohen Arazi (tcohen) 2020-03-13 18:05:52 UTC
Created attachment 100690 [details] [review]
Bug 24830: Fix parse_prefetch recursion and +count cases

When a child of a prefetched element wasn't found on element's
prefetch_whitelist, no prefetch element was returned.. including element
it self. Now we return element's name if no child was found.

Also, embeded keys that ended with +count where taken literaly. Now we
search the correct key by stripping _count of embedded element if it was
declared as "is_count"

To test:
1. Apply test patch but not this one
2. prove t/Koha/REST/Plugin/Query.t
CHECK => tests shoul fail
3. Apply this patch
4. prove t/Koha/REST/Plugin/Query.t
SUCCESS => tests pass
5. Sign off

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 10 Kyle M Hall (khall) 2020-03-20 12:18:26 UTC
Created attachment 101089 [details] [review]
Bug 24830: Add tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 11 Kyle M Hall (khall) 2020-03-20 12:18:40 UTC
Created attachment 101090 [details] [review]
Bug 24830: Fix parse_prefetch recursion and +count cases

When a child of a prefetched element wasn't found on element's
prefetch_whitelist, no prefetch element was returned.. including element
it self. Now we return element's name if no child was found.

Also, embeded keys that ended with +count where taken literaly. Now we
search the correct key by stripping _count of embedded element if it was
declared as "is_count"

To test:
1. Apply test patch but not this one
2. prove t/Koha/REST/Plugin/Query.t
CHECK => tests shoul fail
3. Apply this patch
4. prove t/Koha/REST/Plugin/Query.t
SUCCESS => tests pass
5. Sign off

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 12 Martin Renvoize (ashimema) 2020-03-20 15:43:49 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 13 Joy Nelson 2020-04-03 20:58:00 UTC
not backported to 19.11.x due to dependencies