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

Status: RESOLVED FIXED

(Bugzilla :: Bugzilla-General, enhancement)

Description

[Frédéric Buclin]

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.

Comment 1

[arbingersys]

Attachment: 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

Comment 2

[Frédéric Buclin]

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.

Comment 3

[arbingersys]

Yeah, sorry about that. I realized I had got that part wrong after I submitted. Now I know better. Thanks.

Comment 4

[Frédéric Buclin]

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.

Comment 5

[Frédéric Buclin]

Holding approval till the patch is updated.

Comment 6

[arbingersys]

Attachment: Diff with 'use'

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

Comment 7

[Frédéric Buclin]

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

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

Comment 8

[arbingersys]

Attachment: Diff with all files (we hope) :)

Gah. Sorry. I rushed it. How about this patch?

Comment 9

[Frédéric Buclin]

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

Yes, now it works fine. r=LpSolit

Comment 10

[Frédéric Buclin]

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