Multi-select fields are ignored when cloning a bug

RESOLVED FIXED in Bugzilla 3.2

Status

()

Bugzilla
Creating/Changing Bugs
RESOLVED FIXED
10 years ago
9 years ago

People

(Reporter: Frédéric Buclin, Assigned: Max Kanat-Alexander)

Tracking

3.1.4
Bugzilla 3.2
Bug Flags:
approval +
approval3.2 +
blocking3.2 +
testcase ?

Details

Attachments

(1 attachment, 1 obsolete attachment)

v2
3.41 KB, patch
Frédéric Buclin
: review+
Details | Diff | Splinter Review
(Reporter)

Description

10 years ago
The content of multi-select fields are not copied when cloning a bug, probably because enter_bug.cgi contains:

  $vars->{$field->name} = $cloned_bug->{$field->name};

As multi-select fields have no column in the bugs table but have their own table, the internal variable is empty. At the first place, I don't see why we call internal variables directly instead of using bug methods. With methods, we would have a way to get the content of multi-select fields (as an arrayref).
Flags: blocking3.2+
(Assignee)

Comment 1

10 years ago
Yeah, we should just be passing $cloned_bug to the template, or something like that.
(Assignee)

Comment 2

10 years ago
Created attachment 336008 [details] [diff] [review]
v1

This broadly fixes the fact that cloning accesses hash values directly instead of calling methods.
Assignee: create-and-change → mkanat
Status: NEW → ASSIGNED
Attachment #336008 - Flags: review?(LpSolit)
(Reporter)

Updated

10 years ago
Attachment #336008 - Flags: review?(LpSolit) → review+
(Reporter)

Comment 3

10 years ago
Comment on attachment 336008 [details] [diff] [review]
v1

I see other places in enter_bug.cgi where $cloned_bug->{internal_var} is called instead of $cloned_bug->method; specifically ->{version} and ->{groups}. Could you fix them too while you are on it? We can also do it in a separate bug if you want to. Anyway, your patch works fine. r=LpSolit
(Assignee)

Comment 4

10 years ago
Created attachment 336937 [details] [diff] [review]
v2

Ah, missed those ones, thanks!
Attachment #336008 - Attachment is obsolete: true
Attachment #336937 - Flags: review?(LpSolit)
(Reporter)

Comment 5

10 years ago
Comment on attachment 336937 [details] [diff] [review]
v2

> # We need to ensure that we respect the 'insider' status of
> # the first comment, if it has one. Either way, make a note
> # that this bug was cloned from another bug.

Nit: could you fix the indentation of this comment while you are on it? I always think the IF block is over here, but it's not the case.

Nit#2: also, at the end of this IF block, you can safely remove the two lines below as you now call $cloned_bug->groups explicitly later. I tested, and it works. r=LpSolit

  # Ensure that the groupset information is set up for later use.
      $cloned_bug->groups();
Attachment #336937 - Flags: review?(LpSolit) → review+
(Reporter)

Comment 6

10 years ago
I tested on tip only, but I doubt the cloning code changed at all between 3.2 and tip.
Flags: approval3.2+
Flags: approval+
(Assignee)

Comment 7

10 years ago
tip:

Checking in enter_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/enter_bug.cgi,v  <--  enter_bug.cgi
new revision: 1.163; previous revision: 1.162
done

3.2:

Checking in enter_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/enter_bug.cgi,v  <--  enter_bug.cgi
new revision: 1.160.2.2; previous revision: 1.160.2.1
done
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
(Reporter)

Updated

9 years ago
Flags: testcase?
You need to log in before you can comment on or make changes to this bug.