Bug 24830

Summary: dbic_merge_prefetch is not handling recursive cases correctly
Product: Koha Reporter: Tomás Cohen Arazi <tomascohen>
Component: Architecture, internals, and plumbingAssignee: Agustín Moyano <agustinmoyano>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: 1joynelson, martin.renvoize, tomascohen
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
20.05.00
Bug Depends on: 24356    
Bug Blocks: 20212    
Attachments: Bug 24830: Fix parse_prefetch recursion and +count cases
Bug 24830: Add tests
Bug 24830: Fix parse_prefetch recursion and +count cases
Bug 24830: Fix parse_prefetch recursion and +count cases
Bug 24830: Add tests
Bug 24830: Fix parse_prefetch recursion and +count cases
Bug 24830: Add tests
Bug 24830: Fix parse_prefetch recursion and +count cases

Description Tomás Cohen Arazi 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 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 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 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 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 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 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 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