Bugzilla::Attachment::query() should call new Bugzilla::Attachment for each attachment belonging to a given bug and most of the code inside query() should be moved into new(). Also, Flag.pm should not depend on Attachment.pm and we should add a new GetAttachment() function into Flag.pm itself, avoiding the Flag - Attachment dependency loop, as it's done already for Bugzilla::Flag::GetBug().
Status: NEW → ASSIGNED
Target Milestone: --- → Bugzilla 2.22
Created attachment 192737 [details] [diff] [review] patch, v1 To test Attachment.pm, look at the attachments table in any bug.
Created attachment 192763 [details] [diff] [review] patch, v2 I forgot to fix filterexceptions.pl: attachid -> attach_id. No other changes.
Comment on attachment 192763 [details] [diff] [review] patch, v2 per discussion with myk and justdave on IRC, it appears that the best fix is to use Flag objects which we call from Bug.pm and Attachment.pm. This way, there is no need to do direct call to Flag.pm from process_bug.cgi and attachment.cgi. (03:12:45) myk: justdave, LpSolit: i agree; passing objects around does seem cleaner, and a function like, say, $bug->create_flag() or $attachment->create_flag() along with $bug->flags and $attachment->flags probably makes the most sense
I won't have time to play with it before the 2.22 freeze
Target Milestone: Bugzilla 2.22 → ---
We still want this for 2.24 though, right?
Target Milestone: --- → Bugzilla 2.24
myk did a great job in bug 302699 when rewritting Attachment.pm. Now it's my turn with Flag.pm and FlagType.pm. This is going to be my bigger contribution for 2.24...
Priority: -- → P1
Summary: rewrite Attachment.pm and remove the Flag-Attachment dependency loop → update Attachment.pm, merge Flag.pm and FlagType.pm, and remove the Flag-Attachment dependency loop
This no longer blocks bug 304599. The Attachment <-> Flag dependency loop has been removed as part of bug 339750.
No longer blocks: 304599
Summary: update Attachment.pm, merge Flag.pm and FlagType.pm, and remove the Flag-Attachment dependency loop → Implement real objects in Flag.pm and FlagType.pm, and remove the Flag-Attachment dependency loop
OK, I consider this rewrite as complete. The only remaining improvement I could see is to avoid having to pass the bug and attachment objects to most Flag::* routines, but that's definitely a minor point which I won't fix till I really have a good idea on how to do that very cleanly. As most of these routines are internal, it doesn't really matter for now.
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.