Move flatten_group_membership() from User.pm to Group.pm

RESOLVED FIXED in Bugzilla 3.4

Status

()

Bugzilla
Bugzilla-General
--
enhancement
RESOLVED FIXED
11 years ago
8 years ago

People

(Reporter: Frédéric Buclin, Assigned: arbingersys)

Tracking

3.1.2
Bugzilla 3.4
Bug Flags:
approval +

Details

Attachments

(1 attachment, 2 obsolete attachments)

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

Description

11 years ago
flatten_group_membership() is definitely a group subroutine only. It takes a list of group IDs as argument and returns another list of group IDs. Nothing to do with users.
(Reporter)

Updated

10 years ago
Whiteboard: [Good Intro Bug]
(Assignee)

Comment 1

10 years ago
Created attachment 357397 [details] [diff] [review]
Diff

Hi, I took a stab at moving this sub to Group.pm. I modified the following files

./editgroups.cgi
./editusers.cgi
./Bugzilla/Group.pm
./Bugzilla/Search.pm
./Bugzilla/Install.pm
./Bugzilla/User.pm

and tested via the UI and also with runtests.pl, which was an all pass.

Let me know if this works. Thanks, James
Attachment #357397 - Flags: review?(arbingersys)
(Reporter)

Comment 2

10 years ago
Comment on attachment 357397 [details] [diff] [review]
Diff

You must ask an official reviewer for review; that's what the requestee field is for. :) I will review it.
Attachment #357397 - Flags: review?(arbingersys) → review?(LpSolit)
(Assignee)

Comment 3

10 years ago
Yeah, sorry about that. I realized I had got that part wrong after I submitted. Now I know better. Thanks.
(Reporter)

Updated

10 years ago
Assignee: general → arbingersys
Status: NEW → ASSIGNED
Whiteboard: [Good Intro Bug]
Target Milestone: --- → Bugzilla 3.4
(Reporter)

Comment 4

10 years ago
Comment on attachment 357397 [details] [diff] [review]
Diff

>Index: whine.pl
>Index: Bugzilla/User.pm

Looks good, but please add |use Bugzilla::Group| to both files above as they now use a method being in Group.pm.
Attachment #357397 - Flags: review?(LpSolit) → review+
(Reporter)

Comment 5

10 years ago
Holding approval till the patch is updated.
Flags: approval?
(Assignee)

Comment 6

10 years ago
Created attachment 358682 [details] [diff] [review]
Diff with 'use'

Okay, I think that should do it. :) Thanks, James
(Reporter)

Updated

10 years ago
Attachment #358682 - Flags: review-
(Reporter)

Comment 7

10 years ago
Comment on attachment 358682 [details] [diff] [review]
Diff with 'use'

This patch is incomplete. You forgot at least Bugzilla/Group.pm in the diff.
(Reporter)

Updated

10 years ago
Flags: approval?
(Assignee)

Comment 8

10 years ago
Created attachment 358726 [details] [diff] [review]
Diff with all files (we hope) :)

Gah. Sorry. I rushed it. How about this patch?
(Assignee)

Updated

10 years ago
Attachment #358726 - Flags: review?(LpSolit)
(Reporter)

Comment 9

10 years ago
Comment on attachment 358726 [details] [diff] [review]
Diff with all files (we hope) :)

Yes, now it works fine. r=LpSolit
Attachment #358726 - Flags: review?(LpSolit) → review+
(Reporter)

Updated

10 years ago
Flags: approval+
(Reporter)

Comment 10

10 years ago
Checking in editgroups.cgi;
/cvsroot/mozilla/webtools/bugzilla/editgroups.cgi,v  <--  editgroups.cgi
new revision: 1.90; previous revision: 1.89
done
Checking in editusers.cgi;
/cvsroot/mozilla/webtools/bugzilla/editusers.cgi,v  <--  editusers.cgi
new revision: 1.153; previous revision: 1.152
done
Checking in whine.pl;
/cvsroot/mozilla/webtools/bugzilla/whine.pl,v  <--  whine.pl
new revision: 1.38; previous revision: 1.37
done
Checking in Bugzilla/Group.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Group.pm,v  <--  Group.pm
new revision: 1.24; previous revision: 1.23
done
Checking in Bugzilla/Install.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Install.pm,v  <--  Install.pm
new revision: 1.19; previous revision: 1.18
done
Checking in Bugzilla/Search.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Search.pm,v  <--  Search.pm
new revision: 1.171; previous revision: 1.170
done
Checking in Bugzilla/User.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/User.pm,v  <--  User.pm
new revision: 1.178; previous revision: 1.177
done
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
(Reporter)

Updated

8 years ago
Attachment #357397 - Attachment is obsolete: true
(Reporter)

Updated

8 years ago
Attachment #358682 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.