post_bug.cgi and process_bug.cgi need to be combined

RESOLVED WONTFIX

Status

()

Bugzilla
Creating/Changing Bugs
--
enhancement
RESOLVED WONTFIX
16 years ago
10 years ago

People

(Reporter: Jeff Hedlund, Assigned: Ronaldo Maia)

Tracking

Dependency tree / graph

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

16 years ago
post_bug and process_bug have a lot of common code and need to be combined.
This needs to be done via Bug.pm - its the sensible way, + the way of the future!
Depends on: 171493

Updated

13 years ago
Severity: normal → enhancement
OS: other → All
Hardware: Other → All

Updated

13 years ago
Depends on: 242318
(Assignee)

Comment 2

13 years ago
Created attachment 188450 [details] [diff] [review]
clean up on post_bug.cgi
(Assignee)

Updated

13 years ago
Attachment #188450 - Flags: review?

Updated

13 years ago
Attachment #188450 - Flags: review?(LpSolit)

Comment 3

13 years ago
Essentially this patch moves the Bug-specific code from post_bug to Bug.pm. It's
pretty much a cut-n-paste job with some added methods to do updating of the Bug
object.

The cool part about this is that process_bug can use these methods and become
quite a bit simpler. This is one of the large cleanups we need to do for
Bugzilla in order to survive.

Fixing a dependency -- it was really blocks.

Fred, do you think we should split this part into a separate bug?
Assignee: myk → romaia
Blocks: 242318
No longer depends on: 242318

Comment 4

13 years ago
kiko, please leave this dependency alone!! bug 242318 is a bug we want to fix
for 2.20. Yours is targetted 2.22.

And yes, this patch should be splitted into smaller pieces... as soon as the
actual blockers and the security patches have landed (this patch will bitrot
very soon).

The right way to go is to clean up both process_bug.cgi and post_bug.cgi at the
same time, meaning that the code doing the same job must be moved at the same
time. What is done here is not efficient, as a routine which fits perfectly for
post_bug.cgi has a high probability to need some modifications to suit for
process_bug.cgi.
No longer blocks: 242318
Depends on: 242318
Target Milestone: --- → Bugzilla 2.22

Comment 5

13 years ago
A few points:

  a) The reason I changed the depenency was that it becomes very easy to fix the
bug 242318 once this bug is fixed; I'm sorry I didn't notice the milestone differed.

  b) This dependency is pretty much useless as it stands, then -- there is no
patch on the dependency, and there is one here. The fact that the change there
will bitrot this patch doesn't seem to important enough to justify a dependency
-- any other patch which touches post_bug will do this.
  
  c) I completely refuse to accept that both process_bug and post_bug need to be
done at the same time. This is plain blocking good work done -- if you say "this
can't be done incrementally" then the odds are it won't be done at all. There is
nothing that stops us from doing post_bug and then adapting the methods when
process_bug is done -- in fact, that is the way it /should/ be done.

  d) There is nothing to split this patch out into if the review process isn't
as slow as molasses -- it simply cut-n-pastes code out of post_bug.cgi, and the
swifter, and the least that changes in that process, the better it is.

The only splitting that I suggested was splitting this bug off into a "Convert
post_bug.cgi to use Bug.pm"-themed bug, which I'll ask Ronaldo to do now. I can
do the review myself if noone else makes himself available in able time.

Comment 6

13 years ago
Comment on attachment 188450 [details] [diff] [review]
clean up on post_bug.cgi


>+for my $field ($cgi->param()) {
>+    if ( scalar(@{%form_data->{$field}} = $cgi->param($field)) == 1) {
>+        %form_data->{$field} = $cgi->param($field);
>     }
> }

Great! We did deFORMication (removing %::FORM stuff) and you bring it back
here.


>+my $bug = Bugzilla::Bug::CreateBug(\%form_data, $comment);

Why has $comment a special status?


>+    &::SendSQL("SELECT user_id FROM user_group_map

Deprecated routines. Please use DBI stuff.

Moreover, the patch doesn't apply cleanly anymore, see the dependent bugs.
Attachment #188450 - Flags: review?(LpSolit)
Attachment #188450 - Flags: review?
Attachment #188450 - Flags: review-
(Assignee)

Comment 7

13 years ago
Created attachment 191243 [details] [diff] [review]
v2 - romaia
Attachment #188450 - Attachment is obsolete: true
Attachment #191243 - Flags: review?
(Assignee)

Comment 8

13 years ago
Created attachment 191524 [details] [diff] [review]
v4-romaia -  minor changes

fixing bitrot
Attachment #191243 - Attachment is obsolete: true
Attachment #191524 - Flags: review?(LpSolit)

Comment 9

13 years ago
Comment on attachment 191524 [details] [diff] [review]
v4-romaia -  minor changes

Nit:

-if (defined $cgi->param('product')) {
-    if (defined $cgi->param('version')) {
+if (defined $cgi->param('product') && defined $cgi->param('version')) {
	 $cgi->send_cookie(-name => "VERSION-$product",
			   -value => $cgi->param('version'),
			   -expires => "Fri, 01-Jan-2038 00:00:00 GMT");
-    }
 }


That needs to have the identation fixed as well.
(Assignee)

Updated

13 years ago
Attachment #191524 - Flags: review?(mkanat)

Comment 10

13 years ago
Would it be at all possible to first add those "add_" functions to Bugzilla::Bug
in another bug, and move process_bug code to use them, and then do the whole
CreateBug thing in a separate bug?

This patch is just too big to review over and over, or even once.

Also, by the summary of this bug, all this work should be happening in a
blocker, not in this bug, anyhow.

Comment 11

13 years ago
Comment on attachment 191524 [details] [diff] [review]
v4-romaia -  minor changes

Yeah, in fact, this definitely does generally need to be split into smaller
patches somehow.
Attachment #191524 - Flags: review?(mkanat) → review-

Updated

13 years ago
Attachment #191243 - Flags: review?

Updated

13 years ago
Attachment #191524 - Flags: review?(LpSolit)

Updated

13 years ago
Version: unspecified → 2.21

Updated

13 years ago
Depends on: 305136

Comment 12

13 years ago
The trunk is now frozen to prepare Bugzilla 2.22. Enhancement bugs are retargetted to 2.24.
Target Milestone: Bugzilla 2.22 → Bugzilla 2.24

Updated

12 years ago
QA Contact: mattyt-bugzilla → default-qa

Updated

12 years ago
Depends on: 122922
No longer depends on: 305136

Comment 13

12 years ago
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

Updated

12 years ago
Target Milestone: Bugzilla 3.2 → ---

Comment 14

10 years ago
Max, agree to mark this bug as WONTFIX? I see no benefit in merging post_bug.cgi and process_bug.cgi together.

Comment 15

10 years ago
Yes, agreed.
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.