Removed exception based flows for nss/host and seperated the logic to…

… add a group and add a host group for a hostmachine

app/controllers/application_controller.rb

@@ -17,10 +17,12 @@ def render_404
 
   def authenticate_access_token!
     unless AccessToken.valid_token(params[:token])
-      render json: {
-        success: false,
-        errors: ['Unauthorized']
-      }, status: :unauthorized
+      render_error(['Unauthorized'], :unauthorized)
     end
   end
+
+  def render_error(errors, status = 400)
+    render 'common/errors', locals: { errors: errors }, status: status
+    return
+  end
 end

app/controllers/nss_controller.rb

@@ -23,20 +23,17 @@ def host
   end
 
   def add_host
-    begin
+    if params[:name].present?
       host = HostMachine.find_or_create_by(name: params[:name])
-      host.add_groups(params[:name], params[:group_name])
-      render json: {
-        success: true,
-        access_key: host.access_key,
-        host: host.name,
-        groups: host.groups.map(&:name)
-      }
-    rescue ActiveRecord::RecordInvalid
-      render json: {
-        success: false,
-        errors: host.errors
-      }
+      host.add_host_group(params[:name])
+      host.add_group(params[:group_name])
+      render 'add_host', locals: { host: host }, format: :json
+    else
+      errors = ['Name can\'t be blank']
+      if params.key?(:group_name) && params[:group_name].blank?
+        errors << 'Group Name can\'t be blank'
+      end
+      render_error(errors)
     end
   end
 

app/models/host_machine.rb

@@ -35,10 +35,17 @@ def sysadmins
     users.uniq
   end
 
-  def add_groups(name, group_name)
-    host_group_name = name.blank? ? '' : "#{name}_host_group".downcase.squish
-    self.groups <<  Group.find_or_create_by(name: host_group_name)
-    self.groups <<  Group.find_or_create_by(name: group_name)
-    self.save
+  def add_host_group(name)
+    if name.squish.present?
+      name = "#{name.squish}_host_group"
+      self.add_group(name.squish.downcase)
+    end
+  end
+
+  def add_group(name)
+    if name.squish.present?
+      self.groups << Group.find_or_initialize_by(name: name.squish.downcase)
+      self.save
+    end
   end
 end

app/views/common/errors.json.jbuilder

@@ -0,0 +1,2 @@
+json.success false
+json.errors errors

app/views/nss/add_host.json.jbuilder

@@ -0,0 +1,4 @@
+json.success true
+json.access_key host.access_key
+json.host host.name
+json.groups host.groups.map(&:name)

spec/controllers/nss_controller_spec.rb

@@ -30,6 +30,13 @@
     expect(data["success"]).to eq(true)
   end
 
+  it 'it shouldn\'t return sysadmins for invalid token' do
+    json = { token: '', name: 'random_host', group_name: '', format: :json }
+    post 'add_host', json
+    body = response.body
+    expect(JSON.parse(body)['success']).to eq(false)
+  end
+
   it "should return sysadmins for that host" do
     sign_in user
     access_token = create(:access_token)

spec/models/host_machine_spec.rb

@@ -80,29 +80,49 @@
     end
   end
 
+  context 'add_host_group' do
+    let(:host_machine) { HostMachine.find_or_create_by(name: 'machine')  }
+    it 'should create host group given valid name' do
+      host_machine.add_host_group(host_machine.name)
+      groups = host_machine.groups.map(&:name)
+      expect(groups.include?("#{host_machine.name}_host_group")).to eq(true)
+      expect(host_machine.valid?).to eq(true)
+    end
+
+    it 'should create the group with all downcase' do
+      host_machine.add_host_group(host_machine.name.upcase)
+      groups = host_machine.groups.map(&:name)
+      expect(groups.include?("#{host_machine.name.downcase}_host_group")).to eq(true)
+    end
+
+    it 'shouldn\'t add the group if the name is invalid' do
+      host_machine.add_host_group('')
+      groups = host_machine.groups.map(&:name)
+      expect(groups.include?("")).to eq(false)
+      expect(groups.include?("_host_group")).to eq(false)
+    end
+  end
+
   context 'add_group' do
     let(:host_machine) { HostMachine.find_or_create_by(name: 'machine')  }
-    it 'should create groups given valid name and group_name' do
-      name = host_machine.name; group_name = "#{name}_group"
-      host_machine.add_groups(name, group_name)
+    let(:group_name) { 'machine_group'  }
+    it 'should create host group given valid name' do
+      host_machine.add_group(group_name)
       groups = host_machine.groups.map(&:name)
-      expect(groups.include?("#{name}_host_group")).to eq(true)
       expect(groups.include?(group_name)).to eq(true)
-      expect(host_machine.errors.blank?).to eq(true)
+      expect(host_machine.valid?).to eq(true)
     end
 
-    it 'shouldn\'t create group if name is blank' do
-      name = ''; group_name = "#{host_machine.name}_group"
-      expect {
-        host_machine.add_groups(name, group_name)
-      }.to raise_error { ActiveRecord::RecordInvalid }
+    it 'should create the group with all downcase' do
+      host_machine.add_group(group_name.upcase)
+      groups = host_machine.groups.map(&:name)
+      expect(groups.include?(group_name.downcase)).to eq(true)
     end
 
-    it 'shouldn\'t create group if group name is blank' do
-      name = host_machine.name; group_name = ''
-      expect {
-        host_machine.add_groups(name, group_name)
-      }.to raise_error { ActiveRecord::RecordInvalid }
+    it 'shouldn\'t add the group if the name is invalid' do
+      host_machine.add_group('')
+      groups = host_machine.groups.map(&:name)
+      expect(groups.include?("")).to eq(false)
     end
   end
 end

spec/rails_helper.rb

@@ -35,6 +35,7 @@
   config.include Devise::Test::ControllerHelpers, type: :controller
   config.include Devise::Test::ControllerHelpers, type: :view
   config.include ControllerHelpers, type: :controller
+  config.render_views = true
   # Remove this line if you're not using ActiveRecord or ActiveRecord fixtures
   config.fixture_path = "#{::Rails.root}/spec/fixtures"