Implement real objects in Flag.pm and FlagType.pm, and remove the Flag-Attachment dependency loop

RESOLVED FIXED in Bugzilla 3.0

Status

()

P1
enhancement
RESOLVED FIXED
14 years ago
12 years ago

People

(Reporter: LpSolit, Assigned: LpSolit)

Tracking

2.21
Bugzilla 3.0
Dependency tree / graph

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

14 years ago
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().
(Assignee)

Updated

14 years ago
Status: NEW → ASSIGNED
Target Milestone: --- → Bugzilla 2.22
(Assignee)

Comment 1

14 years ago
Created attachment 192737 [details] [diff] [review]
patch, v1

To test Attachment.pm, look at the attachments table in any bug.
Attachment #192737 - Flags: review?(mkanat)
(Assignee)

Comment 2

14 years ago
Created attachment 192763 [details] [diff] [review]
patch, v2

I forgot to fix filterexceptions.pl: attachid -> attach_id. No other changes.
Attachment #192737 - Attachment is obsolete: true
Attachment #192763 - Flags: review?(mkanat)
(Assignee)

Updated

14 years ago
Attachment #192737 - Flags: review?(mkanat)
(Assignee)

Updated

14 years ago
Blocks: 302669
(Assignee)

Updated

14 years ago
Attachment #192763 - Flags: review?(myk)
(Assignee)

Updated

14 years ago
No longer blocks: 302669
Depends on: 302669
(Assignee)

Comment 3

14 years ago
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
Attachment #192763 - Flags: review?(myk)
Attachment #192763 - Flags: review?(mkanat)
(Assignee)

Updated

14 years ago
Blocks: 286351
(Assignee)

Comment 4

13 years ago
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?
(Assignee)

Comment 6

13 years ago
right
Target Milestone: --- → Bugzilla 2.24
(Assignee)

Comment 7

13 years ago
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
(Assignee)

Comment 8

13 years ago
(In reply to comment #7)
> myk did a great job in bug 302699 

I meant bug 302669 :(
(Assignee)

Updated

13 years ago
Depends on: 322285
(Assignee)

Updated

13 years ago
Blocks: 300549

Updated

13 years ago
Blocks: 330700
(Assignee)

Updated

13 years ago
No longer blocks: 300549
Depends on: 300549
(Assignee)

Updated

13 years ago
Depends on: 339750
(Assignee)

Comment 9

13 years ago
This no longer blocks bug 304599. The Attachment <-> Flag dependency loop has been removed as part of bug 339750.
No longer blocks: 304599
(Assignee)

Updated

13 years ago
Depends on: 343432
(Assignee)

Updated

13 years ago
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
(Assignee)

Updated

13 years ago
Depends on: 343809
(Assignee)

Updated

13 years ago
Depends on: 343810
(Assignee)

Comment 10

13 years ago
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.