Fixes based on PR review + audit log

app/controllers/api/admin/boxes_controller.rb

@@ -1,13 +1,19 @@
 class Api::Admin::BoxesController < ActionController::Base
+  include AuditableApiEvents
   before_action :set_tenant
   rescue_from ActiveRecord::RecordNotFound, with: :not_found
 
   def create
-    @box = @tenant.boxes.new(box_params)
-    #    authorize([:admin, @box])
+    begin
+      @box = @tenant.boxes.new(box_params)
+    rescue ActionController::ParameterMissing, ActiveRecord::NotFound => e
+      @exception = e
+    end
+
     return if @box.save
 
-    render json: { message: @box.errors.full_messages[0] }, status: :unprocessable_entity
+    render :error, status: :unprocessable_entity
+    log_api_call(:create_tenant_box_api_called)
   end
 
   private
@@ -19,8 +25,4 @@ def set_tenant
   def box_params
     params.require(:box).permit(:id, :name, :short_name, :uri, :color, :api_connection_id)
   end
-
-  def not_found
-    render json: { message: 'not found' }, status: :not_found
-  end
 end

app/controllers/api/admin/tenants_controller.rb

@@ -1,26 +1,23 @@
 class Api::Admin::TenantsController < ActionController::Base
+  include AuditableApiEvents
   before_action :set_tenant, only: %i[destroy]
   rescue_from ActiveRecord::RecordNotFound, with: :not_found
 
   def create
-    #    authorize([:admin, Tenant])
-    @tenant = Tenant.create(tenant_params)
-    @admin = User.create(admin_params.merge(tenant_id: @tenant.id)) if @tenant
-    @group_membership = @tenant.admin_group.users.push(@admin) if @tenant && @admin
-    return if @tenant && @admin && @group_membership
-
-    render json: {
-      message: @tenant.errors.full_messages[0] ||
-               @admin.errors.full_messages[0] ||
-               @group_membership.errors.full_messages[0]
-    }, status: :unprocessable_entity
+    begin
+      @tenant, @admin, @group_membership = Tenant.create_with_admin(tenant_params)
+    rescue ActionController::ParameterMissing => e
+      @error = e
+    end
+    render :error, status: :unprocessable_entity unless @group_membership
+    log_api_call(:create_tenant_api_called)
   end
 
   def destroy
-    # authorize([:admin, @tenant])
     return if @tenant.destroy
 
     render json: { message: @tenant.errors.full_messages[0] }, status: :unprocessable_entity
+    log_api_call(:create_tenant_api_called)
   end
 
   private
@@ -30,11 +27,7 @@ def set_tenant
   end
 
   def tenant_params
-    params.require(:tenant).permit(:name)
-  end
-
-  def admin_params
-    params.require(:admin).permit(:name, :email)
+    params.require(:tenant).permit(:name, { admin: [:name, :email] })
   end
 
   def not_found

app/controllers/concerns/auditable_api_events.rb

@@ -0,0 +1,5 @@
+module AuditableApiEvents
+  def log_api_call(action)
+    EventBus.publish(action, request, response)
+  end
+end

app/lib/event_bus.rb

