From 350a9f2a710ed0413c57680321ce2fbc60b1ff53 Mon Sep 17 00:00:00 2001 From: Mike McCune Date: Wed, 18 Jul 2012 12:18:19 -0700 Subject: [PATCH] system groups - API accepts max_systems and CLI unit tests --- cli/src/katello/client/api/system_group.py | 5 +- cli/src/katello/client/core/system_group.py | 6 +- cli/test/katello/__init__.py | 2 +- .../core/system/system_group_copy_test.py | 63 +++++++++++++++++++ .../api/system_groups_controller.rb | 16 ++++- src/script/katello-reset-dbs | 3 +- .../api/system_groups_controller_spec.rb | 26 ++++++-- 7 files changed, 109 insertions(+), 12 deletions(-) create mode 100644 cli/test/katello/tests/core/system/system_group_copy_test.py diff --git a/cli/src/katello/client/api/system_group.py b/cli/src/katello/client/api/system_group.py index 326ef9dad3b..6d0059d2988 100644 --- a/cli/src/katello/client/api/system_group.py +++ b/cli/src/katello/client/api/system_group.py @@ -61,11 +61,12 @@ def create(self, org_id, name, description, max_systems): path = "/api/organizations/%s/system_groups/" % (org_id) return self.server.POST(path, data)[1] - def copy(self, org_id, system_group_id, new_name, description): + def copy(self, org_id, system_group_id, new_name, description, max_systems): data = { "system_group" : { "new_name": new_name, - "description": description + "description": description, + "max_systems": max_systems } } diff --git a/cli/src/katello/client/core/system_group.py b/cli/src/katello/client/core/system_group.py index 813d77e3598..b9ba3099688 100644 --- a/cli/src/katello/client/core/system_group.py +++ b/cli/src/katello/client/core/system_group.py @@ -112,6 +112,9 @@ def setup_parser(self, parser): help=_("name of organization (required)")) parser.add_option('--description', dest='description', help=_("system group description for new group")) + parser.add_option('--max_systems', dest='max_systems', + help=_("maximum number of systems in this group")) + def check_options(self, validator): validator.require(('name', 'org', 'new_name')) @@ -121,9 +124,10 @@ def run(self): name = self.get_option('name') new_name = self.get_option('new_name') description = self.get_option('description') + max_systems = self.get_option('max_systems') source_system_group = get_system_group(org_name, name) - new_system_group = self.api.copy(org_name, source_system_group["id"], new_name, description) + new_system_group = self.api.copy(org_name, source_system_group["id"], new_name, description, max_systems) test_record(new_system_group, _("Successfully copied system group [ %s ] to [ %s ]") % diff --git a/cli/test/katello/__init__.py b/cli/test/katello/__init__.py index 1726038b2bd..6396ad0386e 100644 --- a/cli/test/katello/__init__.py +++ b/cli/test/katello/__init__.py @@ -1,2 +1,2 @@ -__path__.append('../src/katello') \ No newline at end of file +__path__.append('../../src/katello') diff --git a/cli/test/katello/tests/core/system/system_group_copy_test.py b/cli/test/katello/tests/core/system/system_group_copy_test.py new file mode 100644 index 00000000000..9d06c3301d0 --- /dev/null +++ b/cli/test/katello/tests/core/system/system_group_copy_test.py @@ -0,0 +1,63 @@ +import unittest +import os + +from katello.tests.core.action_test_utils import CLIOptionTestCase, CLIActionTestCase +from katello.tests.core.organization import organization_data +from katello.tests.core.system import system_data + +import katello.client.core.system_group +from katello.client.core.system_group import Copy +from katello.client.api.system_group import SystemGroupAPI + +class RequiredCLIOptionsTests(CLIOptionTestCase): + #requires: name, new_name, organization + #optional: maximum systems, description + + action = Copy() + + disallowed_options = [ + (), + ('--org=ACME', ), + ('--name=system_group_1',), + ('--description="This is a system group"'), + ('--max_systems=35'), + ('--org=ACME', '--max_systems=35'), + ('--org=ACME', '-description="This is a system group"'), + ] + + allowed_options = [ + ('--name=orig_system', '--org=ACME', '--new_name=system'), + ('--name=orig_system', '--org=ACME', '--new_name=system', '--max_systems=6'), + ('--name=orig_system', '--org=ACME', '--new_name=system', '--description="This is a desc"'), + ('--name=orig_system', '--org=ACME', '--new_name=system', '--description="This is a desc"', '--max_systems=6') + ] + + +class SystemGroupCopyTest(CLIActionTestCase): + + ORG = organization_data.ORGS[0] + SYSTEM_GROUP = system_data.SYSTEM_GROUPS[1] + NEW_NAME = SYSTEM_GROUP['name'] + "_copied" + + OPTIONS = { + 'org': ORG['name'], + 'name': SYSTEM_GROUP['name'], + 'new_name': NEW_NAME, + 'description': SYSTEM_GROUP['description'], + 'max_systems' : 5 + } + + def setUp(self): + self.set_action(Copy()) + self.set_module(katello.client.core.system_group) + self.mock_printer() + + self.mock_options(self.OPTIONS) + + self.mock(self.action.api, 'copy', self.SYSTEM_GROUP) + self.mock(SystemGroupAPI, 'system_group_by_name', self.SYSTEM_GROUP) + + def test_it_calls_system_group_copy_api(self): + self.run_action() + self.action.api.copy.assert_called_once_with(self.OPTIONS['org'], self.SYSTEM_GROUP['id'], self.NEW_NAME, + self.OPTIONS['description'], self.OPTIONS['max_systems']) diff --git a/src/app/controllers/api/system_groups_controller.rb b/src/app/controllers/api/system_groups_controller.rb index 9168eb2e533..3cb78166bd9 100644 --- a/src/app/controllers/api/system_groups_controller.rb +++ b/src/app/controllers/api/system_groups_controller.rb @@ -42,7 +42,7 @@ def rules def param_rules { :create => {:system_group=>[:name, :description, :system_ids, :max_systems]}, - :copy => {:system_group=>[:new_name, :description]}, + :copy => {:system_group=>[:new_name, :description, :max_systems]}, :update => {:system_group=>[:name, :description, :system_ids, :max_systems]}, :add_systems => {:system_group=>[:system_ids]}, :remove_systems => {:system_group=>[:system_ids]} @@ -119,9 +119,19 @@ def copy grp_param = params[:system_group] new_group = SystemGroup.new new_group.name = grp_param[:new_name] - new_group.description = grp_param[:description] new_group.organization = @group.organization - new_group.max_systems = @group.max_systems + + # Check API params and if not set use the existing group + if grp_param[:description] + new_group.description = grp_param[:description] + else + new_group.description = @group.description + end + if grp_param[:max_systems] + new_group.max_systems = grp_param[:max_systems] + else + new_group.max_systems = @group.max_systems + end new_group.save! new_group.systems = @group.systems diff --git a/src/script/katello-reset-dbs b/src/script/katello-reset-dbs index f1bb9a8a566..65f50b73f61 100755 --- a/src/script/katello-reset-dbs +++ b/src/script/katello-reset-dbs @@ -129,7 +129,8 @@ fi echo "Initializing Candlepin database" sudo /usr/share/candlepin/cpdb --drop --create -sudo /usr/share/candlepin/cpsetup -s +sudo /usr/share/candlepin/cpsetup -s -k `cat /etc/katello/keystore_password-file` + # cpsetup alters tomcat configuration, use the original sudo cp /etc/tomcat6/server.xml.original /etc/tomcat6/server.xml diff --git a/src/spec/controllers/api/system_groups_controller_spec.rb b/src/spec/controllers/api/system_groups_controller_spec.rb index ecf33a2c8ec..9f3a145cfee 100644 --- a/src/spec/controllers/api/system_groups_controller_spec.rb +++ b/src/spec/controllers/api/system_groups_controller_spec.rb @@ -164,18 +164,36 @@ it_should_behave_like "protected action" it "should create a group correctly" do - post :copy, :organization_id=>@org.cp_key, :id=>@group.id, :system_group=>{:new_name=>"foo", :description=>"describe"} + post :copy, :organization_id=>@org.cp_key, :id=>@group.id, :system_group=>{:new_name=>"foo", :description=>"describe", :max_systems=>1234} response.should be_success - SystemGroup.where(:name=>"foo").first.should_not be_nil + first = SystemGroup.where(:name=>"foo").first + first.should_not be_nil + first[:max_systems].should == 1234 end - + + + it "should not create 2 groups with the same name" do post :copy, :organization_id=>@org.cp_key, :id=>@group.id, :system_group=>{:new_name=>"foo2", :description=>"describe"} post :copy, :organization_id=>@org.cp_key, :id=>@group.id, :system_group=>{:new_name=>"foo2", :description=>"describe"} response.should_not be_success SystemGroup.where(:name=>"foo2").count.should == 1 end - + + it "should inherit fields from existing group unless specified in API call" do + post :copy, :organization_id=>@org.cp_key, :id=>@group.id, :system_group=>{:new_name=>"foo", :description=>"describe new", :max_systems=>1234} + response.should be_success + first = SystemGroup.where(:name=>"foo").first + first[:max_systems].should == 1234 + first[:description].should == "describe new" + + post :copy, :organization_id=>@org.cp_key, :id=>@group.id, :system_group=>{:new_name=>"foo2"} + response.should be_success + first = SystemGroup.where(:name=>"foo2").first + first[:max_systems].should == @group.max_systems + first[:description].should == @group.description + end + it "should not let you copy one group to a different org" do @org2 = Organization.create!(:name => 'test_org2', :cp_key => 'test_org2') post :copy, :organization_id=>@org2.cp_key, :id=>@group.id, :system_group=>{:new_name=>"foo2", :description=>"describe"}