Merge pull request #183 from alces-flight/teams/credit-deposits

Teams - credit deposits

app/controllers/credit_deposits_controller.rb

@@ -1,32 +1,41 @@
 class CreditDepositsController < ApplicationController
   def new
-    @user = User.find(params[:id])
-    @credit_deposit = CreditDeposit.new(user: @user)
+    @team = Team.find(params[:team_id])
+    @credit_deposit = CreditDeposit.new(team: @team)
     authorize! :create, @credit_deposit
     @cloud_service_config = CloudServiceConfig.first
-    check_config_and_external_ids
+    if @cloud_service_config.nil?
+      flash[:alert] = "Unable to add credits: cloud environment config not set"
+      redirect_to teams_path
+    elsif !@credit_deposit.valid?
+      flash[:alert] = "Unable to add credits: #{@credit_deposit.errors.full_messages.join("; ")}"
+      redirect_to teams_path
+    end
   end
 
   def create
-    @user = User.find(params[:id])
+    @team = Team.find(params[:team_id])
     @cloud_service_config = CloudServiceConfig.first
-    @credit_deposit = CreditDeposit.new(user: @user, amount: credit_deposit_params[:amount])
+    @credit_deposit = CreditDeposit.new(team: @team, amount: credit_deposit_params[:amount])
     authorize! :create, @credit_deposit
 
-    if check_config_and_external_ids
-      unless @credit_deposit.valid?
-        render action: :new
-        return
-      end
+    if @cloud_service_config.nil?
+      flash[:alert] = "Unable to add credits: cloud environment config not set"
+      redirect_to teams_path
+      return
+    elsif !@credit_deposit.valid?
+      flash.now[:alert] = "Unable to add credits: #{@credit_deposit.errors.full_messages.join("; ")}"
+      render :new
+      return
+    end
 
-      result = CreateCreditDepositJob.perform_now(@credit_deposit, @cloud_service_config, @user)
-      if result.success?
-        flash[:success] = "Credit deposit submitted for #{@user.name}. It may take a few minutes for the user's new balance to be reflected."
-        redirect_to users_path
-      else
-        flash.now[:alert] = "Unable to submit credit deposit: #{result.error_message}"
-        render :new
-      end
+    result = CreateCreditDepositJob.perform_now(@credit_deposit, @cloud_service_config)
+    if result.success?
+      flash[:success] = "Credit deposit submitted for #{@team.name}. It may take a few minutes for the team's new balance to be reflected."
+      redirect_to teams_path
+    else
+      flash.now[:alert] = "Unable to submit credit deposit: #{result.error_message}"
+      render :new
     end
   end
 
@@ -36,27 +45,4 @@ def create
   def credit_deposit_params
     params.require(:credit_deposit).permit(*PERMITTED_PARAMS)
   end
-
-  def check_config_and_external_ids
-    redirect = false
-    if @cloud_service_config.nil?
-      flash[:alert] = "Unable to add credits: cloud environment config not set"
-      redirect = true
-    elsif @user.project_id.nil?
-      flash[:alert] = "Unable to add credits: user does not yet have a project id. " \
-                      "This should be added automatically shortly."
-      redirect = true
-    elsif @user.billing_acct_id.nil?
-      flash[:alert] = "Unable to add credits: user does not yet have a billing account id. " \
-                      "This should be added automatically shortly."
-      redirect = true
-    end
-
-    if redirect
-      redirect_to users_path
-      false
-    else
-      true
-    end
-  end
 end

app/jobs/create_credit_deposit_job.rb

@@ -3,10 +3,9 @@
 class CreateCreditDepositJob < ApplicationJob
   queue_as :default
 
