Bug 7642

Summary: The number that appears when hovering over a tag in the tag cloud isn't the number of items with that tag
Product: Koha Reporter: Robin Sheat <robin>
Component: OPACAssignee: stacey
Status: CLOSED FIXED QA Contact: Ian Walls <koha.sekjal>
Severity: minor    
Priority: P5 - low CC: chris, jonathan.druart, katrin.fischer, nengard, paul.poulain, veron
Version: 3.8   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Bug 7642 - OpacShowTagCloudWeight system preference
[SIGNED-OFF] Bug 7642 - OpacShowTagCloudWeight system preference
Bug 7642 - remove the cloud weight from the tags
signed off patch
Bug 7642 - fix the lost display of tag sizes
Bug 7642 - fix the lost display of tag sizes
Bug 7642 - fix the lost display of tag sizes
Bug 7642 - fix the lost display of tag sizes
[SIGNED-OFF] Bug 7642 - fix the lost display of tag sizes
Bug 7642 - fix the lost display of tag sizes
Bug 7642 - [3.8.x] fix the lost display of tag sizes

Description Robin Sheat 2012-03-05 06:49:36 UTC
When you hover over a tag, the number that appears is the weighting of that tag, not the actual number of items that are tagged with that. This is not really what one would expect.
Comment 1 Robin Sheat 2012-05-26 01:07:45 UTC
*** Bug 8157 has been marked as a duplicate of this bug. ***
Comment 2 Robin Sheat 2012-05-26 01:08:30 UTC
I plan to make a syspref that'll allow this to be turned off.
Comment 3 Robin Sheat 2012-05-26 01:44:15 UTC Comment hidden (obsolete)
Comment 4 Robin Sheat 2012-05-26 01:56:07 UTC
This will apply to the 3.6 (and I guess 3.8) branches happily enough, just with easy-to-fix conflicts due to changes in the prefs and database files.
Comment 5 Owen Leonard 2012-05-26 11:38:58 UTC
Adding a preference for this seems like overkill to me. Why not either make the number represent the actual number of items tagged or remove the title attribute altogether?
Comment 6 Katrin Fischer 2012-05-26 12:25:41 UTC
I agree with Owen here - it seems like it's confusing how it is now and I would really like a way to show the number of titles associated with a tag.
Comment 7 Robin Sheat 2012-05-26 12:29:40 UTC
Making it reflect the real number of items is hard, you have to do a whole nother set of queries to account for multiple people tagging the same thing the same.

