Merge pull request #125 from paviliondev/submission_improvements

Abstract and improve submission handling

assets/javascripts/discourse/routes/admin-wizards-submissions-show.js.es6

@@ -1,17 +1,19 @@
 import CustomWizard from "../models/custom-wizard";
 import DiscourseRoute from "discourse/routes/discourse";
 
+const excludedMetaFields = ["route_to", "redirect_on_complete", "redirect_to"];
+
 export default DiscourseRoute.extend({
   model(params) {
     return CustomWizard.submissions(params.wizardId);
   },
 
   setupController(controller, model) {
-    if (model.submissions) {
+    if (model && model.submissions) {
-      let fields = [];
+      let fields = ["username"];
       model.submissions.forEach((s) => {
-        Object.keys(s).forEach((k) => {
+        Object.keys(s.fields).forEach((k) => {
-          if (fields.indexOf(k) < 0) {
+          if (!excludedMetaFields.includes(k) && fields.indexOf(k) < 0) {
             fields.push(k);
           }
         });
@@ -19,9 +21,13 @@ export default DiscourseRoute.extend({
 
       let submissions = [];
       model.submissions.forEach((s) => {
-        let submission = {};
+        let submission = {
-        fields.forEach((f) => {
+          username: s.username,
-          submission[f] = s[f];
+        };
+        Object.keys(s.fields).forEach((f) => {
+          if (fields.includes(f)) {
+            submission[f] = s.fields[f];
+          }
         });
         submissions.push(submission);
       });

config/locales/server.en.yml

@@ -1,8 +1,8 @@
 en:
   admin:
     wizard:
-      submissions:
+      submission:
-        no_user: "deleted (id: %{id})"
+        no_user: "deleted (user_id: %{user_id})"
 
   wizard:
     custom_title: "Wizard"

controllers/custom_wizard/admin/submissions.rb

@@ -13,34 +13,20 @@ class CustomWizard::AdminSubmissionsController < CustomWizard::AdminController
   def show
     render_json_dump(
       wizard: CustomWizard::BasicWizardSerializer.new(@wizard, root: false),
-      submissions: build_submissions.as_json
+      submissions: ActiveModel::ArraySerializer.new(ordered_submissions, each_serializer: CustomWizard::SubmissionSerializer)
     )
   end
 
   def download
-    send_data build_submissions.to_json,
+    send_data ordered_submissions.to_json,
       filename: "#{Discourse.current_hostname}-wizard-submissions-#{@wizard.name}.json",
       content_type: "application/json",
       disposition: "attachment"
   end
 
-  private
+  protected
 
-  def build_submissions
+  def ordered_submissions
-    PluginStoreRow.where(plugin_name: "#{@wizard.id}_submissions")
+    CustomWizard::Submission.list(@wizard, order_by: 'id')
-      .order('id DESC')
-      .map do |row|
-        value = ::JSON.parse(row.value)
-
-        if user = User.find_by(id: row.key)
-          username = user.username
-        else
-          username = I18n.t('admin.wizard.submissions.no_user', id: row.key)
-        end
-
-        value.map do |v|
-          { username: username }.merge!(v.except("redirect_to"))
-        end
-      end.flatten
   end
 end

controllers/custom_wizard/steps.rb

@@ -8,7 +8,7 @@ class CustomWizard::StepsController < ::ApplicationController
 
     update[:fields] = {}
     if params[:fields]
-      field_ids = @step_template['fields'].map { |f| f['id'] }
+      field_ids = @builder.wizard.field_ids
       params[:fields].each do |k, v|
         update[:fields][k] = v if field_ids.include? k
       end
@@ -28,7 +28,7 @@ class CustomWizard::StepsController < ::ApplicationController
       current_step = @wizard.find_step(update[:step_id])
       current_submission = @wizard.current_submission
       result = {}
-      @wizard.filter_conditional_fields
+
       if current_step.conditional_final_step && !current_step.last_step
         current_step.force_final = true
       end
@@ -36,15 +36,19 @@ class CustomWizard::StepsController < ::ApplicationController
       if current_step.final?
         builder.template.actions.each do |action_template|
           if action_template['run_after'] === 'wizard_completion'
-            CustomWizard::Action.new(
+            action_result = CustomWizard::Action.new(
               action: action_template,
               wizard: @wizard,
-              data: current_submission
+              submission: current_submission
             ).perform
+
+            if action_result.success?
+              current_submission = action_result.submission
+            end
           end
         end
 
-        @wizard.save_submission(current_submission)
+        current_submission.save
 
         if redirect = get_redirect
           updater.result[:redirect_on_complete] = redirect
@@ -54,6 +58,8 @@ class CustomWizard::StepsController < ::ApplicationController
 
         result[:final] = true
       else
+        current_submission.save
+
         result[:final] = false
         result[:next_step_id] = current_step.next.id
       end
@@ -101,9 +107,9 @@ class CustomWizard::StepsController < ::ApplicationController
   def get_redirect
     return @result[:redirect_on_next] if @result[:redirect_on_next].present?
 
-    current_submission = @wizard.current_submission
+    submission = @wizard.current_submission
-    return nil unless current_submission.present?
+    return nil unless submission.present?
     ## route_to set by actions, redirect_on_complete set by actions, redirect_to set at wizard entry
-    current_submission[:route_to] || current_submission[:redirect_on_complete] || current_submission[:redirect_to]
+    submission.route_to || submission.redirect_on_complete || submission.redirect_to
   end
 end

controllers/custom_wizard/wizard.rb

@@ -63,8 +63,9 @@ class CustomWizard::WizardController < ::ApplicationController
 
     if user && wizard.can_access?
       submission = wizard.current_submission
-      if submission && submission['redirect_to']
+
-        result.merge!(redirect_to: submission['redirect_to'])
+      if submission.present? && submission.redirect_to
+        result.merge!(redirect_to: submission.redirect_to)
       end
 
       wizard.final_cleanup!

coverage/.last_run.json

@@ -1,5 +1,5 @@
 {
   "result": {
-    "line": 90.52
+    "line": 91.83
   }
 }

extensions/invites_controller.rb

@@ -5,7 +5,7 @@ module InvitesControllerCustomWizard
       wizard_id = @user.custom_fields['redirect_to_wizard']
 
       if wizard_id && url != '/'
-        CustomWizard::Wizard.set_submission_redirect(@user, wizard_id, url)
+        CustomWizard::Wizard.set_wizard_redirect(@user, wizard_id, url)
         url = "/w/#{wizard_id.dasherize}"
       end
     end

jobs/set_after_time_wizard.rb

@@ -9,7 +9,7 @@ module Jobs
           user_ids = []
 
           User.human_users.each do |user|
-            if CustomWizard::Wizard.set_wizard_redirect(wizard.id, user)
+            if CustomWizard::Wizard.set_user_redirect(wizard.id, user)
               user_ids.push(user.id)
             end
           end

lib/custom_wizard/action.rb

@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 class CustomWizard::Action
-  attr_accessor :data,
+  attr_accessor :submission,
                 :action,
                 :user,
                 :guardian,
@@ -11,7 +11,7 @@ class CustomWizard::Action
     @action = opts[:action]
     @user = @wizard.user
     @guardian = Guardian.new(@user)
-    @data = opts[:data]
+    @submission = opts[:submission]
     @log = []
     @result = CustomWizard::ActionResult.new
   end
@@ -26,14 +26,21 @@ class CustomWizard::Action
     end
 
     if @result.success? && @result.output.present?
-      data[action['id']] = @result.output
+      @submission.fields[action['id']] = @result.output
     end
 
     save_log
+
+    @result.submission = @submission
+    @result
+  end
+
+  def mapper_data
+    @mapper_data ||= @submission&.fields_and_meta || {}
   end
 
   def mapper
-    @mapper ||= CustomWizard::Mapper.new(user: user, data: data)
+    @mapper ||= CustomWizard::Mapper.new(user: user, data: mapper_data)
   end
 
   def create_topic
@@ -47,7 +54,7 @@ class CustomWizard::Action
         messages = creator.errors.full_messages.join(" ")
         log_error("failed to create", messages)
       elsif action['skip_redirect'].blank?
-        data['redirect_on_complete'] = post.topic.url
+        @submission.redirect_on_complete = post.topic.url
       end
 
       if creator.errors.blank?
@@ -65,7 +72,7 @@ class CustomWizard::Action
     if action['required'].present?
       required = CustomWizard::Mapper.new(
         inputs: action['required'],
-        data: data,
+        data: mapper_data,
         user: user
       ).perform
 
@@ -79,7 +86,7 @@ class CustomWizard::Action
 
     targets = CustomWizard::Mapper.new(
       inputs: action['recipient'],
-      data: data,
+      data: mapper_data,
       user: user,
       multiple: true
     ).perform
@@ -115,7 +122,7 @@ class CustomWizard::Action
         messages = creator.errors.full_messages.join(" ")
         log_error("failed to create message", messages)
       elsif action['skip_redirect'].blank?
-        data['redirect_on_complete'] = post.topic.url
+        @submission.redirect_on_complete = post.topic.url
       end
 
       if creator.errors.blank?
@@ -178,7 +185,7 @@ class CustomWizard::Action
   def watch_categories
     watched_categories = CustomWizard::Mapper.new(
       inputs: action['categories'],
-      data: data,
+      data: mapper_data,
       user: user
     ).perform
 
@@ -193,7 +200,7 @@ class CustomWizard::Action
 
     mute_remainder = CustomWizard::Mapper.new(
       inputs: action['mute_remainder'],
-      data: data,
+      data: mapper_data,
       user: user
     ).perform
 
@@ -202,7 +209,7 @@ class CustomWizard::Action
     if action['usernames']
       mapped_users = CustomWizard::Mapper.new(
         inputs: action['usernames'],
-        data: data,
+        data: mapper_data,
         user: user
       ).perform
 
@@ -284,7 +291,7 @@ class CustomWizard::Action
       end
 
       route_to = Discourse.base_uri + url
-      @result.output = data['route_to'] = route_to
+      @result.output = @submission.route_to = route_to
 
       log_success("route: #{route_to}")
     else
@@ -295,7 +302,7 @@ class CustomWizard::Action
   def add_to_group
     group_map = CustomWizard::Mapper.new(
       inputs: action['group'],
-      data: data,
+      data: mapper_data,
       user: user,
       opts: {
         multiple: true
@@ -345,18 +352,18 @@ class CustomWizard::Action
     else
       url = CustomWizard::Mapper.new(
         inputs: url_input,
-        data: data,
+        data: mapper_data,
         user: user
       ).perform
     end
 
     if action['code']
-      data[action['code']] = SecureRandom.hex(8)
+      @submission.fields[action['code']] = SecureRandom.hex(8)
-      url += "&#{action['code']}=#{data[action['code']]}"
+      url += "&#{action['code']}=#{@submission.fields[action['code']]}"
     end
 
     route_to = UrlHelper.encode(url)
-    data['route_to'] = route_to
+    @submission.route_to = route_to
 
     log_info("route: #{route_to}")
   end
@@ -416,7 +423,7 @@ class CustomWizard::Action
   def action_category
     output = CustomWizard::Mapper.new(
       inputs: action['category'],
-      data: data,
+      data: mapper_data,
       user: user
     ).perform
 
@@ -434,7 +441,7 @@ class CustomWizard::Action
   def action_tags
     output = CustomWizard::Mapper.new(
       inputs: action['tags'],
-      data: data,
+      data: mapper_data,
       user: user,
     ).perform
 
@@ -451,7 +458,7 @@ class CustomWizard::Action
     if (custom_fields = action['custom_fields']).present?
       field_map = CustomWizard::Mapper.new(
         inputs: custom_fields,
-        data: data,
+        data: mapper_data,
         user: user
       ).perform
       registered_fields = CustomWizard::CustomField.full_list
@@ -513,7 +520,7 @@ class CustomWizard::Action
 
     params[:title] = CustomWizard::Mapper.new(
       inputs: action['title'],
-      data: data,
+      data: mapper_data,
       user: user
     ).perform
 
@@ -525,7 +532,7 @@ class CustomWizard::Action
         wizard: true,
         template: true
       ) :
-      data[action['post']]
+      @submission.fields[action['post']]
 
     params[:import_mode] = ActiveRecord::Type::Boolean.new.cast(action['suppress_notifications'])
 
@@ -548,7 +555,7 @@ class CustomWizard::Action
         unless action[field].nil? || action[field] == ""
           params[field.to_sym] = CustomWizard::Mapper.new(
             inputs: action[field],
-            data: data,
+            data: mapper_data,
             user: user
           ).perform
         end
@@ -587,7 +594,7 @@ class CustomWizard::Action
       if input.present?
         value = CustomWizard::Mapper.new(
           inputs: input,
-          data: data,
+          data: mapper_data,
           user: user
         ).perform
 
@@ -617,7 +624,7 @@ class CustomWizard::Action
       if action[attr].present?
         value = CustomWizard::Mapper.new(
           inputs: action[attr],
-          data: data,
+          data: mapper_data,
           user: user
         ).perform
 

lib/custom_wizard/action_result.rb

@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 class CustomWizard::ActionResult
-  attr_accessor :success, :handler, :output
+  attr_accessor :success, :handler, :output, :submission
 
   def initialize
     @success = false

lib/custom_wizard/builder.rb

@@ -24,7 +24,7 @@ class CustomWizard::Builder
   def mapper
     CustomWizard::Mapper.new(
       user: @wizard.user,
-      data: @wizard.current_submission
+      data: @wizard.current_submission&.fields_and_meta
     )
   end
 
@@ -47,9 +47,8 @@ class CustomWizard::Builder
 
         step.on_update do |updater|
           @updater = updater
-          @submission = (@wizard.current_submission || {})
+          @submission = @wizard.current_submission
-            .merge(@updater.submission)
+          @submission.fields.merge!(@updater.submission)
-            .with_indifferent_access
 
           @updater.validate
           next if @updater.errors.any?
@@ -60,11 +59,11 @@ class CustomWizard::Builder
           run_step_actions
 
           if @updater.errors.empty?
-            if route_to = @submission['route_to']
+            route_to = @submission.route_to
-              @submission.delete('route_to')
+            @submission.route_to = nil
-            end
+            @submission.save
 
-            @wizard.save_submission(@submission)
+            @wizard.update!
             @updater.result[:redirect_on_next] = route_to if route_to
 
             true
@@ -75,7 +74,7 @@ class CustomWizard::Builder
       end
     end
 
-    @wizard.update_step_order!
+    @wizard.update!
     @wizard
   end
 
@@ -92,8 +91,8 @@ class CustomWizard::Builder
 
     params[:value] = prefill_field(field_template, step_template)
 
-    if !build_opts[:reset] && (submission = @wizard.current_submission)
+    if !build_opts[:reset] && (submission = @wizard.current_submission).present?
-      params[:value] = submission[field_template['id']] if submission[field_template['id']]
+      params[:value] = submission.fields[field_template['id']] if submission.fields[field_template['id']]
     end
 
     if field_template['type'] === 'group' && params[:value].present?
@@ -136,7 +135,7 @@ class CustomWizard::Builder
       content = CustomWizard::Mapper.new(
         inputs: content_inputs,
         user: @wizard.user,
-        data: @wizard.current_submission,
+        data: @wizard.current_submission&.fields_and_meta,
         opts: {
           with_type: true
         }
@@ -171,7 +170,7 @@ class CustomWizard::Builder
       index = CustomWizard::Mapper.new(
         inputs: field_template['index'],
         user: @wizard.user,
-        data: @wizard.current_submission
+        data: @wizard.current_submission&.fields_and_meta
       ).perform
 
       params[:index] = index.to_i unless index.nil?
@@ -217,7 +216,7 @@ class CustomWizard::Builder
       CustomWizard::Mapper.new(
         inputs: prefill,
         user: @wizard.user,
-        data: @wizard.current_submission
+        data: @wizard.current_submission&.fields_and_meta
       ).perform
     end
   end
@@ -227,7 +226,7 @@ class CustomWizard::Builder
       result = CustomWizard::Mapper.new(
         inputs: template['condition'],
         user: @wizard.user,
-        data: @wizard.current_submission,
+        data: @wizard.current_submission&.fields_and_meta,
         opts: {
           multiple: true
         }
@@ -297,19 +296,20 @@ class CustomWizard::Builder
     permitted_data = {}
     submission_key = nil
     params_key = nil
-    submission = @wizard.current_submission || {}
+    submission = @wizard.current_submission
 
     permitted_params.each do |pp|
       pair = pp['pairs'].first
       params_key = pair['key'].to_sym
       submission_key = pair['value'].to_sym
 
-      if submission_key && params_key
+      if submission_key && params_key && params[params_key].present?
-        submission[submission_key] = params[params_key]
+        submission.permitted_param_keys << submission_key.to_s
+        submission.fields[submission_key] = params[params_key]
       end
     end
 
-    @wizard.save_submission(submission)
+    submission.save
   end
 
   def ensure_required_data(step, step_template)
@@ -318,13 +318,13 @@ class CustomWizard::Builder
         pair['key'].present? && pair['value'].present?
       end
 
-      if pairs.any? && !@wizard.current_submission
+      if pairs.any? && !@wizard.current_submission.present?
         step.permitted = false
         break
       end
 
       pairs.each do |pair|
-        pair['key'] = @wizard.current_submission[pair['key']]
+        pair['key'] = @wizard.current_submission.fields[pair['key']]
       end
 
       if !mapper.validate_pairs(pairs)
@@ -348,11 +348,15 @@ class CustomWizard::Builder
     if @template.actions.present?
       @template.actions.each do |action_template|
         if action_template['run_after'] === updater.step.id
-          CustomWizard::Action.new(
+          result = CustomWizard::Action.new(
             action: action_template,
             wizard: @wizard,
-            data: @submission
+            submission: @submission
           ).perform
+
+          if result.success?
+            @submission = result.submission
+          end
         end
       end
     end

lib/custom_wizard/submission.rb

@@ -0,0 +1,118 @@
+# frozen_string_literal: true
+class CustomWizard::Submission
+  include ActiveModel::SerializerSupport
+
+  KEY ||= "submissions"
+  META ||= %w(submitted_at route_to redirect_on_complete redirect_to)
+
+  attr_reader :id,
+              :user,
+              :user_id,
+              :wizard
+
+  attr_accessor :fields,
+                :permitted_param_keys
+
+  META.each do |attr|
+    class_eval { attr_accessor attr }
+  end
+
+  def initialize(wizard, data = {}, user_id = nil)
+    @wizard = wizard
+    @user_id = user_id
+
+    if user_id
+      @user = User.find_by(id: user_id)
+    else
+      @user = wizard.user
+    end
+
+    data = (data || {}).with_indifferent_access
+    @id = data['id'] || SecureRandom.hex(12)
+    non_field_keys = META + ['id']
+    @fields = data.except(*non_field_keys) || {}
+
+    META.each do |attr|
+      send("#{attr}=", data[attr]) if data[attr]
+    end
+
+    @permitted_param_keys = data['permitted_param_keys'] || []
+  end
+
+  def save
+    return nil unless wizard.save_submissions
+    validate
+
+    submission_list = self.class.list(wizard, user_id: user.id)
+    submissions = submission_list.select { |submission| submission.id != self.id }
+    submissions.push(self)
+
+    submission_data = submissions.map { |submission| data_to_save(submission)  }
+    PluginStore.set("#{wizard.id}_#{KEY}", user.id, submission_data)
+  end
+
+  def validate
+    self.fields = fields.select { |key, value| validate_field_key(key) }
+  end
+
+  def validate_field_key(key)
+    wizard.field_ids.include?(key) ||
+    wizard.action_ids.include?(key) ||
+    permitted_param_keys.include?(key)
+  end
+
+  def fields_and_meta
+    result = fields
+
+    META.each do |attr|
+      if value = self.send(attr)
+        result[attr] = value
+      end
+    end
+
+    result
+  end
+
+  def present?
+    fields_and_meta.present?
+  end
+
+  def data_to_save(submission)
+    data = {
+      id: submission.id
+    }
+
+    data.merge!(submission.fields_and_meta)
+
+    if submission.permitted_param_keys.present?
+      data[:permitted_param_keys] = submission.permitted_param_keys
+    end
+
+    data
+  end
+
+  def self.get(wizard, user_id)
+    data = PluginStore.get("#{wizard.id}_#{KEY}", user_id).first
+    new(wizard, data, user_id)
+  end
+
+  def self.list(wizard, user_id: nil, order_by: nil)
+    params = { plugin_name: "#{wizard.id}_#{KEY}" }
+    params[:key] = user_id if user_id.present?
+
+    query = PluginStoreRow.where(params)
+    query = query.order("#{order_by} DESC") if order_by.present?
+
+    result = []
+
+    query.each do |record|
+      if (submission_data = ::JSON.parse(record.value)).any?
+        submission_data.each do |data|
+          result.push(new(wizard, data, record.key))
+        end
+      end
+    end
+
+    result
+  end
+end

lib/custom_wizard/wizard.rb

@@ -26,10 +26,15 @@ class CustomWizard::Wizard
                 :needs_groups,
                 :steps,
                 :step_ids,
+                :field_ids,
                 :first_step,
                 :start,
                 :actions,
-                :user
+                :action_ids,
+                :user,
+                :submissions
+
+  attr_reader   :all_step_ids
 
   def initialize(attrs = {}, user = nil)
     @user = user
@@ -58,11 +63,22 @@ class CustomWizard::Wizard
 
     @first_step = nil
     @steps = []
+
     if attrs['steps'].present?
-      @step_ids = attrs['steps'].map { |s| s['id'] }
+      @step_ids = @all_step_ids = attrs['steps'].map { |s| s['id'] }
+
+      @field_ids = []
+      attrs['steps'].each do |step|
+        if step['fields'].present?
+          step['fields'].each do |field|
+            @field_ids << field['id']
+          end
+        end
+      end
     end
 
-    @actions = []
+    @actions = attrs['actions'] || []
+    @action_ids = @actions.map { |a| a['id'] }
   end
 
   def cast_bool(val)
@@ -83,7 +99,19 @@ class CustomWizard::Wizard
     step.index = (steps.size == 1 ? 0 : steps.size) if step.index.nil?
   end
 
-  def update_step_order!
+  def update!
+    update_step_order
+    update_step_ids
+    update_field_ids
+    update_action_ids
+
+    @submissions = nil
+    @current_submission = nil
+
+    true
+  end
+
+  def update_step_order
     steps.sort_by!(&:index)
 
     steps.each_with_index do |step, index|
@@ -102,7 +130,7 @@ class CustomWizard::Wizard
         step.conditional_final_step = true
       end
 
-      if index === (step_ids.length - 1)
+      if index === (all_step_ids.length - 1)
         step.last_step = true
       end
 
@@ -117,7 +145,7 @@ class CustomWizard::Wizard
         acting_user_id: user.id,
         action: ::UserHistory.actions[:custom_wizard_step],
         context: id,
-        subject: step_ids
+        subject: all_step_ids
       ).order("created_at").last
 
       last_completed_step.subject
@@ -221,50 +249,41 @@ class CustomWizard::Wizard
     @groups ||= ::Site.new(Guardian.new(user)).groups
   end
 
+  def update_step_ids
+    @step_ids = steps.map(&:id)
+  end
+
+  def update_field_ids
+    @field_ids = steps.map { |step| step.fields.map { |field| field.id } }.flatten
+  end
+
+  def update_action_ids
+    @action_ids = []
+
+    @actions.each do |action|
+      if action['run_after'].blank? ||
+         action['run_after'] === 'wizard_completion' ||
+         step_ids.include?(action['run_after'])
+
+        @action_ids << action['id']
+      end
+    end
+  end
+
   def submissions
     return nil unless user.present?
-    @submissions ||= Array.wrap(PluginStore.get("#{id}_submissions", user.id))
+    @submissions ||= CustomWizard::Submission.list(self, user_id: user.id)
   end
 
   def current_submission
-    if submissions.present? && submissions.last.present? && !submissions.last.key?("submitted_at")
+    @current_submission ||= begin
-      submissions.last.with_indifferent_access
+      if submissions.present?
+        unsubmitted = submissions.select { |submission| !submission.submitted_at }
+        unsubmitted.present? ? unsubmitted.first : CustomWizard::Submission.new(self)
       else
-      nil
+        CustomWizard::Submission.new(self)
       end
     end
-
-  def set_submissions(submissions)
-    PluginStore.set("#{id}_submissions", user.id, Array.wrap(submissions))
-    @submissions = nil
-  end
-
-  def save_submission(submission)
-    return nil unless save_submissions
-
-    submissions.pop(1) if unfinished?
-    submissions.push(submission)
-    set_submissions(submissions)
-  end
-
-  def filter_conditional_fields
-    included_fields = steps.map { |s| s.fields.map { |f| f.id } }.flatten
-    filtered_submision = current_submission&.select do |key, _|
-      key = key.to_s
-      included_fields.include?(key) ||
-      required_fields.include?(key) ||
-      key.include?("action")
-    end
-
-    save_submission(filtered_submision)
-  end
-
-  def required_fields
-    %w{
-      submitted_at
-      route_to
-      saved_param
-    }
   end
 
   def final_cleanup!
@@ -273,18 +292,12 @@ class CustomWizard::Wizard
       user.save_custom_fields(true)
     end
 
-    if submission = current_submission
+    if current_submission.present?
-      submission['submitted_at'] = Time.now.iso8601
+      current_submission.submitted_at = Time.now.iso8601
-      save_submission(submission)
+      current_submission.save
-    end
     end
 
-  def self.submissions(wizard_id, user)
+    update!
-    new({ id: wizard_id }, user).submissions
-  end
-
-  def self.set_submissions(wizard_id, user, submissions)
-    new({ id: wizard_id }, user).set_submissions(submissions)
   end
 
   def self.create(wizard_id, user = nil)
@@ -339,11 +352,7 @@ class CustomWizard::Wizard
     end
   end
 
-  def self.set_submission_redirect(user, wizard_id, url)
+  def self.set_user_redirect(wizard_id, user)
-    set_submissions(wizard_id, user, [{ redirect_to: url }])
-  end
-
-  def self.set_wizard_redirect(wizard_id, user)
     wizard = self.create(wizard_id, user)
 
     if wizard.permitted?
@@ -353,4 +362,16 @@ class CustomWizard::Wizard
       false
     end
   end
+
+  def self.set_wizard_redirect(user, wizard_id, url)
+    wizard = self.create(wizard_id, user)
+
+    if wizard.permitted?
+      submission = wizard.current_submission
+      submission.redirect_to = url
+      submission.save
+    else
+      false
+    end
+  end
 end

plugin.rb

@@ -82,6 +82,7 @@ after_initialize do
     ../lib/custom_wizard/log.rb
     ../lib/custom_wizard/step_updater.rb
     ../lib/custom_wizard/step.rb
+    ../lib/custom_wizard/submission.rb
     ../lib/custom_wizard/template.rb
     ../lib/custom_wizard/wizard.rb
     ../lib/custom_wizard/api/api.rb
@@ -102,6 +103,7 @@ after_initialize do
     ../serializers/custom_wizard/wizard_step_serializer.rb
     ../serializers/custom_wizard/wizard_serializer.rb
     ../serializers/custom_wizard/log_serializer.rb
+    ../serializers/custom_wizard/submission_serializer.rb
     ../serializers/custom_wizard/realtime_validation/similar_topics_serializer.rb
     ../extensions/extra_locales_controller.rb
     ../extensions/invites_controller.rb
@@ -127,7 +129,7 @@ after_initialize do
 
       if !wizard.completed?
         custom_redirect = true
-        CustomWizard::Wizard.set_wizard_redirect(wizard.id, user)
+        CustomWizard::Wizard.set_user_redirect(wizard.id, user)
       end
     end
 
@@ -148,7 +150,7 @@ after_initialize do
 
   on(:user_approved) do |user|
     if wizard = CustomWizard::Wizard.after_signup(user)
-      CustomWizard::Wizard.set_wizard_redirect(wizard.id, user)
+      CustomWizard::Wizard.set_user_redirect(wizard.id, user)
     end
   end
 
@@ -159,7 +161,7 @@ after_initialize do
 
     if request.format === 'text/html' && !@excluded_routes.any? { |str| /#{str}/ =~ url } && wizard_id
       if request.referer !~ /\/w\// && request.referer !~ /\/invites\//
-        CustomWizard::Wizard.set_submission_redirect(current_user, wizard_id, request.referer)
+        CustomWizard::Wizard.set_wizard_redirect(current_user, wizard_id, request.referer)
       end
       if CustomWizard::Template.exists?(wizard_id)
         redirect_to "/w/#{wizard_id.dasherize}"

serializers/custom_wizard/submission_serializer.rb

@@ -0,0 +1,16 @@
+# frozen_string_literal: true
+class CustomWizard::SubmissionSerializer < ApplicationSerializer
+  attributes :id,
+             :username,
+             :fields,
+             :submitted_at,
+             :route_to,
+             :redirect_on_complete,
+             :redirect_to
+
+  def username
+    object.user.present? ?
+    object.user.username :
+    I18n.t('admin.wizard.submission.no_user', user_id: object.user_id)
+  end
+end

spec/components/custom_wizard/action_spec.rb

@@ -182,7 +182,7 @@ describe CustomWizard::Action do
       updater = wizard.create_updater(wizard.steps[1].id, {})
       updater.update
 
-      category = Category.find_by(id: wizard.current_submission['action_8'])
+      category = Category.find_by(id: wizard.current_submission.fields['action_8'])
 
       expect(updater.result[:redirect_on_next]).to eq(
         "/new-topic?title=Title%20of%20the%20composer%20topic&body=I%20am%20interpolating%20some%20user%20fields%20Angus%20angus%20angus%40email.com&category_id=#{category.id}&tags=tag1"
@@ -194,11 +194,10 @@ describe CustomWizard::Action do
       open_composer['post_template'] = "Body & more body & more body".dup
 
       wizard = CustomWizard::Wizard.new(@template, user)
-
       action = CustomWizard::Action.new(
         wizard: wizard,
         action: open_composer,
-        data: {}
+        submission: wizard.current_submission
       )
       action.perform
 
@@ -215,20 +214,20 @@ describe CustomWizard::Action do
     wizard = CustomWizard::Builder.new(@template[:id], user).build
     wizard.create_updater(wizard.steps[0].id, step_1_field_1: "Text input").update
     wizard.create_updater(wizard.steps[1].id, {}).update
-    expect(Category.where(id: wizard.current_submission['action_8']).exists?).to eq(true)
+    expect(Category.where(id: wizard.current_submission.fields['action_8']).exists?).to eq(true)
   end
 
   it 'creates a group' do
     wizard = CustomWizard::Builder.new(@template[:id], user).build
     wizard.create_updater(wizard.steps[0].id, step_1_field_1: "Text input").update
-    expect(Group.where(name: wizard.current_submission['action_9']).exists?).to eq(true)
+    expect(Group.where(name: wizard.current_submission.fields['action_9']).exists?).to eq(true)
   end
 
   it 'adds a user to a group' do
     wizard = CustomWizard::Builder.new(@template[:id], user).build
     step_id = wizard.steps[0].id
     updater = wizard.create_updater(step_id, step_1_field_1: "Text input").update
-    group = Group.find_by(name: wizard.current_submission['action_9'])
+    group = Group.find_by(name: wizard.current_submission.fields['action_9'])
     expect(group.users.first.username).to eq('angus')
   end
 
@@ -237,7 +236,7 @@ describe CustomWizard::Action do
     wizard.create_updater(wizard.steps[0].id, step_1_field_1: "Text input").update
     wizard.create_updater(wizard.steps[1].id, {}).update
     expect(CategoryUser.where(
-      category_id: wizard.current_submission['action_8'],
+      category_id: wizard.current_submission.fields['action_8'],
       user_id: user.id
     ).first.notification_level).to eq(2)
     expect(CategoryUser.where(

spec/components/custom_wizard/builder_spec.rb

@@ -189,7 +189,10 @@ describe CustomWizard::Builder do
     context "user has partially completed" do
       before do
         wizard = CustomWizard::Wizard.new(@template, user)
-        wizard.set_submissions(step_1_field_1: 'I am a user submission')
+        data = {
+          step_1_field_1: 'I am a user submission'
+        }
+        CustomWizard::Submission.new(wizard, data).save
       end
 
       it 'returns saved submissions' do
@@ -253,9 +256,9 @@ describe CustomWizard::Builder do
         end
 
         it 'is permitted if required data is present' do
-          CustomWizard::Wizard.set_submissions('super_mega_fun_wizard', user,
+          wizard = CustomWizard::Wizard.create('super_mega_fun_wizard', user)
-            required_data: "required_value"
+          CustomWizard::Submission.new(wizard, step_1_field_1: "required").save
-          )
+
           expect(
             CustomWizard::Builder.new(@template[:id], user).build
               .steps.first
@@ -274,7 +277,7 @@ describe CustomWizard::Builder do
           wizard = CustomWizard::Builder.new(@template[:id], user).build({},
             param: 'param_value'
           )
-          expect(wizard.current_submission['saved_param']).to eq('param_value')
+          expect(wizard.current_submission.fields['saved_param']).to eq('param_value')
         end
       end
 
@@ -336,31 +339,27 @@ describe CustomWizard::Builder do
 
     context 'on update' do
       def perform_update(step_id, submission)
-        wizard = CustomWizard::Builder.new(@template[:id], user).build
+        updater = @wizard.create_updater(step_id, submission)
-        updater = wizard.create_updater(step_id, submission)
         updater.update
         updater
       end
 
       it 'saves submissions' do
+        @wizard = CustomWizard::Builder.new(@template[:id], user).build
         perform_update('step_1', step_1_field_1: 'Text input')
-        expect(
+        expect(@wizard.current_submission.fields['step_1_field_1']).to eq('Text input')
-          CustomWizard::Wizard.submissions(@template[:id], user)
-            .first['step_1_field_1']
-        ).to eq('Text input')
       end
 
       context 'save submissions disabled' do
         before do
           @template[:save_submissions] = false
           CustomWizard::Template.save(@template.as_json)
+          @wizard = CustomWizard::Builder.new(@template[:id], user).build
         end
 
         it "does not save submissions" do
           perform_update('step_1', step_1_field_1: 'Text input')
-          expect(
+          expect(@wizard.current_submission.present?).to eq(false)
-            CustomWizard::Wizard.submissions(@template[:id], user).first
-          ).to eq(nil)
         end
       end
     end

spec/components/custom_wizard/submission_spec.rb

@@ -0,0 +1,43 @@
+# frozen_string_literal: true
+require_relative '../../plugin_helper'
+
+describe CustomWizard::Submission do
+  fab!(:user) { Fabricate(:user) }
+  fab!(:user2) { Fabricate(:user) }
+
+  let(:template_json) {
+    JSON.parse(File.open(
+      "#{Rails.root}/plugins/discourse-custom-wizard/spec/fixtures/wizard.json"
+    ).read)
+  }
+
+  before do
+    CustomWizard::Template.save(template_json, skip_jobs: true)
+
+    template_json_2 = template_json.dup
+    template_json_2["id"] = "super_mega_fun_wizard_2"
+    CustomWizard::Template.save(template_json_2, skip_jobs: true)
+
+    @wizard = CustomWizard::Wizard.create(template_json["id"], user)
+    @wizard2 = CustomWizard::Wizard.create(template_json["id"], user2)
+    @wizard3 = CustomWizard::Wizard.create(template_json_2["id"], user)
+
+    described_class.new(@wizard, step_1_field_1: "I am a user submission").save
+    described_class.new(@wizard2, step_1_field_1: "I am another user's submission").save
+    described_class.new(@wizard3, step_1_field_1: "I am a user submission on another wizard").save
+  end
+
+  it "saves a user's submission" do
+    expect(
+      described_class.get(@wizard, user.id).fields["step_1_field_1"]
+    ).to eq("I am a user submission")
+  end
+
+  it "list submissions by wizard" do
+    expect(described_class.list(@wizard).size).to eq(2)
+  end
+
+  it "list submissions by wizard and user" do
+    expect(described_class.list(@wizard, user_id: user.id).size).to eq(1)
+  end
+end

spec/components/custom_wizard/wizard_spec.rb

@@ -34,7 +34,7 @@ describe CustomWizard::Wizard do
     template_json['steps'].each do |step_template|
       @wizard.append_step(step_template['id'])
     end
-    @wizard.update_step_order!
+    @wizard.update!
   end
 
   def progress_step(step_id, acting_user: user, wizard: @wizard)
@@ -44,7 +44,7 @@ describe CustomWizard::Wizard do
       context: wizard.id,
       subject: step_id
     )
-    @wizard.update_step_order!
+    @wizard.update!
   end
 
   it "appends steps" do
@@ -72,7 +72,7 @@ describe CustomWizard::Wizard do
     expect(@wizard.steps.first.index).to eq(2)
     expect(@wizard.steps.last.index).to eq(0)
 
-    @wizard.update_step_order!
+    @wizard.update!
 
     expect(@wizard.steps.first.id).to eq("step_3")
     expect(@wizard.steps.last.id).to eq("step_1")
@@ -205,14 +205,7 @@ describe CustomWizard::Wizard do
 
   context "submissions" do
     before do
-      @wizard.set_submissions(step_1_field_1: 'I am a user submission')
+      CustomWizard::Submission.new(@wizard, step_1_field_1: "I am a user submission").save
-    end
-
-    it "sets the user's submission" do
-      expect(
-        PluginStore.get("#{template_json['id']}_submissions", user.id)
-          .first['step_1_field_1']
-      ).to eq('I am a user submission')
     end
 
     it "lists the user's submissions" do
@@ -220,20 +213,10 @@ describe CustomWizard::Wizard do
     end
 
     it "returns the user's current submission" do
-      expect(@wizard.current_submission['step_1_field_1']).to eq('I am a user submission')
+      expect(@wizard.current_submission.fields["step_1_field_1"]).to eq("I am a user submission")
     end
   end
 
-  it "provides class methods to set and list submissions" do
-    CustomWizard::Wizard.set_submissions(template_json['id'], user,
-      step_1_field_1: 'I am a user submission'
-    )
-    expect(
-      CustomWizard::Wizard.submissions(template_json['id'], user)
-        .first['step_1_field_1']
-    ).to eq('I am a user submission')
-  end
-
   context "class methods" do
     before do
       CustomWizard::Template.save(@permitted_template, skip_jobs: true)
@@ -274,7 +257,7 @@ describe CustomWizard::Wizard do
 
   it "sets wizard redirects if user is permitted" do
     CustomWizard::Template.save(@permitted_template, skip_jobs: true)
-    CustomWizard::Wizard.set_wizard_redirect('super_mega_fun_wizard', trusted_user)
+    CustomWizard::Wizard.set_user_redirect('super_mega_fun_wizard', trusted_user)
     expect(
       trusted_user.custom_fields['redirect_to_wizard']
     ).to eq("super_mega_fun_wizard")
@@ -282,7 +265,7 @@ describe CustomWizard::Wizard do
 
   it "does not set a wizard redirect if user is not permitted" do
     CustomWizard::Template.save(@permitted_template, skip_jobs: true)
-    CustomWizard::Wizard.set_wizard_redirect('super_mega_fun_wizard', user)
+    CustomWizard::Wizard.set_user_redirect('super_mega_fun_wizard', user)
     expect(
       trusted_user.custom_fields['redirect_to_wizard']
     ).to eq(nil)

spec/fixtures/step/required_data.json

@@ -6,9 +6,9 @@
       "pairs": [
         {
           "index": 0,
-          "key": "required_data",
+          "key": "step_1_field_1",
           "key_type": "text",
-          "value": "required_value",
+          "value": "required",
           "value_type": "text",
           "connector": "equal"
         }

spec/requests/custom_wizard/admin/submissions_controller_spec.rb

@@ -5,6 +5,7 @@ describe CustomWizard::AdminSubmissionsController do
   fab!(:admin_user) { Fabricate(:user, admin: true) }
   fab!(:user1) { Fabricate(:user) }
   fab!(:user2) { Fabricate(:user) }
+  fab!(:user3) { Fabricate(:user) }
 
   let(:template) {
     JSON.parse(File.open(
@@ -12,35 +13,40 @@ describe CustomWizard::AdminSubmissionsController do
     ).read)
   }
 
+  let(:template_2) {
+    temp = template.dup
+    temp["id"] = "super_mega_fun_wizard_2"
+    temp
+  }
+
   before do
     CustomWizard::Template.save(template, skip_jobs: true)
-    CustomWizard::Wizard.set_submissions(template['id'], user1,
+    CustomWizard::Template.save(template_2, skip_jobs: true)
-      step_1_field_1: "I am a user1's submission"
+
-    )
+    wizard1 = CustomWizard::Wizard.create(template["id"], user1)
-    CustomWizard::Wizard.set_submissions(template['id'], user2,
+    wizard2 = CustomWizard::Wizard.create(template["id"], user2)
-      step_1_field_1: "I am a user2's submission"
+    wizard3 = CustomWizard::Wizard.create(template_2["id"], user3)
-    )
+
+    CustomWizard::Submission.new(wizard1, step_1_field_1: "I am a user1's submission").save
+    CustomWizard::Submission.new(wizard2, step_1_field_1: "I am a user2's submission").save
+    CustomWizard::Submission.new(wizard3, step_1_field_1: "I am a user3's submission").save
+
     sign_in(admin_user)
   end
 
-  it "returns a basic list of wizards" do
+  it "returns a list of wizards" do
     get "/admin/wizards/submissions.json"
-    expect(response.parsed_body.length).to eq(1)
+    expect(response.parsed_body.length).to eq(2)
     expect(response.parsed_body.first['id']).to eq(template['id'])
   end
 
-  it "returns the all user's submissions for a wizard" do
+  it "returns users' submissions for a wizard" do
     get "/admin/wizards/submissions/#{template['id']}.json"
     expect(response.parsed_body['submissions'].length).to eq(2)
   end
 
-  it "returns the all user's submissions for a wizard" do
+  it "downloads submissions" do
-    get "/admin/wizards/submissions/#{template['id']}.json"
+    get "/admin/wizards/submissions/#{template_2['id']}/download"
-    expect(response.parsed_body['submissions'].length).to eq(2)
+    expect(response.parsed_body.length).to eq(1)
-  end
-
-  it "downloads all user submissions" do
-    get "/admin/wizards/submissions/#{template['id']}/download"
-    expect(response.parsed_body.length).to eq(2)
   end
 end

spec/requests/custom_wizard/application_controller_spec.rb

@@ -39,8 +39,8 @@ describe ApplicationController do
       it "saves original destination of user" do
         get '/', headers: { 'REFERER' => "/t/2" }
         expect(
-          CustomWizard::Wizard.submissions(@template['id'], user)
+          CustomWizard::Wizard.create(@template['id'], user).submissions
-            .first['redirect_to']
+            .first.redirect_to
         ).to eq("/t/2")
       end
 

spec/requests/custom_wizard/steps_controller_spec.rb

@@ -68,7 +68,7 @@ describe CustomWizard::StepsController do
 
     wizard_id = response.parsed_body['wizard']['id']
     wizard = CustomWizard::Wizard.create(wizard_id, user)
-    expect(wizard.submissions.last['step_1_field_1']).to eq("Text input")
+    expect(wizard.current_submission.fields['step_1_field_1']).to eq("Text input")
   end
 
   context "raises an error" do
@@ -175,8 +175,11 @@ describe CustomWizard::StepsController do
 
     wizard_id = response.parsed_body['wizard']['id']
     wizard = CustomWizard::Wizard.create(wizard_id, user)
-    group_name = wizard.submissions.last['action_9']
+
+    group_name = wizard.submissions.first.fields['action_9']
     group = Group.find_by(name: group_name)
+
+    expect(group.present?).to eq(true)
     expect(group.full_name).to eq("My cool group")
   end
 
@@ -275,7 +278,7 @@ describe CustomWizard::StepsController do
 
     wizard_id = response.parsed_body['wizard']['id']
     wizard = CustomWizard::Wizard.create(wizard_id, user)
-    submission = wizard.submissions.last
+    submission = wizard.current_submission
-    expect(submission.keys).not_to include("step_2_field_1")
+    expect(submission.fields.keys).not_to include("step_2_field_1")
   end
 end

spec/requests/custom_wizard/wizard_controller_spec.rb

@@ -71,9 +71,8 @@ describe CustomWizard::WizardController do
   end
 
   it 'skip response contains a redirect_to if in users submissions' do
-    CustomWizard::Wizard.set_submissions(@template['id'], user,
+    @wizard = CustomWizard::Wizard.create(@template["id"], user)
-      redirect_to: '/t/2'
+    CustomWizard::Submission.new(@wizard, redirect_to: "/t/2").save
-    )
     put '/w/super-mega-fun-wizard/skip.json'
     expect(response.parsed_body['redirect_to']).to eq('/t/2')
   end