Bug 556422 (bz-oldbugmove)

The existing bug-moving functionality should become an extension

RESOLVED FIXED in Bugzilla 4.0

Status

()

Bugzilla
Bugzilla-General
--
enhancement
RESOLVED FIXED
7 years ago
5 years ago

People

(Reporter: Max Kanat-Alexander, Assigned: Max Kanat-Alexander)

Tracking

Bugzilla 4.0
Dependency tree / graph
Bug Flags:
approval +

Details

Attachments

(1 attachment, 5 obsolete attachments)

(Assignee)

Description

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

Updated

7 years ago
Depends on: 556407
(Assignee)

Updated

7 years ago
No longer depends on: 556407
(Assignee)

Updated

7 years ago
Depends on: 556695
(Assignee)

Updated

7 years ago
Depends on: 539865
(Assignee)

Comment 1

7 years ago
Created attachment 436771 [details] [diff] [review]
Work In Progress
Assignee: general → mkanat
Status: NEW → ASSIGNED
(Assignee)

Comment 2

7 years ago
Created attachment 436796 [details] [diff] [review]
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....
Attachment #436771 - Attachment is obsolete: true
Attachment #436796 - Flags: review?(LpSolit)
(Assignee)

Updated

7 years ago
Depends on: 556901
(Assignee)

Updated

7 years ago
Blocks: 418342
(Assignee)

Updated

7 years ago
Alias: bz-oldbugmove
(Assignee)

Updated

7 years ago
Depends on: 567846
(Assignee)

Comment 3

7 years ago
Created attachment 447209 [details] [diff] [review]
v2 (bundle)

  Okay, provided that the dependency chain is satisfied, this one works! :-)
Attachment #436796 - Attachment is obsolete: true
Attachment #447209 - Flags: review?(dkl)
Attachment #436796 - Flags: review?(LpSolit)
(Assignee)

Comment 4

7 years ago
Created attachment 450750 [details] [diff] [review]
v3 (bundle)

The v2 bundle wasn't applying on other machines than landfill. This one should work.
Attachment #447209 - Attachment is obsolete: true
Attachment #450750 - Flags: review?(dkl)
Attachment #447209 - Flags: review?(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

Updated

7 years ago
Attachment #450750 - Flags: review?(dkl) → review-
(Assignee)

Comment 6

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

Comment 7

7 years ago
Created attachment 451830 [details] [diff] [review]
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.
Attachment #450750 - Attachment is obsolete: true
Attachment #451830 - Flags: review?(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 dkl@redhat.com 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
(Assignee)

Comment 9

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

Comment 10

7 years ago
Created attachment 452095 [details] [diff] [review]
v5 (bundle)

Okay, here's the patch with that error changed to a UserError.
Attachment #451830 - Attachment is obsolete: true
Attachment #452095 - Flags: review?(dkl)
Attachment #451830 - Flags: review?(dkl)
Comment on attachment 452095 [details] [diff] [review]
v5 (bundle)

Looks good now. Testing all works good for me. r=dkl
Attachment #452095 - Flags: review?(dkl) → review+

Updated

7 years ago
Flags: approval?
(Assignee)

Comment 12

7 years ago
  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.
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Flags: approval? → approval+
Resolution: --- → FIXED

Updated

7 years ago
Blocks: 581690

Updated

7 years ago
Blocks: 581693
(Assignee)

Updated

7 years ago
Keywords: relnote
(Assignee)

Comment 13

7 years ago
Added to the release notes in bug 604256.
Keywords: relnote

Comment 14

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

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/trunk/
modified Bugzilla/User.pm
Committed revision 7973.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/4.2/
modified Bugzilla/User.pm
Committed revision 7939.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/4.0/
modified Bugzilla/User.pm
Committed revision 7650.

Updated

5 years ago
Blocks: 757935
You need to log in before you can comment on or make changes to this bug.