[bug fix] Run Error API actions (#1327) (#1331)

Addresses: #985

Addresses: #1071

### Demo:

https://user-images.githubusercontent.com/50227291/188673867-186d03e9-a8ec-4cb7-adb0-6a6483ba1cec.mov

https://user-images.githubusercontent.com/50227291/210391370-7d38a132-6d10-4cc4-85ed-d1eb14e088c8.mov


Since api resource is not saved due to any error, api action will keep track of user input with proper error message under 

`additional_data['api_resource']['properties']` and `additional_data['api_resource']['errors']`

Demo shows error message and user input being accessed by api_action

### Demo fix dead set functionality


https://user-images.githubusercontent.com/50227291/189976658-e7c6fbe8-1f69-4740-9e98-98835a8fae26.mov

### Demo showing error message in API 
https://user-images.githubusercontent.com/50227291/196223270-e1d831f3-8403-43eb-84c2-d5419ea078d4.mov

Co-authored-by: Pralish Kayastha <[email protected]>
Co-authored-by: Pralish Kayastha <[email protected]>

app/controllers/concerns/api_actionable.rb

@@ -73,32 +73,12 @@ def execute_api_actions
   end
 
   def handle_error(e)
-    clone_actions(:error_api_actions)
     execute_error_actions
     raise
   end
 
   def execute_error_actions
-    return if @api_resource.nil?
-
-    error_api_actions = @api_resource.error_api_actions
-
-    redirect_action = error_api_actions.where(action_type: 'redirect').last
-    error_api_actions.each do |action|
-      action.execute_action unless action.redirect?
-    end
-
-    if redirect_action
-      redirect_action.update(lifecycle_stage: 'complete', lifecycle_message: redirect_action.redirect_url)
-      # Redirecting with JS is only needed when dealing with reCaptcha.
-      # reCaptcha related request is handled by ResourceController
-      if controller_name == "resource"
-        redirect_with_js(redirect_url) and return
-      else
-        redirect_to redirect_url and return
-      end
-    end
-
+    ErrorApiAction.where(id: create_error_actions.map(&:id)).execute_model_context_api_actions
     @error_api_actions_exectuted = true
   end
 
@@ -115,7 +95,19 @@ def clone_actions(action_name)
     return if @api_resource.nil? || @api_resource.new_record?
 
     @api_namespace.send(action_name).each do |action|
-      @api_resource.send(action_name).create(action.attributes.merge(custom_message: action.custom_message.to_s).except("id", "created_at", "updated_at", "api_namespace_id"))
+      @api_resource.send(action_name).create(action.attributes.merge(custom_message: action.custom_message.to_s, parent_id: action.id).except("id", "created_at", "updated_at", "api_namespace_id"))
+    end
+  end
+
+  # api_resource doesn't get saved if there's any error
+  def create_error_actions
+    @api_namespace.error_api_actions.map do |action|
+      api_resource_json = {
+        properties: @api_resource.properties,
+        api_namespace_id: @api_namespace.id,
+        errors: @api_resource.errors.full_messages.to_sentence
+      }
+      ErrorApiAction.create(action.attributes.merge(custom_message: action.custom_message.to_s, parent_id: action.id, meta_data: { api_resource: api_resource_json }).except("id", "created_at", "updated_at", "api_namespace_id"))
     end
   end
 

app/models/api_action.rb

@@ -6,7 +6,7 @@ class ApiAction < ApplicationRecord
   attr_encrypted :bearer_token
   attr_dynamic :email, :email_subject, :custom_message, :payload_mapping, :custom_headers, :request_url, :redirect_url
 
-  after_update :update_executed_actions_payload, if: Proc.new { api_namespace.present? && saved_change_to_payload_mapping? }
+  after_update :update_api_resource_actions, if: Proc.new { self.api_namespace_action? }
 
   belongs_to :api_namespace, optional: true
   belongs_to :api_resource, optional: true
@@ -17,6 +17,11 @@ class ApiAction < ApplicationRecord
 
   enum redirect_type: { cms_page: 0, dynamic_url: 1 }
 
+  # api_namespace_action acts as class defination and api_resource_actions act as instance of api_namespace_action
+  has_many :api_resource_actions, class_name: 'ApiAction', foreign_key: :parent_id
+
+  belongs_to :api_namespace_action, class_name: 'ApiAction', foreign_key: :parent_id, optional: true
+
   EXECUTION_ORDER = {
     model_level: ['send_email', 'send_web_request', 'custom_action'],
     controller_level: ['serve_file', 'redirect'],
@@ -38,44 +43,95 @@ def self.children
     ['new_api_actions', 'create_api_actions', 'show_api_actions', 'update_api_actions', 'destroy_api_actions', 'error_api_actions']
   end
 
-  def execute_action
+  def execute_action(run_error_action = true)
     self.update(lifecycle_stage: 'executing')
-    send(action_type)
+    send(action_type, run_error_action)
+  end
+
+  def self.execute_model_context_api_actions
+    api_actions = self.where(action_type: ApiAction::EXECUTION_ORDER[:model_level], lifecycle_stage: 'initialized')
+
+    ApiAction::EXECUTION_ORDER[:model_level].each do |action_type|
+      if ApiAction.action_types[action_type] == ApiAction.action_types[:custom_action]
+        custom_actions = api_actions.where(action_type: 'custom_action')
+        custom_actions.each do |custom_action|
+          FireApiActionsJob.perform_async(custom_action.id, Current.user&.id, Current.visit&.id, Current.is_api_html_renderer_request)
+        end
+      elsif [ApiAction.action_types[:send_email], ApiAction.action_types[:send_web_request]].include?(ApiAction.action_types[action_type])
+        api_actions.where(action_type: ApiAction.action_types[action_type]).each do |api_action|
+          FireApiActionsJob.perform_async(api_action.id, Current.user&.id, Current.visit&.id, Current.is_api_html_renderer_request)
+        end
+      end
+    end if api_actions.present?
+  end
+
+
+  def execute_error_actions(error)
+    self.update(lifecycle_stage: 'failed', lifecycle_message: error)
+
+    return if type == 'ErrorApiAction' || api_resource.nil?
+
+    api_resource.api_namespace.error_api_actions.each do |action|
+      api_resource.error_api_actions.create(action.attributes.merge(custom_message: action.custom_message.to_s).except("id", "created_at", "updated_at", "api_namespace_id"))
+    end
+    api_resource.error_api_actions.each(&:execute_action)
+  end
+
+  def api_namespace_action?
+    api_namespace_id.present?
+  end
+
+  def api_resource_action?
+    api_resource_id.present?
   end
 
   private
 
-  def update_executed_actions_payload
-    ApiAction.where(api_resource_id: api_namespace.api_resources.pluck(:id), payload_mapping: payload_mapping_previously_was, type: type, action_type: action_type).update_all(payload_mapping: payload_mapping)
+  def update_api_resource_actions
+    api_actions_to_update = self.api_resource_actions.where.not(lifecycle_stage: [:complete, :discarded])
+    api_actions_to_update.update_all({
+      payload_mapping: payload_mapping,
+      include_api_resource_data: include_api_resource_data,
+      redirect_url: redirect_url,
+      request_url: request_url,
+      position: position,
+      email: email,
+      file_snippet: file_snippet,
+      custom_headers: custom_headers,
+      http_method: http_method,
+      method_definition: method_definition,
+      email_subject: email_subject,
+      redirect_type: redirect_type,
+    })
+    ActionText::RichText.where(record_type: 'ApiAction', record_id: api_actions_to_update.pluck(:id)).update_all(body: custom_message.to_s)
   end
 
-  def send_email
+  def send_email(run_error_action = true)
     begin
       ApiActionMailer.send_email(self).deliver_now
       self.update(lifecycle_stage: 'complete', lifecycle_message: email)
-    rescue => e
-      self.update(lifecycle_stage: 'failed', lifecycle_message: e.message)
-      execute_error_actions
+    rescue Exception => e
+      execute_error_actions(e.message) if run_error_action
+      raise
     end
   end
 
-  def send_web_request
+  def send_web_request(run_error_action = true)
     begin
       response = HTTParty.send(http_method.to_s, request_url_evaluated, 
                     { body: payload_mapping_evaluated, headers: request_headers })
       if response.success?
         self.update(lifecycle_stage: 'complete', lifecycle_message: response.to_s)
       else
-        self.update(lifecycle_stage: 'failed', lifecycle_message: response.to_s)
-        execute_error_actions
+        execute_error_actions(response.to_s)
       end 
     rescue => e
-      self.update(lifecycle_stage: 'failed', lifecycle_message: e.message)
-      execute_error_actions
+      execute_error_actions(e.message) if run_error_action 
+      raise
     end
   end
 
-  def custom_action
+  def custom_action(run_error_action = true)
     begin
       custom_api_action = CustomApiAction.new
       eval("def custom_api_action.run_custom_action(api_action: , api_namespace: , api_resource: , current_visit: , current_user: nil); #{self.method_definition}; end")
@@ -84,27 +140,17 @@ def custom_action
 
       self.update(lifecycle_stage: 'complete', lifecycle_message: response.to_json)
     rescue => e
-      self.update(lifecycle_stage: 'failed', lifecycle_message: e.message)
-      execute_error_actions
+      execute_error_actions(e.message) if run_error_action
       raise
     end
   end
 
-  def redirect;end
+  def redirect(run_error_action = true);end
 
-  def serve_file;end
+  def serve_file(run_error_action = true);end
 
   def request_headers
-    headers = custom_headers_evaluated.gsub('SECRET_BEARER_TOKEN', bearer_token)
+    headers = custom_headers_evaluated.gsub('SECRET_BEARER_TOKEN', bearer_token.to_s)
     { 'Content-Type' => 'application/json' }.merge(JSON.parse(headers))
   end
-
-  def execute_error_actions
-    return if type == 'ErrorApiAction' || api_resource.nil?
-
-    api_resource.api_namespace.error_api_actions.each do |action|
-      api_resource.error_api_actions.create(action.attributes.merge(custom_message: action.custom_message.to_s).except("id", "created_at", "updated_at", "api_namespace_id"))
-    end
-    api_resource.error_api_actions.each(&:execute_action)
-  end
 end

app/models/api_namespace.rb

@@ -25,8 +25,6 @@ class ApiNamespace < ApplicationRecord
 
   has_many :api_actions, dependent: :destroy
 
-  has_many :executed_api_actions, through: :api_resources, class_name: 'ApiAction', source: :api_actions
-
   has_many :new_api_actions, dependent: :destroy
   accepts_nested_attributes_for :new_api_actions, allow_destroy: true
 
@@ -326,6 +324,10 @@ def self.import_as_json(json_str)
     end
   end
 
+  def executed_api_actions
+    ApiAction.where(api_resource_id: api_resources.pluck(:id)).or(ApiAction.where("meta_data->'api_resource' ->> 'api_namespace_id' = '#{self.id}'"))
+  end
+
   def snippet(with_brackets: true)
     return unless self.api_form.present?
 

app/models/api_resource.rb

@@ -44,9 +44,9 @@ class ApiResource < ApplicationRecord
     Arel.sql("api_resources.properties::text") 
   end
 
-  def clone_api_actions(action_name)
+  def initialize_api_resource_actions(action_name)
     api_namespace.send(action_name).each do |action|
-      self.send(action_name).create(action.attributes.merge('custom_message' => action.custom_message.to_s, 'lifecycle_stage' => 'initialized').except("id", "created_at", "updated_at", "api_namespace_id"))
+      self.send(action_name).create(action.attributes.merge('custom_message' => action.custom_message.to_s, 'lifecycle_stage' => 'initialized', parent_id: action.id).except("id", "created_at", "updated_at", "api_namespace_id"))
     end
   end
 
@@ -75,25 +75,7 @@ def tracked_user
   end
 
   def execute_model_context_api_actions(class_name)
-    api_actions = self.send(class_name).where(action_type: ApiAction::EXECUTION_ORDER[:model_level], lifecycle_stage: 'initialized')
-
-    ApiAction::EXECUTION_ORDER[:model_level].each do |action_type|
-      if ApiAction.action_types[action_type] == ApiAction.action_types[:custom_action]
-        begin
-          custom_actions = api_actions.where(action_type: 'custom_action')
-          custom_actions.each do |custom_action|
-            custom_action.execute_action
-          end
-        rescue
-          # error-actions are executed already in api-action level.
-          nil
-        end
-      elsif [ApiAction.action_types[:send_email], ApiAction.action_types[:send_web_request]].include?(ApiAction.action_types[action_type])
-        api_actions.where(action_type: ApiAction.action_types[action_type]).each do |api_action|
-          api_action.execute_action
-        end
-      end
-    end if api_actions.present?
+    self.send(class_name).execute_model_context_api_actions
   end
 
   private
@@ -121,12 +103,12 @@ def set_creator
   end
 
   def execute_create_api_actions
-    clone_api_actions('create_api_actions')
-    FireApiActionsJob.perform_async(self.id, 'create_api_actions', Current.user&.id, Current.visit&.id, Current.is_api_html_renderer_request)
+    initialize_api_resource_actions('create_api_actions')
+    CreateApiAction.where(id: self.create_api_actions.pluck(:id)).execute_model_context_api_actions
   end
 
   def execute_update_api_actions
-    clone_api_actions('update_api_actions')
-    FireApiActionsJob.perform_async(self.id, 'update_api_actions', Current.user&.id, Current.visit&.id, Current.is_api_html_renderer_request)
+    initialize_api_resource_actions('update_api_actions')
+    UpdateApiAction.where(id: self.update_api_actions.pluck(:id)).execute_model_context_api_actions
   end
 end

app/services/api_namespace/plugin/v1/subdomain_events_service.rb

@@ -34,6 +34,7 @@ def track_event
         body: "New Forum Post @ #{domain_with_fallback}.#{ENV['APP_HOST']} - created by: #{created_by_user.name} (#{created_by_user.email})"
       }
     end
-    ApiResourceSpawnJob.perform_async(api_namespace.id, resource_properties.to_json)
+
+    api_namespace.api_resources.create!(properties: resource_properties)
   end
 end

app/sidekiq/fire_api_actions_job.rb

@@ -1,14 +1,26 @@
 class FireApiActionsJob
   include Sidekiq::Job
 
-  def perform(api_resource_id, action_class, current_user_id, current_visit_id, is_api_html_renderer_request)
+  # run error actions only after final retries
+  sidekiq_retries_exhausted do |msg, exception|
+    set_current_visit(msg['args'][1], msg['args'][2], msg['args'][3]) do
+      ApiAction.find(msg['args'][0]).execute_error_actions(exception.message)
+    end
+  end
+
+  def perform(action_id, current_user_id, current_visit_id, is_api_html_renderer_request)
+    FireApiActionsJob.set_current_visit(current_user_id, current_visit_id, is_api_html_renderer_request) do
+      api_action = ApiAction.find(action_id)
+      api_action.execute_action(false) if api_action.present?
+    end
+  end
+
+  def self.set_current_visit(current_user_id, current_visit_id, is_api_html_renderer_request)
     current_user = User.find_by(id: current_user_id)
     current_visit = Ahoy::Visit.find_by(id: current_visit_id)
 
     Current.set(user: current_user, visit: current_visit, is_api_html_renderer_request: is_api_html_renderer_request) do
-      api_resource = ApiResource.find(api_resource_id)
-
-      api_resource.execute_model_context_api_actions(action_class)
+      yield
     end
   end
 end

app/views/comfy/admin/api_actions/index.html.haml

@@ -29,7 +29,14 @@
     - @api_actions.each do |api_action|
       %tr
         %td.px-3.py-2= link_to api_action.id, api_namespace_api_action_path(api_namespace_id: @api_namespace.id, id: api_action.id) 
-        %td.px-3.py-2= link_to api_action.api_resource_id, api_namespace_resource_path(api_namespace_id: @api_namespace.id, id: api_action.api_resource_id) 
+        %td.px-3.py-2
+          - if api_action.api_resource_id
+            = link_to api_action.api_resource_id, api_namespace_resource_path(api_namespace_id: @api_namespace.id, id: api_action.api_resource_id)
+          - else
+            %div= api_action.meta_data.dig('api_resource', 'properties')
+            %div
+              %strong Error:
+              = api_action.meta_data.dig('api_resource', 'errors')
         %td.px-3= api_action.type
         %td.px-3= api_action.action_type
         %td.px-3

db/migrate/20220906063031_add_additional_data_to_api_actions.rb

@@ -0,0 +1,5 @@
+class AddAdditionalDataToApiActions < ActiveRecord::Migration[6.1]
+  def change
+    add_column :api_actions, :meta_data, :jsonb, default: {}
+  end
+end

db/migrate/20230103105118_add_parent_id_to_api_actions.rb

@@ -0,0 +1,5 @@
+class AddParentIdToApiActions < ActiveRecord::Migration[6.1]
+  def change
+    add_column :api_actions, :parent_id, :integer
+  end
+end