From d3c6733e59c73508da3746681c5d8b39bde55d28 Mon Sep 17 00:00:00 2001 From: angusmcleod Date: Thu, 17 Jun 2021 17:50:22 +1000 Subject: [PATCH 1/8] Abstract and improve submission handling --- config/locales/server.en.yml | 4 +- .../custom_wizard/admin/submissions.rb | 26 ++---- controllers/custom_wizard/steps.rb | 10 +-- controllers/custom_wizard/wizard.rb | 5 +- extensions/invites_controller.rb | 2 +- jobs/set_after_time_wizard.rb | 2 +- lib/custom_wizard/action.rb | 54 +++++------ lib/custom_wizard/builder.rb | 35 ++++---- lib/custom_wizard/submission.rb | 89 +++++++++++++++++++ lib/custom_wizard/wizard.rb | 85 +++++++----------- plugin.rb | 7 +- serializers/custom_wizard/submission.rb | 13 +++ spec/components/custom_wizard/action_spec.rb | 10 +-- spec/components/custom_wizard/builder_spec.rb | 28 +++--- .../custom_wizard/submission_spec.rb | 43 +++++++++ spec/components/custom_wizard/wizard_spec.rb | 25 +----- .../admin/submissions_controller_spec.rb | 37 ++++---- .../custom_wizard/steps_controller_spec.rb | 8 +- .../custom_wizard/wizard_controller_spec.rb | 5 +- 19 files changed, 292 insertions(+), 196 deletions(-) create mode 100644 lib/custom_wizard/submission.rb create mode 100644 serializers/custom_wizard/submission.rb create mode 100644 spec/components/custom_wizard/submission_spec.rb diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 4bda825a..7e507450 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1,8 +1,8 @@ en: admin: wizard: - submissions: - no_user: "deleted (id: %{id})" + submission: + no_user: "deleted (user_id: %{user_id})" wizard: custom_title: "Wizard" diff --git a/controllers/custom_wizard/admin/submissions.rb b/controllers/custom_wizard/admin/submissions.rb index 5467587e..682c4030 100644 --- a/controllers/custom_wizard/admin/submissions.rb +++ b/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 + + protected - private - - def build_submissions - PluginStoreRow.where(plugin_name: "#{@wizard.id}_submissions") - .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 + def ordered_submissions + CustomWizard::Submission.list(@wizard, order_by: 'id') end end diff --git a/controllers/custom_wizard/steps.rb b/controllers/custom_wizard/steps.rb index aa4fbd7f..9c0fbf92 100644 --- a/controllers/custom_wizard/steps.rb +++ b/controllers/custom_wizard/steps.rb @@ -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 @@ -44,7 +44,7 @@ class CustomWizard::StepsController < ::ApplicationController end end - @wizard.save_submission(current_submission) + current_submission.save if redirect = get_redirect updater.result[:redirect_on_complete] = redirect @@ -101,9 +101,9 @@ class CustomWizard::StepsController < ::ApplicationController def get_redirect return @result[:redirect_on_next] if @result[:redirect_on_next].present? - current_submission = @wizard.current_submission - return nil unless current_submission.present? + submission = @wizard.current_submission + 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 diff --git a/controllers/custom_wizard/wizard.rb b/controllers/custom_wizard/wizard.rb index 9670fd62..e682a307 100644 --- a/controllers/custom_wizard/wizard.rb +++ b/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 && submission.redirect_to + result.merge!(redirect_to: submission.redirect_to) end wizard.final_cleanup! diff --git a/extensions/invites_controller.rb b/extensions/invites_controller.rb index cafb15bd..5e0094da 100644 --- a/extensions/invites_controller.rb +++ b/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 diff --git a/jobs/set_after_time_wizard.rb b/jobs/set_after_time_wizard.rb index 3b2e9e11..7a5b86c6 100644 --- a/jobs/set_after_time_wizard.rb +++ b/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 diff --git a/lib/custom_wizard/action.rb b/lib/custom_wizard/action.rb index 5388a326..acb8dafb 100644 --- a/lib/custom_wizard/action.rb +++ b/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,18 @@ class CustomWizard::Action end if @result.success? && @result.output.present? - data[action['id']] = @result.output + @submission.fields[action['id']] = @result.output end save_log 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 +51,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 +69,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 +83,7 @@ class CustomWizard::Action targets = CustomWizard::Mapper.new( inputs: action['recipient'], - data: data, + data: mapper_data, user: user, multiple: true ).perform @@ -115,7 +119,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 +182,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 +197,7 @@ class CustomWizard::Action mute_remainder = CustomWizard::Mapper.new( inputs: action['mute_remainder'], - data: data, + data: mapper_data, user: user ).perform @@ -202,7 +206,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 +288,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 +299,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 +349,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) - url += "&#{action['code']}=#{data[action['code']]}" + @submission.fields[action['code']] = SecureRandom.hex(8) + 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 +420,7 @@ class CustomWizard::Action def action_category output = CustomWizard::Mapper.new( inputs: action['category'], - data: data, + data: mapper_data, user: user ).perform @@ -434,7 +438,7 @@ class CustomWizard::Action def action_tags output = CustomWizard::Mapper.new( inputs: action['tags'], - data: data, + data: mapper_data, user: user, ).perform @@ -451,7 +455,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 +517,7 @@ class CustomWizard::Action params[:title] = CustomWizard::Mapper.new( inputs: action['title'], - data: data, + data: mapper_data, user: user ).perform @@ -525,7 +529,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 +552,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 +591,7 @@ class CustomWizard::Action if input.present? value = CustomWizard::Mapper.new( inputs: input, - data: data, + data: mapper_data, user: user ).perform @@ -617,7 +621,7 @@ class CustomWizard::Action if action[attr].present? value = CustomWizard::Mapper.new( inputs: action[attr], - data: data, + data: mapper_data, user: user ).perform diff --git a/lib/custom_wizard/builder.rb b/lib/custom_wizard/builder.rb index a9fc6263..b5c04c27 100644 --- a/lib/custom_wizard/builder.rb +++ b/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 || {}) - .merge(@updater.submission) - .with_indifferent_access + @submission = @wizard.current_submission || CustomWizard::Submission.new(@wizard) + @submission.fields.merge(@updater.submission) @updater.validate next if @updater.errors.any? @@ -60,13 +59,11 @@ class CustomWizard::Builder run_step_actions if @updater.errors.empty? - if route_to = @submission['route_to'] - @submission.delete('route_to') - end + route_to = @submission.route_to + @submission.route_to = nil + @submission.save - @wizard.save_submission(@submission) @updater.result[:redirect_on_next] = route_to if route_to - true else false @@ -93,7 +90,7 @@ class CustomWizard::Builder params[:value] = prefill_field(field_template, step_template) if !build_opts[:reset] && (submission = @wizard.current_submission) - 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 +133,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 +168,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? @@ -195,7 +192,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 @@ -205,7 +202,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 } @@ -275,7 +272,7 @@ class CustomWizard::Builder permitted_data = {} submission_key = nil params_key = nil - submission = @wizard.current_submission || {} + submission = @wizard.current_submission || CustomWizard::Submission.new(@wizard) permitted_params.each do |pp| pair = pp['pairs'].first @@ -283,11 +280,11 @@ class CustomWizard::Builder submission_key = pair['value'].to_sym if submission_key && params_key - submission[submission_key] = params[params_key] + submission.fields[submission_key] = params[params_key] end end - @wizard.save_submission(submission) + submission.save end def ensure_required_data(step, step_template) @@ -302,7 +299,7 @@ class CustomWizard::Builder 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) @@ -329,7 +326,7 @@ class CustomWizard::Builder CustomWizard::Action.new( action: action_template, wizard: @wizard, - data: @submission + submission: @submission ).perform end end diff --git a/lib/custom_wizard/submission.rb b/lib/custom_wizard/submission.rb new file mode 100644 index 00000000..8cb06a4a --- /dev/null +++ b/lib/custom_wizard/submission.rb @@ -0,0 +1,89 @@ +class CustomWizard::Submission + include ActiveModel::SerializerSupport + + KEY ||= "submissions" + ACTION_KEY ||= "action" + META ||= %w(submitted_at route_to redirect_on_complete redirect_to) + + attr_reader :id, + :user, + :wizard + + attr_accessor :fields + + META.each do |attr| + class_eval { attr_accessor attr } + end + + def initialize(wizard, data = {}, user_id = nil) + @wizard = wizard + + if user_id + @user = User.find_by(id: user_id) || OpenStruct.new(deleted: true, id: user_id) + else + @user = wizard.user + end + + data = data.with_indifferent_access + @id = data['id'] || SecureRandom.hex(12) + @fields = data.except(META + ['id']) || {} + + META.each do |attr| + send("#{attr}=", data[attr]) if data[attr] + end + end + + def save + return nil unless wizard.save_submissions + validate_fields + + submissions = self.class.list(wizard, user_id: user.id).select { |s| s.id != self.id } + submissions.push(self) + + submission_data = submissions.map { |s| s.fields_and_meta } + PluginStore.set("#{wizard.id}_#{KEY}", user.id, submission_data) + end + + def validate_fields + self.fields = fields.select do |key, value| + wizard.field_ids.include?(key) || key.include?(ACTION_KEY) + end + end + + def fields_and_meta + result = fields + + META.each do |attr| + if value = self.send(attr) + result[attr] = value + end + end + + result + 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 \ No newline at end of file diff --git a/lib/custom_wizard/wizard.rb b/lib/custom_wizard/wizard.rb index f92f3d61..fff7d159 100644 --- a/lib/custom_wizard/wizard.rb +++ b/lib/custom_wizard/wizard.rb @@ -29,7 +29,8 @@ class CustomWizard::Wizard :first_step, :start, :actions, - :user + :user, + :submissions def initialize(attrs = {}, user = nil) @user = user @@ -221,72 +222,38 @@ class CustomWizard::Wizard @groups ||= ::Site.new(Guardian.new(user)).groups end + def field_ids + steps.map { |step| step.fields.map { |field| field.id } }.flatten + 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") - submissions.last.with_indifferent_access - else - nil + @current_submission ||= begin + if submissions.present? + unsubmitted = submissions.select { |submission| !submission.submitted_at } + unsubmitted.present? ? unsubmitted.first : nil + else + nil + end 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! if id == user.custom_fields['redirect_to_wizard'] user.custom_fields.delete('redirect_to_wizard') user.save_custom_fields(true) end - if submission = current_submission - submission['submitted_at'] = Time.now.iso8601 - save_submission(submission) + if current_submission.present? + current_submission.submitted_at = Time.now.iso8601 + current_submission.save end end - def self.submissions(wizard_id, user) - 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) if template = CustomWizard::Template.find(wizard_id) new(template.to_h, user) @@ -339,11 +306,7 @@ class CustomWizard::Wizard end end - def self.set_submission_redirect(user, wizard_id, url) - set_submissions(wizard_id, user, [{ redirect_to: url }]) - end - - def self.set_wizard_redirect(wizard_id, user) + def self.set_user_redirect(wizard_id, user) wizard = self.create(wizard_id, user) if wizard.permitted? @@ -353,4 +316,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 || CustomWizard::Submission.new(wizard) + submission.redirect_to = url + submission.save + else + false + end + end end diff --git a/plugin.rb b/plugin.rb index a4278f65..ecbf850a 100644 --- a/plugin.rb +++ b/plugin.rb @@ -66,6 +66,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 @@ -110,7 +111,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 @@ -131,7 +132,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 @@ -142,7 +143,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}" diff --git a/serializers/custom_wizard/submission.rb b/serializers/custom_wizard/submission.rb new file mode 100644 index 00000000..bb8bc288 --- /dev/null +++ b/serializers/custom_wizard/submission.rb @@ -0,0 +1,13 @@ +class CustomWizard::SubmissionSerializer + attributes :id, + :username, + :fields, + :redirect_to, + :submitted_at + + def username + object.user.deleted ? + I18n.t('admin.wizard.submission.no_user', user_id: object.user.id) : + object.user.username + end +end \ No newline at end of file diff --git a/spec/components/custom_wizard/action_spec.rb b/spec/components/custom_wizard/action_spec.rb index 9910c0bd..6f77d617 100644 --- a/spec/components/custom_wizard/action_spec.rb +++ b/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" @@ -215,20 +215,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 +237,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( diff --git a/spec/components/custom_wizard/builder_spec.rb b/spec/components/custom_wizard/builder_spec.rb index d9d3524e..0504a238 100644 --- a/spec/components/custom_wizard/builder_spec.rb +++ b/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,12 @@ describe CustomWizard::Builder do end it 'is permitted if required data is present' do - CustomWizard::Wizard.set_submissions('super_mega_fun_wizard', user, - required_data: "required_value" - ) + wizard = CustomWizard::Wizard.create('super_mega_fun_wizard', user) + data = { + step_1_field_1: 'I am a user submission' + } + CustomWizard::Submission.new(wizard, data).save + expect( CustomWizard::Builder.new(@template[:id], user).build .steps.first @@ -336,31 +342,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( - CustomWizard::Wizard.submissions(@template[:id], user) - .first['step_1_field_1'] - ).to eq('Text input') + expect(@wizard.current_submission.fields['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( - CustomWizard::Wizard.submissions(@template[:id], user).first - ).to eq(nil) + expect(@wizard.current_submission).to eq(nil) end end end diff --git a/spec/components/custom_wizard/submission_spec.rb b/spec/components/custom_wizard/submission_spec.rb new file mode 100644 index 00000000..798a2969 --- /dev/null +++ b/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(template_json["id"], 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).size).to eq(1) + end +end \ No newline at end of file diff --git a/spec/components/custom_wizard/wizard_spec.rb b/spec/components/custom_wizard/wizard_spec.rb index 9808f32f..376fce2c 100644 --- a/spec/components/custom_wizard/wizard_spec.rb +++ b/spec/components/custom_wizard/wizard_spec.rb @@ -204,14 +204,7 @@ describe CustomWizard::Wizard do context "submissions" do before do - @wizard.set_submissions(step_1_field_1: 'I am a user submission') - 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') + CustomWizard::Submission.new(@wizard, step_1_field_1: "I am a user submission") end it "lists the user's submissions" do @@ -219,20 +212,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) @@ -273,7 +256,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") @@ -281,7 +264,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) diff --git a/spec/requests/custom_wizard/admin/submissions_controller_spec.rb b/spec/requests/custom_wizard/admin/submissions_controller_spec.rb index f63eead5..eae783aa 100644 --- a/spec/requests/custom_wizard/admin/submissions_controller_spec.rb +++ b/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( @@ -14,33 +15,35 @@ describe CustomWizard::AdminSubmissionsController do before do CustomWizard::Template.save(template, skip_jobs: true) - CustomWizard::Wizard.set_submissions(template['id'], user1, - step_1_field_1: "I am a user1's submission" - ) - CustomWizard::Wizard.set_submissions(template['id'], user2, - step_1_field_1: "I am a user2's submission" - ) + + template_2 = template.dup + template_2["id"] = "super_mega_fun_wizard_2" + CustomWizard::Template.save(template_2, skip_jobs: true) + + wizard1 = CustomWizard::Wizard.create(template["id"], user1) + wizard2 = CustomWizard::Wizard.create(template["id"], user2) + 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(3) 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 - get "/admin/wizards/submissions/#{template['id']}.json" - expect(response.parsed_body['submissions'].length).to eq(2) - end - - it "downloads all user submissions" do - get "/admin/wizards/submissions/#{template['id']}/download" - expect(response.parsed_body.length).to eq(2) + it "downloads submissions" do + get "/admin/wizards/submissions/#{template_2['id']}/download" + expect(response.parsed_body.length).to eq(1) end end diff --git a/spec/requests/custom_wizard/steps_controller_spec.rb b/spec/requests/custom_wizard/steps_controller_spec.rb index 2424274b..ab705cac 100644 --- a/spec/requests/custom_wizard/steps_controller_spec.rb +++ b/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,7 +175,7 @@ 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.current_submission.fields['action_9'] group = Group.find_by(name: group_name) expect(group.full_name).to eq("My cool group") end @@ -275,7 +275,7 @@ describe CustomWizard::StepsController do wizard_id = response.parsed_body['wizard']['id'] wizard = CustomWizard::Wizard.create(wizard_id, user) - submission = wizard.submissions.last - expect(submission.keys).not_to include("step_2_field_1") + submission = wizard.current_submission + expect(submission.fields.keys).not_to include("step_2_field_1") end end diff --git a/spec/requests/custom_wizard/wizard_controller_spec.rb b/spec/requests/custom_wizard/wizard_controller_spec.rb index 4380bc73..7a977539 100644 --- a/spec/requests/custom_wizard/wizard_controller_spec.rb +++ b/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, - redirect_to: '/t/2' - ) + @wizard = CustomWizard::Wizard.create(@template["id"], user) + CustomWizard::Submission.new(@wizard, step_1_field_1: "I am a user submission").save put '/w/super-mega-fun-wizard/skip.json' expect(response.parsed_body['redirect_to']).to eq('/t/2') end From e441588aa3d9f1bbeb3e4abafb642bffc514135d Mon Sep 17 00:00:00 2001 From: angusmcleod Date: Wed, 23 Jun 2021 16:13:58 +1000 Subject: [PATCH 2/8] Fix specs and tighten conditional handling --- controllers/custom_wizard/steps.rb | 12 +++- controllers/custom_wizard/wizard.rb | 2 +- lib/custom_wizard/action.rb | 3 + lib/custom_wizard/action_result.rb | 2 +- lib/custom_wizard/builder.rb | 23 ++++--- lib/custom_wizard/submission.rb | 51 ++++++++++---- lib/custom_wizard/wizard.rb | 66 ++++++++++++++++--- plugin.rb | 1 + serializers/custom_wizard/submission.rb | 13 ---- .../custom_wizard/submission_serializer.rb | 16 +++++ spec/components/custom_wizard/action_spec.rb | 3 +- spec/components/custom_wizard/builder_spec.rb | 9 +-- .../custom_wizard/submission_spec.rb | 4 +- spec/components/custom_wizard/wizard_spec.rb | 8 +-- spec/fixtures/step/required_data.json | 4 +- .../admin/submissions_controller_spec.rb | 11 ++-- .../application_controller_spec.rb | 4 +- .../custom_wizard/steps_controller_spec.rb | 5 +- .../custom_wizard/wizard_controller_spec.rb | 2 +- 19 files changed, 167 insertions(+), 72 deletions(-) delete mode 100644 serializers/custom_wizard/submission.rb create mode 100644 serializers/custom_wizard/submission_serializer.rb diff --git a/controllers/custom_wizard/steps.rb b/controllers/custom_wizard/steps.rb index 9c0fbf92..66ec2da9 100644 --- a/controllers/custom_wizard/steps.rb +++ b/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 @@ -36,11 +36,15 @@ 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 @@ -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 diff --git a/controllers/custom_wizard/wizard.rb b/controllers/custom_wizard/wizard.rb index e682a307..fd93ef15 100644 --- a/controllers/custom_wizard/wizard.rb +++ b/controllers/custom_wizard/wizard.rb @@ -64,7 +64,7 @@ class CustomWizard::WizardController < ::ApplicationController if user && wizard.can_access? submission = wizard.current_submission - if submission && submission.redirect_to + if submission.present? && submission.redirect_to result.merge!(redirect_to: submission.redirect_to) end diff --git a/lib/custom_wizard/action.rb b/lib/custom_wizard/action.rb index acb8dafb..1b5770d7 100644 --- a/lib/custom_wizard/action.rb +++ b/lib/custom_wizard/action.rb @@ -30,6 +30,9 @@ class CustomWizard::Action end save_log + + @result.submission = @submission + @result end def mapper_data diff --git a/lib/custom_wizard/action_result.rb b/lib/custom_wizard/action_result.rb index 07c81284..53484ceb 100644 --- a/lib/custom_wizard/action_result.rb +++ b/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 diff --git a/lib/custom_wizard/builder.rb b/lib/custom_wizard/builder.rb index b5c04c27..3dd881e7 100644 --- a/lib/custom_wizard/builder.rb +++ b/lib/custom_wizard/builder.rb @@ -47,8 +47,8 @@ class CustomWizard::Builder step.on_update do |updater| @updater = updater - @submission = @wizard.current_submission || CustomWizard::Submission.new(@wizard) - @submission.fields.merge(@updater.submission) + @submission = @wizard.current_submission + @submission.fields.merge!(@updater.submission) @updater.validate next if @updater.errors.any? @@ -63,7 +63,9 @@ class CustomWizard::Builder @submission.route_to = nil @submission.save + @wizard.update! @updater.result[:redirect_on_next] = route_to if route_to + true else false @@ -72,7 +74,7 @@ class CustomWizard::Builder end end - @wizard.update_step_order! + @wizard.update! @wizard end @@ -89,7 +91,7 @@ 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.fields[field_template['id']] if submission.fields[field_template['id']] end @@ -272,14 +274,15 @@ class CustomWizard::Builder permitted_data = {} submission_key = nil params_key = nil - submission = @wizard.current_submission || CustomWizard::Submission.new(@wizard) + 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.permitted_param_keys << submission_key.to_s submission.fields[submission_key] = params[params_key] end end @@ -293,7 +296,7 @@ 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 @@ -323,11 +326,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, submission: @submission ).perform + + if result.success? + @submission = result.submission + end end end end diff --git a/lib/custom_wizard/submission.rb b/lib/custom_wizard/submission.rb index 8cb06a4a..b937363d 100644 --- a/lib/custom_wizard/submission.rb +++ b/lib/custom_wizard/submission.rb @@ -2,14 +2,15 @@ class CustomWizard::Submission include ActiveModel::SerializerSupport KEY ||= "submissions" - ACTION_KEY ||= "action" META ||= %w(submitted_at route_to redirect_on_complete redirect_to) attr_reader :id, :user, + :user_id, :wizard - - attr_accessor :fields + + attr_accessor :fields, + :permitted_param_keys META.each do |attr| class_eval { attr_accessor attr } @@ -17,9 +18,10 @@ class CustomWizard::Submission def initialize(wizard, data = {}, user_id = nil) @wizard = wizard + @user_id = user_id if user_id - @user = User.find_by(id: user_id) || OpenStruct.new(deleted: true, id: user_id) + @user = User.find_by(id: user_id) else @user = wizard.user end @@ -31,25 +33,32 @@ class CustomWizard::Submission 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_fields + validate - submissions = self.class.list(wizard, user_id: user.id).select { |s| s.id != self.id } + 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 { |s| s.fields_and_meta } + submission_data = submissions.map { |submission| data_to_save(submission) } PluginStore.set("#{wizard.id}_#{KEY}", user.id, submission_data) end - def validate_fields - self.fields = fields.select do |key, value| - wizard.field_ids.include?(key) || key.include?(ACTION_KEY) - 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 @@ -62,6 +71,24 @@ class CustomWizard::Submission 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) diff --git a/lib/custom_wizard/wizard.rb b/lib/custom_wizard/wizard.rb index fff7d159..8f5a897f 100644 --- a/lib/custom_wizard/wizard.rb +++ b/lib/custom_wizard/wizard.rb @@ -26,12 +26,16 @@ class CustomWizard::Wizard :needs_groups, :steps, :step_ids, + :field_ids, :first_step, :start, :actions, + :action_ids, :user, :submissions + attr_reader :all_step_ids + def initialize(attrs = {}, user = nil) @user = user attrs = attrs.with_indifferent_access @@ -59,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) @@ -84,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| @@ -103,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 @@ -118,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 @@ -222,8 +249,25 @@ class CustomWizard::Wizard @groups ||= ::Site.new(Guardian.new(user)).groups end - def field_ids - steps.map { |step| step.fields.map { |field| field.id } }.flatten + 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 @@ -235,9 +279,9 @@ class CustomWizard::Wizard @current_submission ||= begin if submissions.present? unsubmitted = submissions.select { |submission| !submission.submitted_at } - unsubmitted.present? ? unsubmitted.first : nil + unsubmitted.present? ? unsubmitted.first : CustomWizard::Submission.new(self) else - nil + CustomWizard::Submission.new(self) end end end @@ -252,6 +296,8 @@ class CustomWizard::Wizard current_submission.submitted_at = Time.now.iso8601 current_submission.save end + + update! end def self.create(wizard_id, user = nil) @@ -321,7 +367,7 @@ class CustomWizard::Wizard wizard = self.create(wizard_id, user) if wizard.permitted? - submission = wizard.current_submission || CustomWizard::Submission.new(wizard) + submission = wizard.current_submission submission.redirect_to = url submission.save else diff --git a/plugin.rb b/plugin.rb index ecbf850a..b335971c 100644 --- a/plugin.rb +++ b/plugin.rb @@ -86,6 +86,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 diff --git a/serializers/custom_wizard/submission.rb b/serializers/custom_wizard/submission.rb deleted file mode 100644 index bb8bc288..00000000 --- a/serializers/custom_wizard/submission.rb +++ /dev/null @@ -1,13 +0,0 @@ -class CustomWizard::SubmissionSerializer - attributes :id, - :username, - :fields, - :redirect_to, - :submitted_at - - def username - object.user.deleted ? - I18n.t('admin.wizard.submission.no_user', user_id: object.user.id) : - object.user.username - end -end \ No newline at end of file diff --git a/serializers/custom_wizard/submission_serializer.rb b/serializers/custom_wizard/submission_serializer.rb new file mode 100644 index 00000000..e150273d --- /dev/null +++ b/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 \ No newline at end of file diff --git a/spec/components/custom_wizard/action_spec.rb b/spec/components/custom_wizard/action_spec.rb index 6f77d617..8b617c39 100644 --- a/spec/components/custom_wizard/action_spec.rb +++ b/spec/components/custom_wizard/action_spec.rb @@ -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 diff --git a/spec/components/custom_wizard/builder_spec.rb b/spec/components/custom_wizard/builder_spec.rb index 0504a238..8e80d806 100644 --- a/spec/components/custom_wizard/builder_spec.rb +++ b/spec/components/custom_wizard/builder_spec.rb @@ -257,10 +257,7 @@ describe CustomWizard::Builder do it 'is permitted if required data is present' do wizard = CustomWizard::Wizard.create('super_mega_fun_wizard', user) - data = { - step_1_field_1: 'I am a user submission' - } - CustomWizard::Submission.new(wizard, data).save + CustomWizard::Submission.new(wizard, step_1_field_1: "required").save expect( CustomWizard::Builder.new(@template[:id], user).build @@ -280,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 @@ -362,7 +359,7 @@ describe CustomWizard::Builder do it "does not save submissions" do perform_update('step_1', step_1_field_1: 'Text input') - expect(@wizard.current_submission).to eq(nil) + expect(@wizard.current_submission.present?).to eq(false) end end end diff --git a/spec/components/custom_wizard/submission_spec.rb b/spec/components/custom_wizard/submission_spec.rb index 798a2969..23855daf 100644 --- a/spec/components/custom_wizard/submission_spec.rb +++ b/spec/components/custom_wizard/submission_spec.rb @@ -29,7 +29,7 @@ describe CustomWizard::Submission do it "saves a user's submission" do expect( - described_class.get(template_json["id"], user.id).fields["step_1_field_1"] + described_class.get(@wizard, user.id).fields["step_1_field_1"] ).to eq("I am a user submission") end @@ -38,6 +38,6 @@ describe CustomWizard::Submission do end it "list submissions by wizard and user" do - expect(described_class.list(@wizard, user).size).to eq(1) + expect(described_class.list(@wizard, user_id: user.id).size).to eq(1) end end \ No newline at end of file diff --git a/spec/components/custom_wizard/wizard_spec.rb b/spec/components/custom_wizard/wizard_spec.rb index 376fce2c..1c38ab1f 100644 --- a/spec/components/custom_wizard/wizard_spec.rb +++ b/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") @@ -204,7 +204,7 @@ describe CustomWizard::Wizard do context "submissions" do before do - CustomWizard::Submission.new(@wizard, 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 "lists the user's submissions" do diff --git a/spec/fixtures/step/required_data.json b/spec/fixtures/step/required_data.json index 9f65d516..7ff8bcaf 100644 --- a/spec/fixtures/step/required_data.json +++ b/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" } diff --git a/spec/requests/custom_wizard/admin/submissions_controller_spec.rb b/spec/requests/custom_wizard/admin/submissions_controller_spec.rb index eae783aa..36296e95 100644 --- a/spec/requests/custom_wizard/admin/submissions_controller_spec.rb +++ b/spec/requests/custom_wizard/admin/submissions_controller_spec.rb @@ -13,11 +13,14 @@ 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) - - template_2 = template.dup - template_2["id"] = "super_mega_fun_wizard_2" CustomWizard::Template.save(template_2, skip_jobs: true) wizard1 = CustomWizard::Wizard.create(template["id"], user1) @@ -33,7 +36,7 @@ describe CustomWizard::AdminSubmissionsController do it "returns a list of wizards" do get "/admin/wizards/submissions.json" - expect(response.parsed_body.length).to eq(3) + expect(response.parsed_body.length).to eq(2) expect(response.parsed_body.first['id']).to eq(template['id']) end diff --git a/spec/requests/custom_wizard/application_controller_spec.rb b/spec/requests/custom_wizard/application_controller_spec.rb index e8b45c48..9f5a1a60 100644 --- a/spec/requests/custom_wizard/application_controller_spec.rb +++ b/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) - .first['redirect_to'] + CustomWizard::Wizard.create(@template['id'], user).submissions + .first.fields['redirect_to'] ).to eq("/t/2") end diff --git a/spec/requests/custom_wizard/steps_controller_spec.rb b/spec/requests/custom_wizard/steps_controller_spec.rb index ab705cac..5da75d8d 100644 --- a/spec/requests/custom_wizard/steps_controller_spec.rb +++ b/spec/requests/custom_wizard/steps_controller_spec.rb @@ -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.current_submission.fields['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 diff --git a/spec/requests/custom_wizard/wizard_controller_spec.rb b/spec/requests/custom_wizard/wizard_controller_spec.rb index 7a977539..f2000bda 100644 --- a/spec/requests/custom_wizard/wizard_controller_spec.rb +++ b/spec/requests/custom_wizard/wizard_controller_spec.rb @@ -72,7 +72,7 @@ describe CustomWizard::WizardController do it 'skip response contains a redirect_to if in users submissions' do @wizard = CustomWizard::Wizard.create(@template["id"], user) - CustomWizard::Submission.new(@wizard, step_1_field_1: "I am a user submission").save + 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 From 098e8418fbe90c45365988f46f6938a7c6663c02 Mon Sep 17 00:00:00 2001 From: angusmcleod Date: Wed, 23 Jun 2021 16:15:17 +1000 Subject: [PATCH 3/8] Apply rubocop --- controllers/custom_wizard/admin/submissions.rb | 2 +- lib/custom_wizard/submission.rb | 5 +++-- serializers/custom_wizard/submission_serializer.rb | 2 +- spec/components/custom_wizard/submission_spec.rb | 2 +- 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/controllers/custom_wizard/admin/submissions.rb b/controllers/custom_wizard/admin/submissions.rb index 682c4030..4cb2a0e4 100644 --- a/controllers/custom_wizard/admin/submissions.rb +++ b/controllers/custom_wizard/admin/submissions.rb @@ -23,7 +23,7 @@ class CustomWizard::AdminSubmissionsController < CustomWizard::AdminController content_type: "application/json", disposition: "attachment" end - + protected def ordered_submissions diff --git a/lib/custom_wizard/submission.rb b/lib/custom_wizard/submission.rb index b937363d..47d2581a 100644 --- a/lib/custom_wizard/submission.rb +++ b/lib/custom_wizard/submission.rb @@ -1,3 +1,4 @@ +# frozen_string_literal: true class CustomWizard::Submission include ActiveModel::SerializerSupport @@ -23,7 +24,7 @@ class CustomWizard::Submission if user_id @user = User.find_by(id: user_id) else - @user = wizard.user + @user = wizard.user end data = data.with_indifferent_access @@ -113,4 +114,4 @@ class CustomWizard::Submission result end -end \ No newline at end of file +end diff --git a/serializers/custom_wizard/submission_serializer.rb b/serializers/custom_wizard/submission_serializer.rb index e150273d..52f0cb32 100644 --- a/serializers/custom_wizard/submission_serializer.rb +++ b/serializers/custom_wizard/submission_serializer.rb @@ -13,4 +13,4 @@ class CustomWizard::SubmissionSerializer < ApplicationSerializer object.user.username : I18n.t('admin.wizard.submission.no_user', user_id: object.user_id) end -end \ No newline at end of file +end diff --git a/spec/components/custom_wizard/submission_spec.rb b/spec/components/custom_wizard/submission_spec.rb index 23855daf..a8c33861 100644 --- a/spec/components/custom_wizard/submission_spec.rb +++ b/spec/components/custom_wizard/submission_spec.rb @@ -40,4 +40,4 @@ describe CustomWizard::Submission do it "list submissions by wizard and user" do expect(described_class.list(@wizard, user_id: user.id).size).to eq(1) end -end \ No newline at end of file +end From 39ee61b42283a467731f68102435914963d28551 Mon Sep 17 00:00:00 2001 From: angusmcleod Date: Wed, 23 Jun 2021 16:17:18 +1000 Subject: [PATCH 4/8] Update coverage --- coverage/.last_run.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coverage/.last_run.json b/coverage/.last_run.json index 3e7f27f6..2d4d0378 100644 --- a/coverage/.last_run.json +++ b/coverage/.last_run.json @@ -1,5 +1,5 @@ { "result": { - "line": 90.52 + "line": 91.83 } } From 3d9f6aac9894ff3bf7a8be10ac2c8d3e4a19109d Mon Sep 17 00:00:00 2001 From: angusmcleod Date: Wed, 23 Jun 2021 17:02:21 +1000 Subject: [PATCH 5/8] Ensure data is not nil --- lib/custom_wizard/submission.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/custom_wizard/submission.rb b/lib/custom_wizard/submission.rb index 47d2581a..a1f93d82 100644 --- a/lib/custom_wizard/submission.rb +++ b/lib/custom_wizard/submission.rb @@ -27,7 +27,7 @@ class CustomWizard::Submission @user = wizard.user end - data = data.with_indifferent_access + data = (data || {}).with_indifferent_access @id = data['id'] || SecureRandom.hex(12) @fields = data.except(META + ['id']) || {} From 56f58414b3e24f398a525824760e8934c9a102b3 Mon Sep 17 00:00:00 2001 From: angusmcleod Date: Mon, 12 Jul 2021 15:53:58 +0800 Subject: [PATCH 6/8] Remove meta keys from fields attribute and update submissions ui to handle new submission structure --- .../admin-wizards-submissions-show.js.es6 | 26 ++++++++++++++----- lib/custom_wizard/submission.rb | 3 ++- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/assets/javascripts/discourse/routes/admin-wizards-submissions-show.js.es6 b/assets/javascripts/discourse/routes/admin-wizards-submissions-show.js.es6 index abcaeb9e..7d8e2cf5 100644 --- a/assets/javascripts/discourse/routes/admin-wizards-submissions-show.js.es6 +++ b/assets/javascripts/discourse/routes/admin-wizards-submissions-show.js.es6 @@ -1,17 +1,25 @@ 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) { - let fields = []; + if (model && model.submissions) { + let fields = [ + 'username' + ]; model.submissions.forEach((s) => { - Object.keys(s).forEach((k) => { - if (fields.indexOf(k) < 0) { + Object.keys(s.fields).forEach((k) => { + if (!excludedMetaFields.includes(k) && fields.indexOf(k) < 0) { fields.push(k); } }); @@ -19,9 +27,13 @@ export default DiscourseRoute.extend({ let submissions = []; model.submissions.forEach((s) => { - let submission = {}; - fields.forEach((f) => { - submission[f] = s[f]; + let submission = { + username: s.username + }; + Object.keys(s.fields).forEach((f) => { + if (fields.includes(f)) { + submission[f] = s.fields[f]; + } }); submissions.push(submission); }); diff --git a/lib/custom_wizard/submission.rb b/lib/custom_wizard/submission.rb index a1f93d82..e50cb259 100644 --- a/lib/custom_wizard/submission.rb +++ b/lib/custom_wizard/submission.rb @@ -29,7 +29,8 @@ class CustomWizard::Submission data = (data || {}).with_indifferent_access @id = data['id'] || SecureRandom.hex(12) - @fields = data.except(META + ['id']) || {} + non_field_keys = META + ['id'] + @fields = data.except(*non_field_keys) || {} META.each do |attr| send("#{attr}=", data[attr]) if data[attr] From dfd382cd8affc344ff4049cec695580df35dc8b1 Mon Sep 17 00:00:00 2001 From: angusmcleod Date: Mon, 12 Jul 2021 16:01:52 +0800 Subject: [PATCH 7/8] Apply prettier --- .../routes/admin-wizards-submissions-show.js.es6 | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/assets/javascripts/discourse/routes/admin-wizards-submissions-show.js.es6 b/assets/javascripts/discourse/routes/admin-wizards-submissions-show.js.es6 index 7d8e2cf5..73168ff3 100644 --- a/assets/javascripts/discourse/routes/admin-wizards-submissions-show.js.es6 +++ b/assets/javascripts/discourse/routes/admin-wizards-submissions-show.js.es6 @@ -1,11 +1,7 @@ import CustomWizard from "../models/custom-wizard"; import DiscourseRoute from "discourse/routes/discourse"; -const excludedMetaFields = [ - 'route_to', - 'redirect_on_complete', - 'redirect_to' -]; +const excludedMetaFields = ["route_to", "redirect_on_complete", "redirect_to"]; export default DiscourseRoute.extend({ model(params) { @@ -14,9 +10,7 @@ export default DiscourseRoute.extend({ setupController(controller, model) { if (model && model.submissions) { - let fields = [ - 'username' - ]; + let fields = ["username"]; model.submissions.forEach((s) => { Object.keys(s.fields).forEach((k) => { if (!excludedMetaFields.includes(k) && fields.indexOf(k) < 0) { @@ -28,7 +22,7 @@ export default DiscourseRoute.extend({ let submissions = []; model.submissions.forEach((s) => { let submission = { - username: s.username + username: s.username, }; Object.keys(s.fields).forEach((f) => { if (fields.includes(f)) { From d602281a6a8849b0e9284015c70e72967c01e52d Mon Sep 17 00:00:00 2001 From: angusmcleod Date: Mon, 12 Jul 2021 16:39:37 +0800 Subject: [PATCH 8/8] Fix failing test --- spec/requests/custom_wizard/application_controller_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/requests/custom_wizard/application_controller_spec.rb b/spec/requests/custom_wizard/application_controller_spec.rb index 9f5a1a60..0835f246 100644 --- a/spec/requests/custom_wizard/application_controller_spec.rb +++ b/spec/requests/custom_wizard/application_controller_spec.rb @@ -40,7 +40,7 @@ describe ApplicationController do get '/', headers: { 'REFERER' => "/t/2" } expect( CustomWizard::Wizard.create(@template['id'], user).submissions - .first.fields['redirect_to'] + .first.redirect_to ).to eq("/t/2") end