@@ -35,9 +35,7 @@ def self.reset!
   Searchable::ReindexMessageThreadJob.perform_later(message.message_thread_id)
 }
 EventBus.subscribe :message_changed, ->(message) {
-  if Searchable::Indexer.message_searchable_fields_changed?(message)
-    Searchable::ReindexMessageThreadJob.perform_later(message.message_thread_id)
-  end
+  Searchable::ReindexMessageThreadJob.perform_later(message.message_thread_id) if Searchable::Indexer.message_searchable_fields_changed?(message)
 }
 EventBus.subscribe :message_thread_changed, ->(message_thread) {
   Searchable::ReindexMessageThreadJob.perform_later(message_thread.id)
@@ -92,3 +90,6 @@ def self.reset!
 EventBus.subscribe :filter_created, ->(filter) { AuditLog::FilterCreated.create_audit_record(filter) }
 EventBus.subscribe :filter_updated, ->(filter) { AuditLog::FilterUpdated.create_audit_record(filter) }
 EventBus.subscribe :filter_destroyed, ->(filter) { AuditLog::FilterDestroyed.create_audit_record(filter) }
+
+EventBus.subscribe :create_tenant_api_called, ->(request, response) { AuditLog::CreateTenantApiCalled.create_audit_record(request, response) }
+EventBus.subscribe :destroy_tenant_api_called, ->(request, response) { AuditLog::DestroyTenantApiCalled.create_audit_record(request, response) }

app/models/audit_log.rb

@@ -270,6 +270,20 @@ def self.create_audit_record(message_object)
     end
   end
 
+  class CreateTenantApiCalled < AuditLog
+    def self.create_audit_record(request, response)
+      # TODO: jwt missing
+      create_record(object: { request_url: request.url, request_method: request.method, response_code: response.code })
+    end
+  end
+
+  class DestroyTenantApiCalled < AuditLog
+    def self.create_audit_record(request, response)
+      # TODO: jwt missing
+      create_record(object: { request_url: request.url, request_method: request.method, response_code: response.code })
+    end
+  end
+
   def self.create_record(object:, **args)
     create(
       tenant: Current.tenant,

app/models/tenant.rb

@@ -66,6 +66,13 @@ def make_admins_see_everything!
     everything_tag.groups << admin_group
   end
 
+  def self.create_with_admin(params)
+    tenant = create(name: params[:name])
+    admin = tenant.users.create(name: params[:admin][:name], email: params[:admin][:email])
+    group_membership = tenant.admin_group.users.push(admin)
+    [tenant, admin, group_membership]
+  end
+
   private
 
   def create_default_objects

app/views/api/admin/boxes/create.json.jbuilder

@@ -1,7 +1 @@
-json.tenant_id @box.tenant_id
 json.id @box.id
-json.name @box.name
-json.uri @box.uri
-json.short_name @box.short_name
-json.color @box.color
-json.api_connection_id @box.api_connection_id

app/views/api/admin/boxes/error.json.jbuilder

@@ -0,0 +1 @@
+json.message @error || @box.errors.full_messages[0]

app/views/api/admin/tenants/create.json.jbuilder

@@ -1,8 +1 @@
 json.id @tenant.id
-json.name @tenant.name
-json.feature_flags @tenant.feature_flags
-json.admin do
-  json.id @admin.id
-  json.name @admin.name
-  json.email @admin.email
-end

app/views/api/admin/tenants/error.json.jbuilder

@@ -0,0 +1,4 @@
+json.message @error ||
+             @tenant.errors.full_messages[0] ||
+             @admin.errors.full_messages[0] ||
+             @group_membership.errors.full_messages[0]

test/integration/tenant_api_test.rb

@@ -2,15 +2,16 @@
 
 class TenantApiTest < ActionDispatch::IntegrationTest
   test "can create tenant" do
-    post "/api/admin/tenants", params: { tenant: { name: "Testovaci tenant" }, admin: { name: "Testovaci admin", email: "[email protected]" } }, as: :json
+    post "/api/admin/tenants", params: { tenant: { name: "Testovaci tenant", admin: { name: "Testovaci admin", email: "[email protected]" } } }, as: :json
     assert_response :success
     json_response = JSON.parse(response.body)
-    assert_equal "Testovaci tenant", json_response["name"]
     assert_not_nil json_response["id"]
-    assert_empty json_response["feature_flags"]
     assert Tenant.exists?(json_response["id"])
-    assert Tenant.find(json_response["id"]).admin_group.users.first.name, "Testovaci admin"
-    assert Tenant.find(json_response["id"]).admin_group.users.first.email, "[email protected]"
+    tenant = Tenant.find(json_response["id"])
+    assert_empty tenant.feature_flags
+    assert_equal "Testovaci tenant", tenant.name
+    assert tenant.admin_group.users.first.name, "Testovaci admin"
+    assert tenant.admin_group.users.first.email, "[email protected]"
   end
 
   test "can destroy tenant" do
@@ -26,6 +27,9 @@ class TenantApiTest < ActionDispatch::IntegrationTest
     post "/api/admin/tenants/#{tenant.id}/boxes", params: { box: { name: "Test box", uri: "ico://sk//12345678", short_name: "TST", color: "blue", api_connection_id: api_connections(:govbox_api_api_connection1).id } }, as: :json
     assert_response :success
     json_response = JSON.parse(response.body)
-    assert_equal "Test box", json_response["name"]
+    box = tenant.boxes.find_by(name: "Test box")
+    assert box
+    assert json_response["id"]
+    assert_equal json_response["id"], box.id
   end
 end