Closed Bug 658926 Opened 13 years ago Closed 6 years ago

[Voting] add "including you" to "with 37 votes" on show_bug

Categories

(Bugzilla :: User Interface, enhancement, P3)

enhancement

Tracking

()

RESOLVED INCOMPLETE

People

(Reporter: worcester12345, Unassigned)

References

()

Details

Attachments

(1 file, 4 obsolete files)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.0.4) Gecko/2008102920 Firefox/3.0.4 (Splashtop-v1.4.10.21)
Build Identifier: current bugzilla

reference:
Importance:  	-- normal   with 37 votes  (vote) 

CC List:   	26 users including you  (edit) 


It would be nice if the former also had the "including you" part, so one doesn't need to check if they have already voted for the bug.

Reproducible: Always

Steps to Reproduce:
1.view bug
2.don't see if you have voted for it
3.

Actual Results:  
have to click on "vote" and go to another screen

Expected Results:  
be able to see voting status right then and there

no additional information at this time
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows XP → All
Priority: -- → P3
Hardware: x86 → All
Summary: add "including you" to: Importance: -- normal with 37 votes (vote) → [Voting] add "including you" to "with 37 votes" on show_bug
Bugzilla project does not use those keywords for its workflow.

thanks
dkl
Quick patch that adds "including you" to the show_bug.cgi page if the current user has voted at least 1 vote on the current bug.

dkl
Attachment #534365 - Flags: review?(mkanat)
Comment on attachment 534365 [details] [diff] [review]
Patch to show including you on bug edit if user has voted for bug (v1)

>=== modified file 'extensions/Voting/Extension.pm'
>+BEGIN {
>+    *Bugzilla::Bug::user_votes = \&_user_votes;
>+}

  Move this block to near the top of the file, below the constants.

