Bug 372979 (bz-plugin-vote)

[Voting] Voting should be an extension

RESOLVED FIXED in Bugzilla 4.0

Status

()

enhancement
P2
normal
RESOLVED FIXED
12 years ago
8 years ago

People

(Reporter: mkanat, Assigned: mkanat)

Tracking

(Blocks 1 bug)

Bugzilla 4.0
Dependency tree / graph
Bug Flags:
approval +

Details

(Whiteboard: [3.6 Focus])

Attachments

(1 attachment, 10 obsolete attachments)

(Assignee)

Description

12 years ago
"Voting" isn't really part of the core concept of defect tracking, and could easily become a plugin for Bugzila instead of being a core feature.

This would possibly also help out with the fact of voting not really being maintained currently, in Bugzilla.
(Assignee)

Updated

12 years ago
Summary: Voting should be a plugin → [Voting] Voting should be a plugin
(Assignee)

Updated

12 years ago
Priority: -- → P3
(Assignee)

Updated

12 years ago
Depends on: 162060
(Assignee)

Updated

12 years ago
Duplicate of this bug: voteoverhaul
(Assignee)

Updated

10 years ago
Blocks: bz-majorarch
Summary: [Voting] Voting should be a plugin → [Voting] Voting should be an extension
(Assignee)

Updated

10 years ago
Assignee: general → mkanat
Priority: P3 → P2
Target Milestone: --- → Bugzilla 4.0

Comment 2

10 years ago
As said in bug 156174 comment 5 (and confirmed by bug 156174 comment 6), the "confirm by popular vote" feature is silly, as users who should be allowed to confirm bugs have canconfirm privs already. So if we remove this bit of the voting system, this would make the extension much easier to write, especially with bug 162060 being checked in.

Would someone object to remove the "confirm by popular vote" feature before we move the voting system into an extension?
(Assignee)

Updated

10 years ago
Keywords: student-project
Max, any objection to this being reassigned to nobody so it's clear that a student can work on it?
(Assignee)

Comment 4

10 years ago
No objection. However, in the Bugzilla product, just reassign to the default assignee, because it's a watching account.
Assignee: mkanat → general
(Assignee)

Updated

10 years ago
Whiteboard: [3.6 Focus]
(Assignee)

Updated

10 years ago
Component: Bugzilla-General → Extensions
(Assignee)

Updated

10 years ago
Assignee: general → extensions
(Assignee)

Comment 5

9 years ago
Okay, given that a few of the enhancement requests with the highest votes in the Bugzilla product are related to the voting system, I think that I'm going to fix this, now that the blocker is fixed.
Assignee: extensions → mkanat
Status: NEW → ASSIGNED
Keywords: student-project
Target Milestone: Bugzilla 4.0 → Bugzilla 3.8
(Assignee)

Updated

9 years ago
Depends on: 545524
(Assignee)

Updated

9 years ago
Depends on: 545541
(Assignee)

Updated

9 years ago
Depends on: 545551
(Assignee)

Comment 6

9 years ago
Posted patch Work In Progress (obsolete) — Splinter Review
Here's a work in progress. I don't even know if this compiles, but I've moved pretty much everything out of Bugzilla::Bug, Bugzilla::Product, and votes.cgi.

Comment 7

9 years ago
Please take comment 2 into account, and also because you can set the bug status back to UNCO even when a bug has enough votes to confirm, leading to inconsistencies and making sanitycheck.cgi to complain. Even as an extension, this shouldn't generate inconsistencies.
(Assignee)

Updated

9 years ago
Depends on: 545576
(Assignee)

Comment 8

9 years ago
Well, we can consider removing that feature out after I move this to an extension. I've already spent several hours on this patch as it is, and I'd have to re-do a lot of it if I removed the feature now.
(Assignee)

Comment 9

9 years ago
Posted patch Work In Progress 2 (obsolete) — Splinter Review
This one compiles, but probably doesn't work yet. There's still a lot of work to go, some of it very complicated.
Attachment #426428 - Attachment is obsolete: true
(Assignee)

Updated

9 years ago
Depends on: 545587
(Assignee)

Comment 10

9 years ago
Posted patch Work In Progress 3 (obsolete) — Splinter Review
Some more progress. This compiles and has sanitycheck and a lot of the admin stuff done.
Attachment #426438 - Attachment is obsolete: true
(Assignee)

Comment 11

9 years ago
Posted patch Work In Progress 4 (obsolete) — Splinter Review
Almost done now.
Attachment #426454 - Attachment is obsolete: true
(In reply to comment #8)
> Well, we can consider removing that feature out after I move this to an
> extension. I've already spent several hours on this patch as it is, and I'd
> have to re-do a lot of it if I removed the feature now.

Well, I think it's more valuable to remove the feature first, which certainly leads to a cleaner code, than wasting time moving it into an extension and remove it later.
(Assignee)

Updated

9 years ago
Depends on: 545682
(Assignee)

Updated

9 years ago
Depends on: 545683
(Assignee)

Updated

9 years ago
Depends on: 545715
(Assignee)

Comment 13

9 years ago
Posted patch Work In Progress 5 (obsolete) — Splinter Review
REL_VOTER is now moved entirely into the extension.
Attachment #426455 - Attachment is obsolete: true
(Assignee)

Updated

9 years ago
Depends on: 545766
(Assignee)

Updated

9 years ago
Depends on: 545770
(Assignee)

Comment 14

9 years ago
Posted patch Work In Progress 6 (obsolete) — Splinter Review
Okay, I've moved all the code, but very little of it has been tested. I know that some things in this patch won't work at all.
Attachment #426558 - Attachment is obsolete: true
(Assignee)

Updated

9 years ago
Depends on: 545787
(Assignee)

Comment 15

9 years ago
Posted patch WIP 7 (obsolete) — Splinter Review
More work on getting things to actually function. I'm having a little problem with changing votes using user.html right now, though.
Attachment #426602 - Attachment is obsolete: true
(Assignee)

Comment 16

9 years ago
Posted patch v1 (obsolete) — Splinter Review
Okay, here we go, this is feature-complete and fully-functional. I tested every single subroutine, every single template, and every single hook.

Currently, if a vote confirms a bug, TT will exit without output due to a very strange TT bug that I cannot work around currently ( https://rt.cpan.org/Public/Bug/Display.html?id=47929 ). However, I think it will be handled if we stop sending email from the template, which is something I think we want to do anyway, and it's not the most common occurrence that a vote confirms a bug.

There are a few features that are not fully active, but will be active once all the blockers are cleared. However, this patch functions and applies without any of the blockers.

This is an enormous patch, and it took a tremendous amount of work that I have tested extremely thoroughly. Most of the actual templates and main code was actually just copied around, so only a small percentage of it is new.

What I would like to do is to basically grant myself "module owner" review, since it's an Extension, and then check it in after a quick look-over from LpSolit or whoever would like to.

Does that sound OK?
Attachment #426623 - Attachment is obsolete: true
Attachment #426798 - Flags: review?(LpSolit)
(Assignee)

Comment 17

9 years ago
Posted patch v2 (obsolete) — Splinter Review
This fixes the fake "votes.cgi" to properly do redirects.
Attachment #426798 - Attachment is obsolete: true
Attachment #426917 - Flags: review?(LpSolit)
Attachment #426798 - Flags: review?(LpSolit)
(Assignee)

Comment 18

9 years ago
Posted patch v3 (obsolete) — Splinter Review
I found one more bug, which may have actually been happening in Bugzilla already, where voting/user.html was showing every single product instead of just products that had bugs with votes in them.
Attachment #426917 - Attachment is obsolete: true
Attachment #426922 - Flags: review?(LpSolit)
Attachment #426917 - Flags: review?(LpSolit)
(Assignee)

Comment 19

9 years ago
Posted patch v4Splinter Review
The votes table wasn't getting created properly in an empty database.
Attachment #426922 - Attachment is obsolete: true
Attachment #426925 - Flags: review?(LpSolit)
Attachment #426922 - Flags: review?(LpSolit)
(Assignee)

Comment 20

9 years ago
Comment on attachment 426925 [details] [diff] [review]
v4

Okay, so I talked to LpSolit about this on IRC, and he says that it's OK to check this in, but he still wants to look it over after checkin, so I'm going to keep the review flag set.
Attachment #426925 - Flags: review+
(Assignee)

Updated

9 years ago
Flags: approval+
(Assignee)

Comment 21

9 years ago
Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/trunk/                                                                                                      
modified .bzrignore
modified buglist.cgi
modified bugzilla.dtd
modified colchange.cgi                                                                                                                                        
modified editproducts.cgi
modified editusers.cgi
modified importxml.pl
modified process_bug.cgi
modified query.cgi
modified report.cgi
modified sanitycheck.cgi
added votes.cgi
modified Bugzilla/Bug.pm
modified Bugzilla/BugMail.pm
modified Bugzilla/Comment.pm
modified Bugzilla/Constants.pm
modified Bugzilla/Field.pm
modified Bugzilla/Object.pm
modified Bugzilla/Product.pm
modified Bugzilla/Search.pm
modified Bugzilla/Config/BugFields.pm
modified Bugzilla/DB/Schema.pm
modified Bugzilla/Install/DB.pm
modified Bugzilla/Search/Quicksearch.pm
modified Bugzilla/WebService/Bug.pm
modified contrib/merge-users.pl
modified docs/en/images/bzLifecycle.xml
added extensions/Voting
renamed votes.cgi => extensions/Voting/Extension.pm
added extensions/Voting/template
added extensions/Voting/web
added extensions/Voting/template/en
added extensions/Voting/template/en/default
added extensions/Voting/template/en/default/hook
added extensions/Voting/template/en/default/pages
added extensions/Voting/template/en/default/voting
added extensions/Voting/template/en/default/hook/account
added extensions/Voting/template/en/default/hook/admin
added extensions/Voting/template/en/default/hook/bug
missing extensions/Voting/template/en/default/hook/email
added extensions/Voting/template/en/default/hook/global
added extensions/Voting/template/en/default/hook/search
added extensions/Voting/template/en/default/hook/account/prefs
added extensions/Voting/template/en/default/hook/account/prefs/email-relationships.html.tmpl
added extensions/Voting/template/en/default/hook/admin/products
added extensions/Voting/template/en/default/hook/admin/sanitycheck
added extensions/Voting/template/en/default/hook/admin/users
added extensions/Voting/template/en/default/hook/admin/products/edit-common-rows.html.tmpl
added extensions/Voting/template/en/default/hook/admin/products/updated-changes.html.tmpl
added extensions/Voting/template/en/default/hook/admin/sanitycheck/messages-statuses.html.tmpl
added extensions/Voting/template/en/default/hook/admin/users/confirm-delete-warn_safe.html.tmpl
added extensions/Voting/template/en/default/hook/bug/edit-after_importance.html.tmpl
added extensions/Voting/template/en/default/hook/bug/format_comment-type.txt.tmpl
added extensions/Voting/template/en/default/hook/bug/process
added extensions/Voting/template/en/default/hook/bug/process/header-title.html.tmpl
added extensions/Voting/template/en/default/hook/bug/process/results-title.html.tmpl
missing extensions/Voting/template/en/default/hook/email/newchangedmail-relationship.txt.tmpl
missing extensions/Voting/template/en/default/hook/email/newchangedmail-watch_relationship.txt.tmpl
added extensions/Voting/template/en/default/hook/global/field-descs-end.none.tmpl
added extensions/Voting/template/en/default/hook/global/reason-descs-end.none.tmpl
added extensions/Voting/template/en/default/hook/global/user-error-errors.html.tmpl
added extensions/Voting/template/en/default/hook/search/form-email_numbering_end.html.tmpl
added extensions/Voting/template/en/default/hook/search/search-report-select-rep_fields.html.tmpl
renamed template/en/default/bug/votes => extensions/Voting/template/en/default/pages/voting
renamed template/en/default/pages/voting.html.tmpl => extensions/Voting/template/en/default/pages/voting.html.tmpl
renamed template/en/default/bug/votes/list-for-bug.html.tmpl => extensions/Voting/template/en/default/pages/voting/bug.html.tmpl
renamed template/en/default/bug/votes/list-for-user.html.tmpl => extensions/Voting/template/en/default/pages/voting/user.html.tmpl
missing extensions/Voting/template/en/default/voting/README
renamed template/en/default/bug/votes/delete-all.html.tmpl => extensions/Voting/template/en/default/voting/delete-all.html.tmpl
renamed template/en/default/email/votes-removed.txt.tmpl => extensions/Voting/template/en/default/voting/votes-removed.txt.tmpl
renamed skins/standard/voting.css => extensions/Voting/web/style.css
modified skins/standard/show_bug.css
modified template/en/default/filterexceptions.pl
modified template/en/default/sidebar.xul.tmpl
modified template/en/default/account/prefs/email.html.tmpl
modified template/en/default/admin/params/bugfields.html.tmpl
modified template/en/default/admin/products/create.html.tmpl                                                                                                  
modified template/en/default/admin/products/edit-common.html.tmpl
modified template/en/default/admin/products/list.html.tmpl
modified template/en/default/admin/products/updated.html.tmpl
modified template/en/default/admin/sanitycheck/messages.html.tmpl
modified template/en/default/admin/users/confirm-delete.html.tmpl
modified template/en/default/bug/edit.html.tmpl
modified template/en/default/bug/format_comment.txt.tmpl
modified template/en/default/bug/process/header.html.tmpl
modified template/en/default/bug/process/results.html.tmpl
modified template/en/default/email/newchangedmail.txt.tmpl
modified template/en/default/global/field-descs.none.tmpl
added template/en/default/global/reason-descs.none.tmpl
modified template/en/default/global/site-navigation.html.tmpl
modified template/en/default/global/user-error.html.tmpl
modified template/en/default/list/list.rdf.tmpl
modified template/en/default/search/form.html.tmpl
modified template/en/default/search/search-help.html.tmpl
modified template/en/default/search/search-report-select.html.tmpl
Committed revision 6991.
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
(Assignee)

Comment 22

9 years ago
I also had to move the no_open_bug_status CodeError into the extension:

Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/trunk/                       
modified extensions/Voting/Extension.pm
added extensions/Voting/template/en/default/hook/global/code-error-errors.html.tmpl
modified template/en/default/global/code-error.html.tmpl
Committed revision 6992.
(Assignee)

Comment 23

9 years ago
Fixed a few other errors found by the tinderbox, and disabled the extension by default:

Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/trunk/                       
modified extensions/Voting/Extension.pm
added extensions/Voting/disabled
modified extensions/Voting/template/en/default/hook/global/code-error-errors.html.tmpl
Committed revision 6993

Updated

9 years ago
Keywords: relnote

Updated

9 years ago
Attachment #426925 - Flags: review?(LpSolit) → review+
Comment on attachment 426925 [details] [diff] [review]
v4

Passes some basic tests. r=LpSolit (we really need a UI to enable/disable extensions!)
(Assignee)

Comment 25

9 years ago
(In reply to comment #24)
> (we really need a UI to enable/disable extensions!)

  Agreed! It wouldn't be too hard now, even, since extensions have an "enabled" method. Just have to do it.

Updated

9 years ago
Blocks: 558032

Updated

9 years ago
Blocks: 561745

Updated

9 years ago
Blocks: 578258

Updated

9 years ago
Blocks: 581316
(Assignee)

Comment 26

9 years ago
Added to the release notes in bug 604256.
Keywords: relnote

Updated

8 years ago
Blocks: 652381

Updated

8 years ago
Blocks: 313448
You need to log in before you can comment on or make changes to this bug.