Summary: | Tag cloud feature broken | ||
---|---|---|---|
Product: | Koha | Reporter: | Tomás Cohen Arazi (tcohen) <tomascohen> |
Component: | OPAC | Assignee: | Tomás Cohen Arazi (tcohen) <tomascohen> |
Status: | CLOSED FIXED | QA Contact: | Katrin Fischer <katrin.fischer> |
Severity: | major | ||
Priority: | P5 - low | CC: | costalc, dcook, jonathan.druart, josef.moravec, lucas, martin.renvoize, nick, tomascohen |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
See Also: |
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21846 https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22766 https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22985 |
||
Change sponsored?: | --- | Patch complexity: | Small patch |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: |
19.05.00, 18.11.06
|
|
Circulation function: | |||
Bug Depends on: | 14385 | ||
Bug Blocks: | |||
Attachments: |
Bug 22420: Remove duplicated tags
[ALTERNATIVE] Bug 22420: Remove wrong calaculation on approved tags Bug 22420: Remove wrong calculation on approved tags |
Description
Tomás Cohen Arazi (tcohen)
2019-02-26 19:27:16 UTC
I'm sure it was caused by bug 14385 because it overwrites $results with something they calculate, and commmenting out the overwriting line fixes the problem. 324 } 325 #$results = \@filtered_results; 326 stratify_tags(10, $results); # work out the differents sizes for things Created attachment 85748 [details] [review] Bug 22420: Remove duplicated tags Caused by commit e1b5fa657de843a177fb4bf57947a1376152d021 Bug 14385: Squash of a lot of patches rebased Test plan: - Have the OPAC tags feature enabled - Add a couple tags to different records, make sure some of them are used more than once - Go to 'Tag cloud' in the OPAC => the repeated tags must not be repeated many times. [- Enjoy the lack of tests.] (In reply to Jonathan Druart from comment #2) > Created attachment 85748 [details] [review] [review] > Bug 22420: Remove duplicated tags > > Caused by > commit e1b5fa657de843a177fb4bf57947a1376152d021 > Bug 14385: Squash of a lot of patches rebased > > Test plan: > - Have the OPAC tags feature enabled > - Add a couple tags to different records, make sure some of them are > used more than once > - Go to 'Tag cloud' in the OPAC > => the repeated tags must not be repeated many times. > > [- Enjoy the lack of tests.] This doesn't solve the problem. Created attachment 85755 [details] [review] [ALTERNATIVE] Bug 22420: Remove wrong calaculation on approved tags Before the patch from bug 14385, the passed structure (that is returned by get_approval_rows() looks like: \ [ [0] { approved 1, approved_by 51, approved_by_name undef, date_approved "2019-02-26 15:36:42", term "another", weight_total 3 }, [1] { ... After the introduced 'filtering', we loose the information about the term weight!: \ [ [0] { approved 1, author "Heylin, Clinton.", biblionumber 1, borrowernumber 51, date_created "2019-02-26 15:36:37", language undef, subtitle [], tag_id 1, term "word", time_created_display "15:36:37", title "E Street shuffle :", visible 1, XSLTBloc " <<<BUNCH OF GENERATED HTML HERE>>> ... The code even calls GetMarcBiblio. This needs to be reverted as it is plain wrong. If it was worth removing *some* biblios from the weight_total value, then this calculation should be done somewhere else, without fetching the MARC data again. What you are suggesting is temporary and must be fixed, right? (In reply to Jonathan Druart from comment #5) > What you are suggesting is temporary and must be fixed, right? After looking at this and having feedback on koha-devel I can say the 'feature' of hiding tags for hidden bibliographic records (due to OpacHiddenItems modulo OpacHiddenExceptions) is broken altogether. i.e. The calculation is wrong anyway and the loop doesn't generate a suitable data structure either. I prefer my approach to solve the immediate issue (no tags are hidden anyway, just wrongly displayed), and file a separate bug for the leakage bug. Sending to NSO with your patch then ;) *** Bug 22766 has been marked as a duplicate of this bug. *** Upping severity. Moving this to signed off as per this comment: bug 22766 comment 5 Created attachment 89163 [details] [review] Bug 22420: Remove wrong calculation on approved tags Before the patch from bug 14385, the passed structure (that is returned by get_approval_rows() looks like: \ [ [0] { approved 1, approved_by 51, approved_by_name undef, date_approved "2019-02-26 15:36:42", term "another", weight_total 3 }, [1] { ... After the introduced 'filtering', we loose the information about the term weight!: \ [ [0] { approved 1, author "Heylin, Clinton.", biblionumber 1, borrowernumber 51, date_created "2019-02-26 15:36:37", language undef, subtitle [], tag_id 1, term "word", time_created_display "15:36:37", title "E Street shuffle :", visible 1, XSLTBloc " <<<BUNCH OF GENERATED HTML HERE>>> ... The code even calls GetMarcBiblio. This needs to be reverted as it is plain wrong. If it was worth removing *some* biblios from the weight_total value, then this calculation should be done somewhere else, without fetching the MARC data again. Signed-off-by: Claudio <costalc@gmail.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Fixes a major regression. I agree if filtering is needed, we should resolve that separately. Awesome work all! Pushed to master for 19.05 Pushed to 18.11.x for 18.11.06 missing dependencies, wont backport to 18.05.x |