Like Bug 20702 defined GetHostItemsInfo does nothing if EasyAnalyticalRecords pref is disabled, there are other places where code must be dependant on this pref.
Created attachment 84060 [details] [review] Bug 22140: More use of EasyAnalyticalRecords pref Like Bug 20702 defined GetHostItemsInfo does nothing if EasyAnalyticalRecords pref is disabled, there are other places where code must be dependant on this pref. Test plan : 1) Build an analitical record with 773$0 and $9 2) Enable EasyAnalyticalRecords 3) Don't apply patch 4) Go to OPAC 5) Perform a search that displays the record, check there is the linked item 6) Open record detail page, check there is the linked item 7) Apply patch and redo 5) and 6) 8) Disable EasyAnalyticalRecords 9) redo 5) and 6), you should not see the linked item
We restarted memcached and plack and the link still did not disappear.
(In reply to helene from comment #2) > We restarted memcached and plack and the link still did not disappear. Sorry could you describe more please ? Where does the test plan fail ?
If memory serves there's some fun to be had regarding the EasyAnalytics and the UseControlNumber preferences around this code.. you may (or may not) want to investiage that preference a little further too before making this change. I do however like the general idea here.. tidying up a little.
Also.. looking at the original diff (https://github.com/Koha-Community/Koha/commit/f09e2ca27ee89860dbdd0ca512735f37fbfaf8a6) of the EasyAnalytics submission, it looks like this could go futher.
I raise to major importance because its impact on display is very strange and difficult to debug. @Martin you mean your OK with the patch ?
Created attachment 84820 [details] [review] Bug 22140: Use of EasyAnalyticalRecords pref in search Like Bug 20702 defined GetHostItemsInfo does nothing if EasyAnalyticalRecords pref is disabled, there are other places where code must be dependant on this pref. Test plan : 1) Build an analitical record with 773$0 and $9 2) Enable EasyAnalyticalRecords 3) Don't apply patch 4) Go to OPAC 5) Perform a search that displays the record, check there is the linked item 6) Open record detail page, check there is the linked item 7) Apply patch and redo 5) and 6) 8) Disable EasyAnalyticalRecords 9) redo 5) and 6), you should not see the linked item
Created attachment 84822 [details] [review] Bug 22140: Use of EasyAnalyticalRecords pref is get_hostitemnumbers_of Like Bug 20702 defined GetHostItemsInfo does nothing if EasyAnalyticalRecords pref is disabled, get_hostitemnumbers_of must be dependant on this pref too. Test plan : 1) Build an analytical record with 773$0 and $9 2) Enable EasyAnalyticalRecords 4) Place an hold on the record 5) => You see the linked item 6) Disable EasyAnalyticalRecords 7) Place an hold on the record 8) => You dont see the linked item
Found a new method missing the syspref check. I try to create a UT on get_hostitemnumbers_of but its hard since it uses db biblio and items.
Created attachment 84828 [details] [review] Bug 22140: Larger unit test for get_hostitemnumbers_of Add syspref EasyAnalyticalRecords impact in unit test for get_hostitemnumbers_of Test plan : Run t/db_dependent/Items.t
Piouf I did it in the end in Items.t in addition to existing test.
Created attachment 84958 [details] [review] Bug 22140: Use of EasyAnalyticalRecords pref in search Like Bug 20702 defined GetHostItemsInfo does nothing if EasyAnalyticalRecords pref is disabled, there are other places where code must be dependant on this pref. Test plan : 1) Build an analitical record with 773$0 and $9 2) Enable EasyAnalyticalRecords 3) Don't apply patch 4) Go to OPAC 5) Perform a search that displays the record, check there is the linked item 6) Open record detail page, check there is the linked item 7) Apply patch and redo 5) and 6) 8) Disable EasyAnalyticalRecords 9) redo 5) and 6), you should not see the linked item Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 84959 [details] [review] Bug 22140: Use of EasyAnalyticalRecords pref is get_hostitemnumbers_of Like Bug 20702 defined GetHostItemsInfo does nothing if EasyAnalyticalRecords pref is disabled, get_hostitemnumbers_of must be dependant on this pref too. Test plan : 1) Build an analytical record with 773$0 and $9 2) Enable EasyAnalyticalRecords 4) Place an hold on the record 5) => You see the linked item 6) Disable EasyAnalyticalRecords 7) Place an hold on the record 8) => You dont see the linked item Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 84960 [details] [review] Bug 22140: Larger unit test for get_hostitemnumbers_of Add syspref EasyAnalyticalRecords impact in unit test for get_hostitemnumbers_of Test plan : Run t/db_dependent/Items.t Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Sorry it took me a while to test this after you mentioned it, I wanted to give it my due attention. Looks good and works as expected for me with no regressions that I can see. Signing Off.
Created attachment 85143 [details] [review] Bug 22140: Use of EasyAnalyticalRecords pref in search Like Bug 20702 defined GetHostItemsInfo does nothing if EasyAnalyticalRecords pref is disabled, there are other places where code must be dependant on this pref. Test plan : 1) Build an analitical record with 773$0 and $9 2) Enable EasyAnalyticalRecords 3) Don't apply patch 4) Go to OPAC 5) Perform a search that displays the record, check there is the linked item 6) Open record detail page, check there is the linked item 7) Apply patch and redo 5) and 6) 8) Disable EasyAnalyticalRecords 9) redo 5) and 6), you should not see the linked item Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 85144 [details] [review] Bug 22140: Use of EasyAnalyticalRecords pref is get_hostitemnumbers_of Like Bug 20702 defined GetHostItemsInfo does nothing if EasyAnalyticalRecords pref is disabled, get_hostitemnumbers_of must be dependant on this pref too. Test plan : 1) Build an analytical record with 773$0 and $9 2) Enable EasyAnalyticalRecords 4) Place an hold on the record 5) => You see the linked item 6) Disable EasyAnalyticalRecords 7) Place an hold on the record 8) => You dont see the linked item Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 85145 [details] [review] Bug 22140: Larger unit test for get_hostitemnumbers_of Add syspref EasyAnalyticalRecords impact in unit test for get_hostitemnumbers_of Test plan : Run t/db_dependent/Items.t Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Awesome work all! Pushed to master for 19.05
Pushed to 18.11.x for 18.11.04
This was a "fun" backport.. had to backport the build_sample_* tree for t/lib/TestBuilder too... shout if you need a had with that subsequent rmaints.. hopefully I've taken the edge off it if you do decide to backport.
cant cleanly apply this to 18.05.x. If needed please rebase.