Move GroupNameToId into Bugzilla/Group.pm and eliminate GroupExists

RESOLVED FIXED in Bugzilla 3.0

Status

()

Bugzilla
Bugzilla-General
--
enhancement
RESOLVED FIXED
12 years ago
12 years ago

People

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

Tracking

2.23
Bugzilla 3.0
Dependency tree / graph
Bug Flags:
approval +

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

12 years ago
This function obviously belongs in Bugzilla::Group.

However, generally, instead of using this function we should be creating Group objects. It takes about as much time to create a Group object as it does to call this function.
(Assignee)

Comment 1

12 years ago
Also, the "GroupExists" function is used only once, and is the *exact same function* as GroupNameToId. So we just use group_name_to_id instead.
Status: NEW → ASSIGNED
Summary: Move GroupNameToId into Bugzilla/Group.pm → Move GroupNameToId into Bugzilla/Group.pm and eliminate GroupExists
(Assignee)

Comment 2

12 years ago
Created attachment 213023 [details] [diff] [review]
v1

Once again, a very simple patch. :-)
Attachment #213023 - Flags: review?(wicked)
(Assignee)

Comment 3

12 years ago
Created attachment 213024 [details] [diff] [review]
v2

I improved the POD slightly.
Attachment #213023 - Attachment is obsolete: true
Attachment #213024 - Flags: review?(wicked)
Attachment #213023 - Flags: review?(wicked)
Comment on attachment 213024 [details] [diff] [review]
v2

>Index: Bugzilla/Group.pm
>===================================================================
>+=item C<group_name_to_id($name)>

Nit: You could have also added this to the synopsis section in the POD. I wouldn't mind if the Bugzilla::get_all_groups would be corrected at the same time (should be Bugzilla::Group::get_all_groups). If you want, both can be fixed on checkin.

>+ Description: Converts a group name to an id.
>+              In general, instead of using this function, you should
>+              create a Group object and get its name. This function
>+              does not offer any real performance advantage.

Makes me think this sub shouldn't exist but the three places where this is used be converted to Group objects.
Attachment #213024 - Flags: review?(wicked) → review+
Flags: approval?
Flags: approval? → approval+
(Assignee)

Updated

12 years ago
Blocks: 329022
(Assignee)

Comment 5

12 years ago
Okay, did the checkin fix for the POD.

Checking in editgroups.cgi;
/cvsroot/mozilla/webtools/bugzilla/editgroups.cgi,v  <--  editgroups.cgi
new revision: 1.66; previous revision: 1.65
done
Checking in editproducts.cgi;
/cvsroot/mozilla/webtools/bugzilla/editproducts.cgi,v  <--  editproducts.cgi
new revision: 1.114; previous revision: 1.113
done
Checking in globals.pl;
/cvsroot/mozilla/webtools/bugzilla/globals.pl,v  <--  globals.pl
new revision: 1.353; previous revision: 1.352
done
Checking in Bugzilla/Group.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Group.pm,v  <--  Group.pm
new revision: 1.11; previous revision: 1.10
done
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
(Assignee)

Comment 6

12 years ago
Created attachment 213646 [details] [diff] [review]
v3 (checked-in version)

Okay, here's the version that I checked-in.
Attachment #213024 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.