-  def perform(credit_deposit, cloud_service_config, user, **options)
+  def perform(credit_deposit, cloud_service_config, **options)
     runner = Runner.new(
       credit_deposit: credit_deposit,
-      user: user,
       cloud_service_config: cloud_service_config,
       logger: logger,
       **options
@@ -31,9 +30,8 @@ def error_message
   end
 
   class Runner < HttpRequests::Faraday::JobRunner
-    def initialize(credit_deposit:, user:, **kwargs)
+    def initialize(credit_deposit:, **kwargs)
       @credit_deposit = credit_deposit
-      @user = user
       super(**kwargs)
     end
 

app/models/ability.rb

@@ -31,12 +31,6 @@ def root_abilities
 
     # Don't allow any admin users to be deleted.
     cannot :destroy, User, root: true
-
-    # Don't allow admins to receive credits
-    cannot :create, CreditDeposit do |deposit|
-      user = deposit.user
-      user.root || user.project_id.nil? || user.billing_acct_id.nil?
-    end
   end
 
   # Abilities for non-root users.

app/models/credit_deposit.rb

@@ -11,41 +11,46 @@ class CreditDeposit
             presence: true,
             numericality: { greater_than: 0 }
 
-  validates :user,
+  validates :team,
             presence: true
 
-  validate :user_not_root
-
+  validate :team_has_project_id
+  validate :team_has_billing_account
 
   ####################################
   #
   # Attributes
   #
   ####################################
 
-  attr_accessor :amount, :user
-  delegate :billing_acct_id, to: :user
+  attr_accessor :amount, :team
+  delegate :billing_acct_id, to: :team
 
   ############################
   #
   # Public Instance Methods
   #
   ############################
 
-  def initialize(user:, amount: 1)
-    @user = user
+  def initialize(team:, amount: 1)
+    @team = team
     @amount = amount
   end
 
-  ######################################
+  ############################
   #
   # Private Instance Methods
   #
-  ######################################
+  ############################
 
   private
 
-  def user_not_root
-    errors.add(:user, "cannot be an admin") if user&.root
+  def team_has_project_id
+    errors.add(:team, "must have a project id") if team && !team.project_id
   end
+
+  def team_has_billing_account
+    errors.add(:team, "must have a billing account id") if team && !team.billing_acct_id
+  end
+
 end

app/views/credit_deposits/new.html.erb

@@ -1,10 +1,10 @@
 <% set_title 'Add credits' -%>
 
-<p>Add credits for user <strong><%= @user.name %></strong>.</p>
+<p>Add credits for team <strong><%= @team.name %></strong>.</p>
 
-<p>They currently have <strong><%= pluralize(presenter_for(@user).formatted_credits, 'credit') %>.</strong></p>
+<p>It currently has <strong><%= pluralize(presenter_for(@team).formatted_credits, 'credit') %>.</strong></p>
 
-<%= simple_form_for @credit_deposit, html: { class: 'no_border' } do |f| %>
+<%= simple_form_for @credit_deposit, html: { class: 'no_border' }, url: team_credit_deposits_path(@team) do |f| %>
   <%= f.input :amount, as: :float, input_html: { min: 0.01, step: 0.01 } %>
   <%= f.button :submit %>
 <% end %>

app/views/teams/index.html.erb

@@ -36,6 +36,7 @@
     <% actions.add_with_auth can: :edit, on: team, title: 'Edit', path: edit_team_path(team) %>
     <% actions.add_with_auth can: :manage, on: TeamRole.new(team_id: team.id), title: 'Manage Users', path: team_team_roles_path(team) %>
     <% actions.add_with_auth can: :read, on: Invoice.new(account: team), title: 'View Invoices', path: team_invoices_path(team) %>
+    <% actions.add_with_auth can: :create, on: CreditDeposit.new(team: team), title: 'Add Credits', path: new_team_credit_deposit_path(team) %>
     <%
       actions.add_with_auth(can: :destroy, on: team,
                             title: 'Delete',

config/routes.rb

@@ -41,15 +41,7 @@
     end
   end
 
-  resources :users, only: [:index, :edit, :update, :destroy] do
-    member do
-      # A placeholder action for developing the resource table used on the
-      # users/index page.  This should be removed once we have real actions to
-      # go in the actions dropdown.
-      get :placeholder
-      resources :credit_deposits, only: [:new, :create]
-    end
-  end
+  resources :users, only: [:index, :edit, :update, :destroy]
 
   resource :settings, only: [:edit, :update]
 
@@ -60,6 +52,7 @@
         get 'draft'
       end
     end
+    resources :credit_deposits, only: [:new, :create]
   end
 
   resources :team_roles, only: [:edit, :update, :destroy]

spec/factories/credit_deposit.rb

@@ -1,7 +1,7 @@
 FactoryBot.define do
   factory :credit_deposit, class: 'CreditDeposit' do
     amount { rand(1..10) }
-    user { create(:user, :with_openstack_account) }
+    team { create(:team, :with_openstack_details) }
   end
 
   initialize_with { new(**attributes) }

spec/jobs/create_credit_deposit_job_spec.rb

@@ -3,10 +3,10 @@
 RSpec.describe CreateCreditDepositJob, type: :job do
   let(:stubs) { Faraday::Adapter::Test::Stubs.new }
   let(:cloud_service_config) { create(:cloud_service_config) }
-  let(:user) { create(:user, :with_openstack_account) }
+  let(:team) { create(:team, :with_openstack_details) }
   let(:path) { "#{cloud_service_config.user_handler_base_url}/add_credits" }
-  let(:credit_deposit) { build(:credit_deposit, user: user) }
-  subject { CreateCreditDepositJob::Runner.new(credit_deposit: credit_deposit, cloud_service_config: cloud_service_config, user: user) }
+  let(:credit_deposit) { build(:credit_deposit, team: team) }
+  subject { CreateCreditDepositJob::Runner.new(credit_deposit: credit_deposit, cloud_service_config: cloud_service_config) }
 
   describe "url" do
     before(:each) do
@@ -32,7 +32,7 @@ class << subject
       end
 
       it "returns a successful result" do
-        result = described_class.perform_now(credit_deposit, cloud_service_config, user, test_stubs: stubs)
+        result = described_class.perform_now(credit_deposit, cloud_service_config, test_stubs: stubs)
         expect(result).to be_success
       end
     end
@@ -43,12 +43,12 @@ class << subject
       end
 
       it "returns an unsuccessful result" do
-        result = described_class.perform_now(credit_deposit, cloud_service_config, user, test_stubs: stubs)
+        result = described_class.perform_now(credit_deposit, cloud_service_config, test_stubs: stubs)
         expect(result).not_to be_success
       end
 
       it "returns a sensible error_message" do
-        result = described_class.perform_now(credit_deposit, cloud_service_config, user, test_stubs: stubs)
+        result = described_class.perform_now(credit_deposit, cloud_service_config, test_stubs: stubs)
         expect(result.error_message).to eq "the server responded with status 404"
       end
     end
@@ -60,12 +60,12 @@ class << subject
       let(:timeout) { 0.1 }
 
       it "returns an unsuccessful result" do
-        result = described_class.perform_now(credit_deposit, cloud_service_config, user, test_stubs: stubs, timeout: timeout)
+        result = described_class.perform_now(credit_deposit, cloud_service_config, test_stubs: stubs, timeout: timeout)
         expect(result).not_to be_success
       end
 
       it "returns a sensible error_message" do
-        result = described_class.perform_now(credit_deposit, cloud_service_config, user, test_stubs: stubs, timeout: timeout)
+        result = described_class.perform_now(credit_deposit, cloud_service_config, test_stubs: stubs, timeout: timeout)
         expect(result.error_message).to eq "execution expired"
       end
     end

spec/models/credit_deposit_spec.rb

@@ -21,4 +21,19 @@
     subject.amount = 0
     expect(subject).to have_error(:amount, :greater_than)
   end
+
+  it "is not valid without a team" do
+    subject.team = nil
+    expect(subject).to have_error(:team, :blank)
+  end
+
+  it "is not valid if team has no project id" do
+    subject.team.project_id = nil
+    expect(subject).to have_error(:team, "must have a project id")
+  end
+
+  it "is not valid if team has no billing account id" do
+    subject.team.billing_acct_id = nil
+    expect(subject).to have_error(:team, "must have a billing account id")
+  end
 end