>+sub _user_votes {

  Let's call this _bug_user_votes.


  You can make these fixes on checkin.
Attachment #534365 - Flags: review?(mkanat) → review+
Flags: approval+
Target Milestone: --- → Bugzilla 5.0
Severity: normal → enhancement
Holding approval until we branch.
Flags: approval+ → approval?
(In reply to comment #5)
> Holding approval until we branch.

Thanks.
(In reply to comment #5)
> Holding approval until we branch.

This seems to be either fixed or WFM.
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → WORKSFORME
(In reply to comment #7)
> This seems to be either fixed or WFM.

the fix has been applied to bmo, however it hasn't been committed to bugzilla itself yet.
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
Assignee: ui → dkl
Status: REOPENED → ASSIGNED
(In reply to comment #7)
> (In reply to comment #5)
> This seems to be either fixed or WFM.


(In reply to comment #8)
> (In reply to comment #7)
> the fix has been applied to bmo, however it hasn't been committed to
> bugzilla itself yet.

(In reply to comment #0)
> It would be nice if the former also had the "including you" part, so one
> doesn't need to check if they have already voted for the bug.

> Expected Results:  
> be able to see voting status right then and there

Actually, Bugzilla now has what it was I was looking for when I started this bug, so for me, it is working. Thanks.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago13 years ago
Resolution: --- → WORKSFORME
Leave this bug open! This patch has not been committed upstream. This bug is about Bugzilla the product, not bugzilla.mozilla.org. We will mark it as resolved once the patch is committed.
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
Status: REOPENED → ASSIGNED
When a user already has votes for a bug, you now see:

"with 37 votes including you (vote)"

The "(vote)" link looks confusing to me. Bugzilla tells you that you have already voted for this bug, and suggests you to vote again. Maybe it should be reworded to "(edit votes)" or "(remove votes)" or even not displayed, and linkify "including you" instead.
Good idea LpSolit. Here is a new patch that is less confusing.

dkl
Attachment #534365 - Attachment is obsolete: true
Attachment #551391 - Flags: review?(LpSolit)
Comment on attachment 551391 [details] [diff] [review]
Patch to show including you on bug edit if user has voted for bug (v2)

My preference would be to not display (vote) at all and linkify "including you" instead, to save some room (and you could add title="Edit your votes" to make things clearer). Else the page could become too wide, especially in some languages where "edit votes" would become much longer. In french, it would be "editer les votes". And once someone has voted for a bug, I doubt he changes his votes very often, and the penalty from a UI point of view seems important.

I will let pyrzak decide what's best.
Attachment #551391 - Flags: review?(LpSolit) → review?(guy.pyrzak)
Here is a revised patch with your suggested change. Once we get Guy's feedback we can choose which one can go in.

dkl
Attachment #551904 - Flags: review?(LpSolit)
Comment on attachment 551904 [details] [diff] [review]
Patch to show including you on bug edit if user has voted for bug (v3)


>+    *Bugzilla::Bug::user_votes = \&_user_votes;

pyrzak and I both agree that it should be a Bugzilla::User object method, not a Bugzilla::Bug object method. $bug->user_votes makes us think about the number of total votes a bug has. Probably $user->has_votes($bug) would make more sense.
Comment on attachment 551391 [details] [diff] [review]
Patch to show including you on bug edit if user has voted for bug (v2)

There are a few things I'd like to see different.

First I think the API to find out if the user has voted on the current bug is weird. It should probably be something like user.has_votes(bug), that's what LpSolit suggested.

Second, I think votes needs its own proper field with this addition. I tried a few different way of keeping it on 1 line, but since it is its own module and adding "including you" and (edit votes) makes the line a lot longer, and any variations I could think of still made the line too long, I think the only reasonable thing to do is give votes it's own "field"
Attachment #551391 - Flags: review?(guy.pyrzak) → review-
Comment on attachment 551904 [details] [diff] [review]
Patch to show including you on bug edit if user has voted for bug (v3)

r- as pyrzak disagrees with the UI, and we don't agree with the object method.
Attachment #551904 - Flags: review?(LpSolit) → review-
Removing the approval request for now.
Flags: approval?
Thanks for the suggestions. Here is a new patch that:

1. Makes has_votes a function of Bugzilla::User.pm.
2. Moves the votes to its own field on the right side

Please review
dkl
Attachment #551391 - Attachment is obsolete: true
Attachment #551904 - Attachment is obsolete: true
Attachment #557015 - Flags: review?(LpSolit)
Comment on attachment 557015 [details] [diff] [review]
Patch to show including you on bug edit if user has voted for bug (v4)

>+sub _has_votes {

>+    $self->{'has_votes'} = {} if !exists $self->{'has_votes'};

err... here $self->{'has_votes'} is a hashref, but...

>+    return $self->{'has_votes'}{$bug_id} if exists $self->{'has_votes'}{$bug_id};

... here it's a hash?


>+    return $self->{'has_votes'}{$bug_id};

The method is of type boolean, i.e. it should return either 0 or 1, not the number of votes.


I didn't look at the template yet.
(In reply to Frédéric Buclin from comment #20)
> Comment on attachment 557015 [details] [diff] [review]
> Patch to show including you on bug edit if user has voted for bug (v4)
> 
> >+sub _has_votes {
> 
> >+    $self->{'has_votes'} = {} if !exists $self->{'has_votes'};
> 
> err... here $self->{'has_votes'} is a hashref, but...
> 
> >+    return $self->{'has_votes'}{$bug_id} if exists $self->{'has_votes'}{$bug_id};
> 
> ... here it's a hash?

This is a shortcut. $self->{'has_votes'}{$bug_id} works out to be the same
as $self->{'has_votes'}->{$bug_id}. I will change it to make it more apparent.

> >+    return $self->{'has_votes'}{$bug_id};
> 
> The method is of type boolean, i.e. it should return either 0 or 1, not the
> number of votes.

New patch coming up.
New patch with suggested changes. Please review.

dkl
Attachment #557015 - Attachment is obsolete: true
Attachment #557015 - Flags: review?(LpSolit)
Attachment #557091 - Flags: review?(LpSolit)
While you're all in their changing and fixing this, I wonder if you can flip it so when one already has voted for the bug, the wording would change to:

"with 2 votes including you (un-vote)"

Notice the "un" before vote. This would make it more clear to those who want to remove their vote. Thanks.
(In reply to Worcester12345 from comment #23)
> While you're all in their changing and fixing this, I wonder if you can flip
> it so when one already has voted for the bug, the wording would change to:
> 
> "with 2 votes including you (un-vote)"
> 
> Notice the "un" before vote. This would make it more clear to those who want
> to remove their vote. Thanks.

Not sure if un-vote would be the best wording as a user can also go to the same form to add additional votes on top of the ones they have already. 

dkl
(In reply to David Lawrence [:dkl] from comment #24)
> Not sure if un-vote would be the best wording as a user can also go to the
> same form to add additional votes on top of the ones they have already. 

This will no longer be a problem once we remove the ability to add more than one vote per bug, which I plan to do in the coming few months. I think the current voting system is overkill. All the user should be allowed to do is to vote for or against a bug, period. It doesn't mean anything to be able to give 3 votes to a bug. Everywhere where the limit is larger than one, users put the max number of votes allowed for one bug. So you could have 60 votes, but this means 6 users with 10 votes each, which is no-sense. So I propose to postpone this patch/RFE till bug 48570 is implemented.
Depends on: 48570
Comment on attachment 557091 [details] [diff] [review]
Patch to show including you on bug edit if user has voted for bug (v5)

Per my previous comment.
Attachment #557091 - Flags: review?(LpSolit) → review-
I've been out of the loop a while, but hasn't this one been fixed?
No. Maybe you have the "including you" from the CC list in mind?
We are going to branch for Bugzilla 4.4 next week and this bug is too invasive or too risky to be accepted for 4.4 at this point. The target milestone is set to 5.0 which is our next major release.

I ask the assignee to reassign the bug to the default assignee if you don't plan to work on this bug in the near future, to make it clearer which bugs should be fixed by someone else on time for 5.0.
Target Milestone: Bugzilla 4.4 → Bugzilla 5.0
Flags: blocking4.2.5?
This bug is targetted 5.0, and is not a bug anyway but an enhancement. Not a blocker.
Flags: blocking4.2.5? → blocking4.2.5-
(In reply to Worcester12345 from comment #0)
> User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.0.4)
> Gecko/2008102920 Firefox/3.0.4 (Splashtop-v1.4.10.21)
> Build Identifier: current bugzilla
> 
> reference:
> Importance:  	-- normal   with 37 votes  (vote) 
> 
> CC List:   	26 users including you  (edit) 
> 
> 
> It would be nice if the former also had the "including you" part, so one
> doesn't need to check if they have already voted for the bug.

Seeing:
 with 2 votes including you (vote) 

WFM!
Status: ASSIGNED → RESOLVED
Closed: 13 years ago12 years ago
Resolution: --- → WORKSFORME
(In reply to Worcester12345 from comment #31)
> Seeing:
>  with 2 votes including you (vote) 
> WFM!

this issue hasn't been fixed in bugzilla.  you're seeing it on bugzilla.mozilla.org because we're running a customised version which includes dkl's latest patch.
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
OK, thanks.
Target Milestone: Bugzilla 5.0 → ---
Assignee: dkl → ui
(In reply to Worcester12345 from comment #27)
> I've been out of the loop a while, but hasn't this one been fixed?

(In reply to Worcester12345 from comment #31)
> (In reply to Worcester12345 from comment #0)
> > User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.0.4)
> > Gecko/2008102920 Firefox/3.0.4 (Splashtop-v1.4.10.21)
> > Build Identifier: current bugzilla
> > 
> > reference:
> > Importance:  	-- normal   with 37 votes  (vote) 
> > 
> > CC List:   	26 users including you  (edit) 
> > 
> > 
> > It would be nice if the former also had the "including you" part, so one
> > doesn't need to check if they have already voted for the bug.
> 
> Seeing:
>  with 2 votes including you (vote) 
> 
> WFM!

Can this close yet? Or are we waiting for this?
(In reply to Frédéric Buclin from comment #25)
> (In reply to David Lawrence [:dkl] from comment #24)
> > Not sure if un-vote would be the best wording as a user can also go to the
> > same form to add additional votes on top of the ones they have already. 
> 
> This will no longer be a problem once we remove the ability to add more than
> one vote per bug, which I plan to do in the coming few months. I think the
> current voting system is overkill. All the user should be allowed to do is
> to vote for or against a bug, period. It doesn't mean anything to be able to
> give 3 votes to a bug. Everywhere where the limit is larger than one, users
> put the max number of votes allowed for one bug. So you could have 60 votes,
> but this means 6 users with 10 votes each, which is no-sense. So I propose
> to postpone this patch/RFE till bug 48570 is implemented.
Circling back through some of my older bugs while out sick. 

Anyone?
The current voting feature will probably be removed in favour of comment reactions like GitHub. (Bug 1421401)
If you’re not familiar with GitHub, see this post for reference: https://blog.github.com/2016-03-10-add-reactions-to-pull-requests-issues-and-comments/
So as current assistant bugzilla project, this is one of those old bugs that I'm embarrassed about.

As kohei points out above, voting will probably be replaced with reactions.

Additionally, it is now obvious if the number of votes includes you or not, on the modern show bug UI.

Anyway, I am really sorry your patch had been left stale for this long.

The Bugzilla project is kind of different now, in the following ways:

1) The upstream master branch is considered "legacy", the current version of bugzilla.mozilla.org will be the 6.0 release.
2) Lots of stuff about project governance is different, detailed as #3 this wiki: https://github.com/bugzilla/harmony/wiki/Guiding-Principals

I'm going to resolve this as incomplete, because the patch doesn't seem to cleanly apply now
and what it attempted to do is *somewhat* like the current UI.

https://screenshots.firefox.com/DK4dv5nTqDztxw88/bugzilla.mozilla.org
Status: REOPENED → RESOLVED
Closed: 12 years ago6 years ago
Resolution: --- → INCOMPLETE
Might as well go whole hog and just "WONTFIX".
But it's sort of fixed, because we display something different depending on if you've voted or not.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: