Closed
Bug 530468
Opened 15 years ago
Closed 14 years ago
Create a read-only Bugzilla::Whine object for whine events and have editwhines.cgi use it
Categories
(Bugzilla :: Whining, enhancement)
Bugzilla
Whining
Tracking
()
RESOLVED
FIXED
Bugzilla 4.0
People
(Reporter: eblack, Assigned: eblack)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 1 obsolete file)
5.99 KB,
patch
|
mkanat
:
review+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.0.13) Gecko/2009080316 Ubuntu/8.04 (hardy) Firefox/3.0.13 Build Identifier: Based on Bug 509959, Max Kanat-Alexander suggested I create read-only objects to make the transition easier to objects for whining. Bug 511028 was for read only schedules, bug 530467 for read only queries, and this is the third bug for the read only whine event objects Reproducible: Always
Status: UNCONFIRMED → RESOLVED
Closed: 15 years ago
Resolution: --- → DUPLICATE
Comment 2•15 years ago
|
||
This one is about whine events, the other one about whine queries.
Status: RESOLVED → UNCONFIRMED
Resolution: DUPLICATE → ---
(In reply to comment #2) > This one is about whine events, the other one about whine queries. Whoops! My apologies.
Updated•15 years ago
|
Assignee: whining → eblack
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment 4•15 years ago
|
||
Eric: I added you to the canconfirm and editbugs groups today, so you can now assign bugs to yourself and confirm your own bugs if you think that they should be confirmed.
Updated•15 years ago
|
Summary: Create a read-only object using Bugzilla::Object for whine events → Create a read-only Bugzilla::Whine::Event object for whine events and have editwhines.cgi use it
Assignee | ||
Comment 5•14 years ago
|
||
Max, I'm adding this patch for the event object, but if you look at editwhines, there are still alot of select statements that are joins between two tables/objects, and selects of multiple events (this object). They don't seem to belong in this object class, but I think they should be in an overall 'whine' object. I think that's what you meant in Bug 509959. Please let me know if that is correct and if I should create another bug for a read-only Bugzilla::Whine object. Thanks
Attachment #426613 -
Flags: review?(mkanat)
Updated•14 years ago
|
Attachment #426613 -
Attachment is patch: true
Attachment #426613 -
Attachment mime type: application/octet-stream → text/plain
Comment 6•14 years ago
|
||
Hmm, an Event *is* a Whine, right? I think we should just call this one Bugzilla::Whine. Selects of multiple events are handled by new_from_list and match(). I suspect that the joins that we have in direct SQL are not necessary and can be re-implemented in an object-oriented fashion.
Comment 7•14 years ago
|
||
Comment on attachment 426613 [details] [diff] [review] Read only Bugzilla::Whine::Event mod and changes to editwhines.cgi >Index: editwhines.cgi > sub get_events { > my $userid = shift; > my $dbh = Bugzilla->dbh; > my $events = {}; > >- my $sth = $dbh->prepare("SELECT DISTINCT id, subject, body, mailifnobugs " . >+ my $sth = $dbh->prepare("SELECT DISTINCT id " . > "FROM whine_events " . > "WHERE owner_userid=?"); This can be replaced with: my $events = Bugzilla::Whine->match({ owner_userid => $userid }); >+++ Bugzilla/Whine/Event.pm 2010-01-31 14:39:55.000000000 -0500 [snip] >+sub get_schedules { We don't prefix our accessors with get_, so this would just be "schedules". Also, I didn't see editwhines.cgi using any of these complex accessors? >+sub whine_users { I think this might be better as to_users? >+ my $ids = Bugzilla->dbh->selectcol_arrayref('SELECT owner_userid >+ FROM whine_events >+ INNER JOIN whine_queries >+ ON whine_events.id = whine_queries.eventid >+ WHERE query_id = ? and owner_userid != ?', I'm not quite familiar enough with the whine code to understand this, but why are you checking query_id as the event's id? Also, why even join whine_queries for this? Isn't owner_userid from this table, and it will just return one user? >+sub userid { return $_[0]->{'owner_userid'}; } I'd rather have only "sub owner" or "sub user" that returns a Bugzilla::User object, unless there's a good reason. If there *is* a good reason to keep this, it should be called owner_id or user_id probably.
Attachment #426613 -
Flags: review?(mkanat) → review-
Updated•14 years ago
|
Summary: Create a read-only Bugzilla::Whine::Event object for whine events and have editwhines.cgi use it → Create a read-only Bugzilla::Whine object for whine events and have editwhines.cgi use it
Updated•14 years ago
|
Target Milestone: --- → Bugzilla 3.8
Comment 8•14 years ago
|
||
Hey Eric. Any hope of getting an updated version of this patch some time soon?
Assignee | ||
Comment 9•14 years ago
|
||
(In reply to comment #8) > Hey Eric. Any hope of getting an updated version of this patch some time soon? My apologies, yes, I'll work on it. Thanks for reminding me.
Assignee | ||
Comment 10•14 years ago
|
||
I changed this from a Bugzilla::Whine::Event object to a Bugzilla::Whine object. I also removed the complex accessors for now as they were for the sql joins in editwhines.cgi, but I agree that they might not be necessary. I was figuring on incremental patch submissions to remove them to make it simpler, but your suggestion is simpler, and the sql joins can be handled later. A user object is now returned rather than the userid for the 'user' method.
Attachment #426613 -
Attachment is obsolete: true
Attachment #455046 -
Flags: review?(mkanat)
Comment 11•14 years ago
|
||
Comment on attachment 455046 [details] [diff] [review] New patch of read only Bugzilla::Whine object and editwhines.cgi updates to use it This looks simple and excellent. Thank you! :-)
Attachment #455046 -
Flags: review?(mkanat) → review+
Updated•14 years ago
|
Flags: approval+
Comment 12•14 years ago
|
||
Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/trunk/ modified editwhines.cgi added Bugzilla/Whine.pm Committed revision 7270.
Status: NEW → RESOLVED
Closed: 15 years ago → 14 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•