Open Bug 304989 Opened 19 years ago Updated 14 years ago

Cleanup, create classes, and add recipients table for whining

Categories

(Bugzilla :: Whining, enhancement)

2.21
enhancement
Not set
normal

Tracking

()

People

(Reporter: karl, Unassigned)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 4 obsolete files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.2) Gecko/20040804 Netscape/7.2 (ax)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.2) Gecko/20040804 Netscape/7.2 (ax)

I'd like to do the following things to Whining:

1) Create a new DB table, whine_recipients, which is used to store the
recipients for some given whine_schedule.  The new table would have the
following columns:
  schedule_id (indexed, not null): The ID of the associated schedule (foreign
key whine_schedules.id).
  user_id (null): If not NULL, this user is one recipient for the given schedule
(foreign key profiles.userid).
  group_id (null): If not NULL, the members of this group are one recipient for
the given schedule (foreign key groups.id).

2) Create the following classes: Whine (used to retrieve, update, and store
whines), Whine::Queries (same as Whine, except for whining queries),
Whine::Schedule (same for whining schedules), Whine::Recipient (same for whining
recipients.  Whine.pl and editwhines.cgi would be updated as needed to use the
new classes.

I'm mainly thinking of doing this so that future changes will be easier to
implement.  For example, implementing (1) means it will be easier for one
schedule to be associated with more than one recipient.

Reproducible: Always

Steps to Reproduce:
A Whine::Recipient should just be a Bugzilla::User, yes? And a Whine::Query is a
Bugzilla::Search.

However, having a Whine object is a good idea.

All of these separate changes (object, table structure, code re-working) should
happen in separate bugs.
Assignee: erik → karl
Version: unspecified → 2.21
(In reply to comment #1)
> A Whine::Recipient should just be a Bugzilla::User, yes?

Right now a whining recipient can either be a user or a group.

> And a Whine::Query is a Bugzilla::Search.

Not entirely.  A query is a reference to a saved search, with a whine-specific
title, sorting key, and at least one additional option.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Target Milestone: --- → Bugzilla 2.22
Blocks: 305057
Attached file Whine class diagram v1
This is the class diagram that I am using.  It shows all Whine classes that
will appear in the upcoming Patch v1.  Prefix "Bugzilla::" to all class names
to get the actual Perl package/class names.  Attributes are shown at the top of
each class; each attribute matches a column in the associated database table. 
Attributes with the weird T shape next to them are foreign keys.  Attributes
with a / get their values from other attributes.  Methods are shown at the
bottom.  The attached notes list all keys and indexes used by the database
table associated with the class.
Attached file Whine class diagram v1 (obsolete) —
This is the class diagram that I am using.  It shows all Whine classes that
will appear in the upcoming Patch v1.  Prefix "Bugzilla::" to all class names
to get the actual Perl package/class names.  Attributes are shown at the top of
each class; each attribute matches a column in the associated database table. 
Attributes with the weird T shape next to them are foreign keys.  Attributes
with a / get their values from other attributes.  Methods are shown at the
bottom.  The attached notes list all keys and indexes used by the database
table associated with the class.
Attached file Whine class diagram v1 (obsolete) —
This is the class diagram that I am using.  It shows all Whine classes that
will appear in the upcoming Patch v1.  Prefix "Bugzilla::" to all class names
to get the actual Perl package/class names.  Attributes are shown at the top of
each class; each attribute matches a column in the associated database table. 
Attributes with the weird T shape next to them are foreign keys.  Attributes
with a / get their values from other attributes.  Methods are shown at the
bottom.  The attached notes list all keys and indexes used by the database
table associated with the class.
Attachment #195333 - Attachment is obsolete: true
Attachment #195332 - Attachment is obsolete: true
Attached patch Patch v1 (obsolete) — Splinter Review
OK, here, finally, is the patch!  I hopefully it'll only be attached once...

The biggest changes are the additions of the 4 new Perl Modules. 
Bugzilla::Whine is directly associated with the whine_events table, and
reads/validates/writes entries in/to that table.  The same thing exists for
Bugzilla::Whine::Schedule and whine_schedules, Bugzilla::Whine::Query and
whine_queries, and Bugzilla::Whine::Recipients and whine_recipients.

Once question you might have is "Why separate the recipients from the
schedules"?  Well, I'm looking forward to the time when you might have multiple
recipients listed in a single schedule (which is bug 305370).

All 4 modules include several operations: new, all_*_for_*, can_access, delete,
validate, and write_to_database.  Those operations are fairly self-explanatory,
except for all_*_for_*, which returns a reference to an array of all
Bugzilla::Whine::Query objects that are associated with a given Whine, all
Bugzilla::Whine::Recipient objects that are associated with a given
Bugzilla::Whine::Schedule, etc.  Accessors are also included.  Full PODs are
also included.	Bugzilla::Whine::Recipient deserves special mention since it
contains a method to expand a recipient list (a user name and/or group name),
replacing a group name with a list of member users (eliminating duplicates that
appear).  Bugzilla::Whine::Schedule also deserves special mention since it
includes a method that calculates the next execution time of a schedule (not
the easiest thing in the world) without relying on database queries.

There have been some DB changes, including the addition of a new table. 
Bugzilla/DB/Schema.pm has been updated to reflect the changes.	checksetup.pl
has also been updated to incorporate the changes.

whine.pl and editwhines.cgi have both been modified to take advantage of the
new Perl Modules.  Both scripts have been enhanced with more error checking and
reporting, which means changes have been made to the global/user-error and
global/code-error templates.

sanitycheck.cgi has been updated to verify foreign key references in the couple
of areas where they weren't checked.  Other whining-related checks have also
been added.

Requesting review from LpSolit.
Attachment #195382 - Flags: review?(LpSolit)
Depends on: 277073
Blocks: 277073
No longer depends on: 277073
Comment on attachment 195382 [details] [diff] [review]
Patch v1

joel: You seem to know a lot about whining.  Would you be able to do a review
of this patch?
Attachment #195382 - Flags: review?(bugreport)
Attached file Test Driver v0.5
This is just a basic test program that I used to test the Whine classes as I
was creating them.  I don't know if it will help anyone, but here it is just in
case!
Comment on attachment 195382 [details] [diff] [review]
Patch v1

And here comes person #3!  Somewhere on the Internet someone is taking bets
over who will come in with a review first 8-)
Attachment #195382 - Flags: review?(jouni)
Attachment #195382 - Flags: review?(LpSolit)
Blocks: 277672
Attachment #195382 - Flags: review?(jouni)
Attachment #195382 - Flags: review?(bugreport)
Attached patch Patch v1.2 (obsolete) — Splinter Review
This is a modification of attachment 195382 [details] [diff] [review].  I found a few areas of Bugzilla
where the whining-related tables were being accessed directly, and have updated
them.
Attachment #195382 - Attachment is obsolete: true
Attachment #196393 - Flags: review?(bugreport)
Attachment #196393 - Flags: review?(jouni)
Karl:

I've crawled out from under my rock.  Want another reviewer?
Comment on attachment 196393 [details] [diff] [review]
Patch v1.2

(In reply to comment #11)
> Karl:
> 
> I've crawled out from under my rock.  Want another reviewer?

Certainly!  After some looking around I noticed that joel & jouni had reviewed
previous major whining changed, so I imagine that they would like a break!
Attachment #196393 - Flags: review?(erik)
Attachment #196393 - Flags: review?(bugreport)
drat
Target Milestone: Bugzilla 2.22 → Bugzilla 2.24
No longer blocks: 277073
Comment on attachment 196393 [details] [diff] [review]
Patch v1.2

This patch has become at least a little bitrotten in the time since I requested review, so I'm pulling the review requests until I can get a newly-derotted version in.

erik, jouni: If you managed to look over the patch at all, would you mind posting your comments?  Thanks!
Attachment #196393 - Attachment is obsolete: true
Attachment #196393 - Flags: review?(jouni)
Attachment #196393 - Flags: review?(erik)
This bug is retargetted to Bugzilla 3.2 for one of the following reasons:

- it has no assignee (except the default one)
- we don't expect someone to fix it in the next two weeks (i.e. before we freeze the trunk to prepare Bugzilla 3.0 RC1)
- it's not a blocker

If you are working on this bug and you think you will be able to submit a patch in the next two weeks, retarget this bug to 3.0.

If this bug is something you would like to see implemented in 3.0 but you are not a developer or you don't think you will be able to fix this bug yourself in the next two weeks, please *do not* retarget this bug.

If you think this bug should absolutely be fixed before we release 3.0, either ask on IRC or use the "blocking3.0 flag".
Target Milestone: Bugzilla 3.0 → Bugzilla 3.2
Bugzilla 3.2 is now frozen. Only enhancements blocking 3.2 or specifically approved for 3.2 may be checked in to the 3.2 branch. If you would like to nominate your enhancement for Bugzilla 3.2, set the "blocking3.2" flag to "?". Then, either the target milestone will be changed back, or the blocking3.2 flag will be granted, if we will accept this enhancement for Bugzilla 3.2.

This particular bug has not been touched in over eight months, and thus is being retargeted to "---" instead of "Bugzilla 4.0". If you believe this is a mistake, feel free to retarget it to Bugzilla 4.0.
Target Milestone: Bugzilla 3.2 → ---
Assignee: karl.kornel → whining
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: