556422 - (bz-oldbugmove) The existing bug-moving functionality should become an extension

Status: RESOLVED FIXED

(Bugzilla :: Bugzilla-General, enhancement)

Description

[Max Kanat-Alexander]

I don't know how many, if any, installations are using the bug-moving functionality. It should become an extension.

importxml.pl can stay in the main code, because it has other uses than just importing moved bugs.

Comment 1

[Max Kanat-Alexander]

Attachment: Work In Progress

Comment 2

[Max Kanat-Alexander]

Attachment: v1

Okay, here we go. This doesn't work quite yet, because other code is required.

  I removed several parameters and replaced them with better default behavior.

  I also removed the move-multiple-bugs functionality, because, according to some folks on the support list, importxml hasn't been able to import multiple bugs for quite some time, and it made it much simpler to make this into an extension.

  I couldn't test all the changes to importxml.pl, because apparently importxml.pl isn't working at all on trunk for reading bug-move emails (maybe it hasn't been working for some time). I guess that shows you how many people use bug moving....

Comment 3

[Max Kanat-Alexander]

Attachment: v2 (bundle)

Okay, provided that the dependency chain is satisfied, this one works! :-)

Comment 4

[Max Kanat-Alexander]

Attachment: v3 (bundle)

The v2 bundle wasn't applying on other machines than landfill. This one should work.

Comment 5

[David Lawrence [:dkl]]

Some initial thoughts:

1. Why call it OldBugMove as opposed to just BugMove. BugMove still sound slike it could be useful for those that need it.

2. When moving a bug, it adds a blank comment which is confusing to the move bug which is confusing. The comment is being added in object_end_of_set() where it is setting type => CMT_MOVED_TO. I see where format_comment-type.html.tmpl is supposed to be adding the text but it is not working for me.

3. When trying to delete the MOVED resolution in editvalues.cgi, instead of throwing a proper error it instead dies with:

Undefined subroutine &Bugzilla::Extension::OldBugMove::ThrowUserError called at ./extensions/OldBugMove/Extension.pm line 78.

extensions/OldBugMove/Extension.pm needs to import Bugzilla::Error.

4. Should the [Move bug to...] button still be visible when a bug has already been moved and set to RESOLVED/MOVED.

5. The product and component config for GetOptions in importxml.pl needs to
be "product=s" and "component=s" respectively.
 
6. importxml.pl when importing a bug that has a product that does not exist, without specifying a default product, instead of displaying the descriptive error you wrote it dies instead with:

Bad argument param sent to Bugzilla::Product::new function.
 at ./importxml.pl line 1301

Similar with a default product but no component specified, it dies with:

Bad argument name sent to Bugzilla::Component::new function.
 at ./importxml.pl line 1301

When specifying a default product and default component, it dies with the error: 

Bad argument name sent to Bugzilla::Component::new function.
 at ./importxml.pl line 1301

-        $component = new Bugzilla::Component({
-           component => $default_component_name, product => $product });
+        $component = new Bugzilla::Component({
+           name => $default_component_name, product => $product });

7. The pod documentation in importxml.pl needs to be updated so that importxml.pl --help shows the new default product and component options.

8. Due to Bugzilla::Error not being in Extension.pm, when manually changing a bug to RESOLVED/MOVED without clicking the [Move bug to...] button, it dies with the following error:

Undefined subroutine &Bugzilla::Extension::OldBugMove::ThrowCodeError called at ./extensions/OldBugMove/Extension.pm line 137.

Dave

Comment 6

[Max Kanat-Alexander]

(In reply to comment #5)
> 1. Why call it OldBugMove as opposed to just BugMove. BugMove still sound slike
> it could be useful for those that need it.

  Because there will be another solution for moving bugs in the future, using RPC.

  Everything else, I will fix! :-)

Comment 7

[Max Kanat-Alexander]

Attachment: v4 (bundle)

Okay, this fixes all the issues that you found. Thank you for finding all of those! :-)

I fixed the importxml.pl POD in general so that it's actual normal pod2usage style, now.

Comment 8

[David Lawrence [:dkl]]

Ok, all of my suggestions have been addressed except scenario #8 from comment 5 is doing something different. When I try to do the manual move by changing a bug to RESOLVED/MOVED without clicking the [Move bug to ...] button, I get the following internal error:

Bugzilla has suffered an internal error. Please save this page and send it to [e-mail] with details of what you were doing at the time this message appeared.