Removing it totally is obviously easy, but I didn't want to remove that option in case there were people that liked it. Basically, I didn't want to remove an existing "feature."
Comment 8 Katrin Fischer 2012-06-09 14:09:10 UTC Comment hidden (obsolete)
Comment 9 Katrin Fischer 2012-06-09 14:09:32 UTC
Leaving decision to QA team and RM.
Comment 10 Paul Poulain 2012-06-10 10:54:58 UTC
(In reply to comment #9)
> Leaving decision to QA team and RM.

RM fully agreeing with owen & katrin & is happy to see he's not the only member of FAUSP ("Fighters Against Useless System Preferences")

let's go to disabling this option : if the number is confusing (and I agree it is), just remove it !
Comment 11 Robin Sheat 2012-06-10 11:30:05 UTC
Alrighty then, I'll modify it to do so.
Comment 12 Robin Sheat 2012-06-10 14:02:30 UTC Comment hidden (obsolete)
Comment 13 stacey 2012-06-10 14:56:48 UTC Comment hidden (obsolete)
Comment 14 Paul Poulain 2012-06-11 16:21:04 UTC
QA comment: tiniest patch ever maybe, but I've a doubt. In opac-tags.pl, there is still some references to cloudweight field that you remove from the template (and is no more used in the template), for example:
$_->{cloudweight} = $strata;

I'm not sure I'm right, so I let someone else investigating and confirming or infirming. Not changing QA status for now.
Comment 15 Ian Walls 2012-06-11 16:34:22 UTC
cloudweight looks to be an attempt to cluster the various weightings of terms into strata groups, 1 through 5.  This isn't even the tag's raw weight, just it's relative weight projected to a 5 point scale.  I don't think this is worth keeping, as it does nothing to inform the patron about anything they can actually use.

Marking this as Passed QA, as it's perfectly valid code, and that it removes is okay (by my opinion) to remove.
Comment 16 Chris Cormack 2012-06-23 07:22:11 UTC
Pushed to 3.8.x, will be in 3.8.3
Comment 17 Robin Sheat 2012-06-29 09:07:40 UTC
I've found this causes a regression: the tags no longer display in their correct size, they all appear the same, as the value in the 'title' field was used by javascript to calculate the font-size attribute.

I'm hoping to get this fixed in not too long.
Comment 18 Robin Sheat 2012-06-29 09:08:18 UTC
Also, the size calculation logic should probably get moved to template toolkit rather than javascript.
Comment 19 Robin Sheat 2012-07-18 15:40:21 UTC Comment hidden (obsolete)
Comment 20 Robin Sheat 2012-07-18 15:41:39 UTC
This should be applied on top of the previous patch.
Comment 21 Robin Sheat 2012-07-18 15:49:19 UTC Comment hidden (obsolete)
Comment 22 Robin Sheat 2012-07-18 15:50:09 UTC
This one just removes some CSS stuff that wasn't supposed to be there.
Comment 23 Robin Sheat 2012-07-18 15:55:26 UTC Comment hidden (obsolete)
Comment 24 Robin Sheat 2012-07-18 15:56:03 UTC
For serious this time.
Comment 25 Owen Leonard 2012-07-25 18:49:21 UTC
The first patch no longer applies for me.
Comment 26 Owen Leonard 2012-07-25 18:51:06 UTC
Oh never mind, the first one was already pushed.
Comment 27 Owen Leonard 2012-07-25 18:53:43 UTC
*** Bug 8445 has been marked as a duplicate of this bug. ***
Comment 28 Owen Leonard 2012-07-25 18:55:12 UTC
Same problem with the follow-up patch. Does not apply.

Apply? [y]es/[n]o/[e]dit/[v]iew patch/[a]ccept all y
Applying: Bug 7642 - fix the lost display of tag sizes
fatal: sha1 information is lacking or useless (koha-tmpl/opac-tmpl/prog/en/modules/opac-tags.tt).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 Bug 7642 - fix the lost display of tag sizes
Comment 29 Marc Véron 2012-09-06 12:58:37 UTC
I had the same error as well and do not know how to resolve it:

Applying: Bug 7642 - fix the lost display of tag sizes
fatal: sha1 information is lacking or useless (koha-tmpl/opac-tmpl/prog/en/modules/opac-tags.tt).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.

Marc
Comment 30 Robin Sheat 2012-09-06 14:37:05 UTC Comment hidden (obsolete)
Comment 31 Robin Sheat 2012-09-06 14:48:56 UTC
I think the issue was that I'd based it off a branch that you didn't have, and so it couldn't do 3-way merge when it found a conflict. Try this, which is based off current master.
Comment 32 Marc Véron 2012-09-06 18:00:40 UTC Comment hidden (obsolete)
Comment 33 Jonathan Druart 2012-09-20 12:03:14 UTC
QA Comments:
- Remove an unused routine (rectify_weights)
- Simplify tag weight management
- add UT

not blocker:
- add a tabulation character in C4/Tags.pm (but a lot of already exist)
- useless "use Data::Dumper" in Tags.t

Marked as Passed QA.
Comment 34 Katrin Fischer 2012-09-25 11:45:05 UTC
*** Bug 8814 has been marked as a duplicate of this bug. ***
Comment 35 Paul Poulain 2012-09-28 15:42:49 UTC
Some comments:
 * the patch does not apply anymore
Applying: Bug 7642 - fix the lost display of tag sizes
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
Auto-merging koha-tmpl/opac-tmpl/prog/en/css/opac.css
CONFLICT (content): Merge conflict in koha-tmpl/opac-tmpl/prog/en/css/opac.css
Auto-merging C4/Tags.pm
CONFLICT (content): Merge conflict in C4/Tags.pm
Failed to merge in the changes.
Patch failed at 0001 Bug 7642 - fix the lost display of tag sizes
When you have resolved this problem run "git am --resolved".
If you would prefer to skip this patch, instead run "git am --skip".
To restore the original branch and stop patching run "git am --abort".
Patch left in /tmp/SIGNED-OFF-Bug-7642---fix-the-lost-display-of-tag--OPHe1P.patch

 * you should add the css to ccsr theme

 * this patch is attached to 7642, but it's not really a follow-up of the previous part, shouldn't it be another bug number ?
Comment 36 Robin Sheat 2012-09-28 23:38:43 UTC
Created attachment 12604 [details] [review]
Bug 7642 - fix the lost display of tag sizes

This fixes the display of different sized words for differently popular
tags. It is a bit of a refactor of that part of the system, moving
logic to more sensible places (and removing an unused method on the
way.)

Note that it isn't an attempt to reproduce what was there previously,
just to do something similar, and in an easier to change fashion.

Sponsored-By: New Zealand Educational Institute
Signed-off-by: Marc Veron <veron@veron.ch>
Works as expected.
Comment 37 Robin Sheat 2012-09-28 23:40:01 UTC
Rebased against master, added CSS to ccsr theme, fixed Jonathan's QA comments.
Comment 38 Robin Sheat 2012-09-28 23:41:08 UTC
Oh, it probably could have got its own bug number, but I thought it was going to be a simple followup when I started it.
Comment 39 Paul Poulain 2012-10-01 15:38:54 UTC
Comment on attachment 12022 [details] [review]
[SIGNED-OFF] Bug 7642 - fix the lost display of tag sizes

this patch obsoleted by the rebased one
Comment 40 Paul Poulain 2012-10-01 15:43:47 UTC
Patch pushed to master, on master branch directly

The QA tools have detected a mistake in POD doc (that was here before this patch), I've fixed it too
Comment 41 Chris Cormack 2012-10-01 20:33:48 UTC
This wont apply to 3.8.x please rebase if it is needed there.
Comment 42 Robin Sheat 2012-10-01 22:10:06 UTC
Created attachment 12626 [details] [review]
Bug 7642 - [3.8.x] fix the lost display of tag sizes

This fixes the display of different sized words for differently popular
tags. It is a bit of a refactor of that part of the system, moving
logic to more sensible places (and removing an unused method on the
way.)

Note that it isn't an attempt to reproduce what was there previously,
just to do something similar, and in an easier to change fashion.

Sponsored-By: New Zealand Educational Institute
Signed-off-by: Marc Veron <veron@veron.ch>
Works as expected.
Comment 43 Robin Sheat 2012-10-01 22:11:06 UTC
Rebased against 3.8.x
Comment 44 Chris Cormack 2012-10-01 22:24:02 UTC
Pushed to 3.8.x, will be in 3.8.6