URL: http://10.211.55.113/bugzilla/process_bug.cgi
You cannot set the resolution of a bug to MOVED without moving the bug.

Traceback:

 at ./extensions/OldBugMove/Extension.pm line 138
	Bugzilla::Extension::OldBugMove::_check_bug_resolution(...) called at ./extensions/OldBugMove/Extension.pm line 127
	Bugzilla::Extension::OldBugMove::__ANON__(...) called at Bugzilla/Object.pm line 302
	Bugzilla::Object::set(...) called at Bugzilla/Bug.pm line 2322
	Bugzilla::Bug::set_resolution(...) called at Bugzilla/Bug.pm line 2385
	Bugzilla::Bug::set_bug_status(...) called at Bugzilla/Object.pm line 329
	Bugzilla::Object::set_all(...) called at Bugzilla/Bug.pm line 1980
	Bugzilla::Bug::set_all(...) called at /var/www/html/bugzilla/process_bug.cgi line 380

Comment 9

[Max Kanat-Alexander]

(In reply to comment #8)
> Ok, all of my suggestions have been addressed except scenario #8 from comment 5
> is doing something different. When I try to do the manual move by changing a
> bug to RESOLVED/MOVED without clicking the [Move bug to ...] button, I get the
> following internal error:

  That's actually the correct behavior--it's a CodeError. I suppose a UserError would make more sense, though. I'll change it.

Comment 10

[Max Kanat-Alexander]

Attachment: v5 (bundle)

Okay, here's the patch with that error changed to a UserError.

Comment 11

[David Lawrence [:dkl]]

Comment on attachment 452095 [details] [diff] [review]
v5 (bundle)

Looks good now. Testing all works good for me. r=dkl

Comment 12

[Max Kanat-Alexander]

  Awesome, thanks for reviewing that huge patch, dkl!! :-)


Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/trunk/
modified importxml.pl
modified process_bug.cgi
modified Bugzilla/Bug.pm
modified Bugzilla/Comment.pm
modified Bugzilla/Constants.pm
modified Bugzilla/DB.pm
modified Bugzilla/User.pm
modified Bugzilla/Field/ChoiceInterface.pm
added extensions/OldBugMove
added extensions/OldBugMove/Config.pm
added extensions/OldBugMove/Extension.pm
added extensions/OldBugMove/disabled
added extensions/OldBugMove/lib
added extensions/OldBugMove/template
renamed Bugzilla/Config/BugMove.pm => extensions/OldBugMove/lib/Params.pm
added extensions/OldBugMove/template/en
added extensions/OldBugMove/template/en/default
added extensions/OldBugMove/template/en/default/admin
added extensions/OldBugMove/template/en/default/hook
added extensions/OldBugMove/template/en/default/admin/params
renamed template/en/default/admin/params/bugmove.html.tmpl => extensions/OldBugMove/template/en/default/admin/params/oldbugmove.html.tmpl
added extensions/OldBugMove/template/en/default/hook/bug
added extensions/OldBugMove/template/en/default/hook/global
added extensions/OldBugMove/template/en/default/hook/bug/edit-after_comment_textarea.html.tmpl
added extensions/OldBugMove/template/en/default/hook/bug/format_comment-type.txt.tmpl
added extensions/OldBugMove/template/en/default/hook/global/user-error-auth_failure_action.html.tmpl
added extensions/OldBugMove/template/en/default/hook/global/user-error-errors.html.tmpl
modified template/en/default/bug/edit.html.tmpl
modified template/en/default/bug/format_comment.txt.tmpl
modified template/en/default/global/code-error.html.tmpl
modified template/en/default/global/field-descs.none.tmpl
modified template/en/default/global/user-error.html.tmpl                       
modified template/en/default/list/edit-multiple.html.tmpl
modified template/en/default/pages/fields.html.tmpl
Committed revision 7217.

Comment 13

[Max Kanat-Alexander]

Added to the release notes in bug 604256.

Comment 14

[Frédéric Buclin]

$user->is_mover no longer exists, and so must be removed from the POD.

Committing to: bzr+ssh://[e-mail]/bugzilla/trunk/
modified Bugzilla/User.pm
Committed revision 7973.

Committing to: bzr+ssh://[e-mail]/bugzilla/4.2/
modified Bugzilla/User.pm
Committed revision 7939.

Committing to: bzr+ssh://[e-mail]/bugzilla/4.0/
modified Bugzilla/User.pm
Committed revision 7650.