From a6354e68026d5bfc0492f252d263cb3721dcd861 Mon Sep 17 00:00:00 2001 From: Ryan Arakawa Date: Tue, 12 Aug 2025 16:41:51 -0500 Subject: [PATCH] add ability to request changes for a completed submission * new controller to handle change requests * add button and modal on completed submission view to request changes * webhook job will send out to external API when submission is updated for changes_requested_at * email will be sent to user that need to make changes * submission status steps back from "completed" --- .rubocop.yml | 1 + .../submitters_request_changes_controller.rb | 58 ++++++++++++++++++ ...m_changes_requested_webhook_request_job.rb | 33 ++++++++++ app/mailers/submitter_mailer.rb | 17 ++++++ app/models/submission_event.rb | 1 + app/models/submitter.rb | 45 +++++++------- app/services/export_submission_service.rb | 2 + app/views/submissions/show.html.erb | 17 ++++++ .../changes_requested_email.html.erb | 10 +++ .../request_changes.html.erb | 34 +++++++++++ config/locales/i18n.yml | 1 + config/routes.rb | 5 ++ ..._add_changes_requested_at_to_submitters.rb | 5 ++ db/schema.rb | 3 +- lib/submitters/submit_values.rb | 1 + ...nges_requested_webhook_request_job_spec.rb | 61 +++++++++++++++++++ spec/mailers/submitter_mailer_spec.rb | 40 ++++++++++++ spec/models/submitter_spec.rb | 21 +++++++ ...mitters_request_changes_controller_spec.rb | 56 +++++++++++++++++ .../export_submission_service_spec.rb | 24 ++++++++ 20 files changed, 413 insertions(+), 22 deletions(-) create mode 100644 app/controllers/submitters_request_changes_controller.rb create mode 100644 app/jobs/send_form_changes_requested_webhook_request_job.rb create mode 100644 app/views/submitter_mailer/changes_requested_email.html.erb create mode 100644 app/views/submitters_request_changes/request_changes.html.erb create mode 100644 db/migrate/20250811211829_add_changes_requested_at_to_submitters.rb create mode 100644 spec/jobs/send_form_changes_requested_webhook_request_job_spec.rb create mode 100644 spec/mailers/submitter_mailer_spec.rb create mode 100644 spec/requests/submitters_request_changes_controller_spec.rb diff --git a/.rubocop.yml b/.rubocop.yml index a679c86a..faa4fb01 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -6,6 +6,7 @@ require: AllCops: NewCops: enable Exclude: + - db/migrate/**/* - db/schema.rb - node_modules/**/* - bin/* diff --git a/app/controllers/submitters_request_changes_controller.rb b/app/controllers/submitters_request_changes_controller.rb new file mode 100644 index 00000000..da39fc28 --- /dev/null +++ b/app/controllers/submitters_request_changes_controller.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +class SubmittersRequestChangesController < ApplicationController + before_action :load_submitter + skip_before_action :verify_authenticity_token, only: :request_changes + + def request_changes + if request.get? + render 'submitters_request_changes/request_changes', layout: false if request.xhr? + else + return redirect_back(fallback_location: root_path, alert: 'Invalid request') unless can_request_changes? + + ApplicationRecord.transaction do + @submitter.update!( + changes_requested_at: Time.current, + completed_at: nil + ) + + SubmissionEvents.create_with_tracking_data( + @submitter, + 'request_changes', + request, + { reason: params[:reason], requested_by: current_user.id } + ) + end + + if @submitter.email.present? + SubmitterMailer.changes_requested_email(@submitter, current_user, params[:reason]).deliver! + end + + WebhookUrls.for_account_id(@submitter.account_id, 'form.changes_requested').each do |webhook_url| + SendFormChangesRequestedWebhookRequestJob.perform_async( + 'submitter_id' => @submitter.id, + 'webhook_url_id' => webhook_url.id + ) + end + + redirect_back(fallback_location: submission_path(@submitter.submission), + notice: 'Changes have been requested and the submitter has been notified.') + end + end + + private + + def load_submitter + @submitter = Submitter.find_by!(slug: params[:slug]) + authorize! :read, @submitter + end + + def can_request_changes? + # Only the user who created the submission can request changes + # Only for completed submissions that haven't been declined + current_user == @submitter.submission.created_by_user && + @submitter.completed_at? && + !@submitter.declined_at? && + !@submitter.changes_requested_at? + end +end diff --git a/app/jobs/send_form_changes_requested_webhook_request_job.rb b/app/jobs/send_form_changes_requested_webhook_request_job.rb new file mode 100644 index 00000000..79bf345e --- /dev/null +++ b/app/jobs/send_form_changes_requested_webhook_request_job.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +class SendFormChangesRequestedWebhookRequestJob + include Sidekiq::Job + + sidekiq_options queue: :webhooks + + MAX_ATTEMPTS = 10 + + def perform(params = {}) + submitter = Submitter.find(params['submitter_id']) + webhook_url = WebhookUrl.find(params['webhook_url_id']) + + attempt = params['attempt'].to_i + + return if webhook_url.url.blank? || webhook_url.events.exclude?('form.changes_requested') + + ActiveStorage::Current.url_options = Docuseal.default_url_options + + resp = SendWebhookRequest.call(webhook_url, event_type: 'form.changes_requested', + data: Submitters::SerializeForWebhook.call(submitter)) + + if (resp.nil? || resp.status.to_i >= 400) && attempt <= MAX_ATTEMPTS && + (!Docuseal.multitenant? || submitter.account.account_configs.exists?(key: :plan)) + SendFormChangesRequestedWebhookRequestJob.perform_in((2**attempt).minutes, { + 'submitter_id' => submitter.id, + 'webhook_url_id' => webhook_url.id, + 'attempt' => attempt + 1, + 'last_status' => resp&.status.to_i + }) + end + end +end diff --git a/app/mailers/submitter_mailer.rb b/app/mailers/submitter_mailer.rb index c4627969..36716915 100644 --- a/app/mailers/submitter_mailer.rb +++ b/app/mailers/submitter_mailer.rb @@ -104,6 +104,23 @@ class SubmitterMailer < ApplicationMailer end end + def changes_requested_email(submitter, user, reason) + @current_account = submitter.submission.account + @submitter = submitter + @submission = submitter.submission + @user = user + @reason = reason + + assign_message_metadata('submitter_changes_requested', @submitter) + + I18n.with_locale(@current_account.locale) do + mail(from: from_address_for_submitter(submitter), + to: @submitter.friendly_name, + reply_to: user.friendly_name, + subject: "Changes requested for #{(@submission.name || @submission.template.name).truncate(20)}") + end + end + def documents_copy_email(submitter, to: nil, sig: false) @current_account = submitter.submission.account @submitter = submitter diff --git a/app/models/submission_event.rb b/app/models/submission_event.rb index c18b3b97..ec02ffb3 100644 --- a/app/models/submission_event.rb +++ b/app/models/submission_event.rb @@ -54,6 +54,7 @@ class SubmissionEvent < ApplicationRecord invite_party: 'invite_party', complete_form: 'complete_form', decline_form: 'decline_form', + request_changes: 'request_changes', api_complete_form: 'api_complete_form' }, scope: false diff --git a/app/models/submitter.rb b/app/models/submitter.rb index 22ecbb19..67ba8582 100644 --- a/app/models/submitter.rb +++ b/app/models/submitter.rb @@ -4,27 +4,28 @@ # # Table name: submitters # -# id :bigint not null, primary key -# completed_at :datetime -# declined_at :datetime -# email :string -# ip :string -# metadata :text not null -# name :string -# opened_at :datetime -# phone :string -# preferences :text not null -# sent_at :datetime -# slug :string not null -# timezone :string -# ua :string -# uuid :string not null -# values :text not null -# created_at :datetime not null -# updated_at :datetime not null -# account_id :integer not null -# external_id :string -# submission_id :integer not null +# id :bigint not null, primary key +# changes_requested_at :datetime +# completed_at :datetime +# declined_at :datetime +# email :string +# ip :string +# metadata :text not null +# name :string +# opened_at :datetime +# phone :string +# preferences :text not null +# sent_at :datetime +# slug :string not null +# timezone :string +# ua :string +# uuid :string not null +# values :text not null +# created_at :datetime not null +# updated_at :datetime not null +# account_id :integer not null +# external_id :string +# submission_id :integer not null # # Indexes # @@ -73,6 +74,8 @@ class Submitter < ApplicationRecord def status if declined_at? 'declined' + elsif changes_requested_at? + 'changes_requested' elsif completed_at? 'completed' elsif opened_at? diff --git a/app/services/export_submission_service.rb b/app/services/export_submission_service.rb index d717dc3a..39d5d8a8 100644 --- a/app/services/export_submission_service.rb +++ b/app/services/export_submission_service.rb @@ -67,6 +67,8 @@ class ExportSubmissionService < ExportService 'declined' elsif statuses.all?('completed') 'completed' + elsif statuses.include?('changes_requested') + 'changes_requested' elsif statuses.any?('opened') 'in_progress' elsif statuses.any?('sent') diff --git a/app/views/submissions/show.html.erb b/app/views/submissions/show.html.erb index ded86629..f0f74f01 100644 --- a/app/views/submissions/show.html.erb +++ b/app/views/submissions/show.html.erb @@ -180,6 +180,8 @@ <% if submitter&.declined_at? %> <%= t('declined_on_time', time: l(submitter.declined_at.in_time_zone(@submission.account.timezone), format: :short, locale: @submission.account.locale)) %> + <% elsif submitter&.changes_requested_at? %> + Changes requested on <%= l(submitter.changes_requested_at.in_time_zone(@submission.account.timezone), format: :short, locale: @submission.account.locale) %> <% elsif submitter %> <% if submitter.completed_at? %> <%= l(submitter.completed_at.in_time_zone(@submission.account.timezone), format: :long, locale: @submission.account.locale) %> @@ -205,6 +207,14 @@ <% end %> + <% if submitter&.changes_requested_at? %> +
+ + Changes requested: + <%= simple_format(h(submitter.submission_events.find_by(event_type: :request_changes).data['reason'])) %> + +
+ <% end %> <% if signed_in? && submitter && submitter.email && !submitter.completed_at && !@submission.archived_at? && can?(:update, @submission) && Accounts.can_send_emails?(current_account) && !@submission.expired? && !submitter.declined_at? %>
<%= button_to button_title(title: submitter.sent_at? ? t('re_send_email') : t('send_email'), disabled_with: t('sending')), submitter_send_email_index_path(submitter_slug: submitter.slug), class: 'btn btn-sm btn-primary w-full' %> @@ -225,6 +235,13 @@ <%= button_to t('resubmit'), submitters_resubmit_path(submitter), method: :put, class: 'btn btn-sm btn-primary w-full', form: { target: '_blank' }, data: { turbo: false } %>
<% end %> + <% if signed_in? && submitter && submitter.completed_at? && !submitter.declined_at? && !submitter.changes_requested_at? && current_user == @submission.created_by_user %> +
+ <%= link_to 'Request Changes', request_changes_submitter_path(submitter.slug), + class: 'btn btn-sm btn-warning w-full', + data: { turbo_frame: :modal } %> +
+ <% end %>
diff --git a/app/views/submitter_mailer/changes_requested_email.html.erb b/app/views/submitter_mailer/changes_requested_email.html.erb new file mode 100644 index 00000000..a71159f3 --- /dev/null +++ b/app/views/submitter_mailer/changes_requested_email.html.erb @@ -0,0 +1,10 @@ +

Hello <%= @submitter.name || @submitter.email %>,

+ +

<%= [@user.first_name, @user.last_name].join(' ').strip.presence || @user.email %> has requested changes to your submission for "<%= @submission.name || @submission.template.name %>".

+ +

Message from <%= [@user.first_name, @user.last_name].join(' ').strip.presence || @user.email %>:

+<%= simple_format(h(@reason)) %> + +

To make the requested changes, please log in to your account and resubmit the form.

+ +

If you have any questions, please reply to this email.

diff --git a/app/views/submitters_request_changes/request_changes.html.erb b/app/views/submitters_request_changes/request_changes.html.erb new file mode 100644 index 00000000..ed8fb4c1 --- /dev/null +++ b/app/views/submitters_request_changes/request_changes.html.erb @@ -0,0 +1,34 @@ + +
+
+ +

Request Changes

+ +

+ Request changes from <%= @submitter.name || @submitter.email %> for this submission. + They will receive an email with your message and be able to resubmit the form. +

+ + <%= form_for '', url: request_changes_submitter_path(@submitter.slug), method: :post do |f| %> +
+ + <%= f.text_area :reason, + required: true, + class: 'textarea textarea-bordered w-full', + dir: 'auto', + placeholder: 'Please provide specific details about what needs to be changed...', + rows: '6' %> +
+ +
+ + + <%= f.button 'Request Changes', class: 'btn btn-warning' %> + +
+ <% end %> +
+
+ diff --git a/config/locales/i18n.yml b/config/locales/i18n.yml index 8fe4ca46..3ffb9b95 100644 --- a/config/locales/i18n.yml +++ b/config/locales/i18n.yml @@ -134,6 +134,7 @@ en: &en download: Download decline: Decline declined: Declined + changes_requested: Changes Requested decline_reason: Decline reason provide_a_reason: Provide a reason notify_the_sender_with_the_reason_you_declined: Notify the sender with the reason you declined diff --git a/config/routes.rb b/config/routes.rb index 18390c8a..d76fba77 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -170,6 +170,11 @@ Rails.application.routes.draw do resources :download, only: %i[index], controller: 'submissions_download' resources :send_email, only: %i[create], controller: 'submitters_send_email' resources :debug, only: %i[index], controller: 'submissions_debug' if Rails.env.development? + + member do + get :request_changes, controller: 'submitters_request_changes' + post :request_changes, controller: 'submitters_request_changes' + end end scope '/settings', as: :settings do diff --git a/db/migrate/20250811211829_add_changes_requested_at_to_submitters.rb b/db/migrate/20250811211829_add_changes_requested_at_to_submitters.rb new file mode 100644 index 00000000..a29946b7 --- /dev/null +++ b/db/migrate/20250811211829_add_changes_requested_at_to_submitters.rb @@ -0,0 +1,5 @@ +class AddChangesRequestedAtToSubmitters < ActiveRecord::Migration[8.0] + def change + add_column :submitters, :changes_requested_at, :datetime + end +end diff --git a/db/schema.rb b/db/schema.rb index 326eb62d..658ea3bd 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.0].define(version: 2025_07_08_172115) do +ActiveRecord::Schema[8.0].define(version: 2025_08_11_211829) do # These are extensions that must be enabled in order to support this database enable_extension "btree_gin" enable_extension "pg_catalog.plpgsql" @@ -341,6 +341,7 @@ ActiveRecord::Schema[8.0].define(version: 2025_07_08_172115) do t.integer "account_id", null: false t.datetime "declined_at" t.string "timezone" + t.datetime "changes_requested_at" t.index ["account_id", "id"], name: "index_submitters_on_account_id_and_id" t.index ["completed_at", "account_id"], name: "index_submitters_on_completed_at_and_account_id" t.index ["email"], name: "index_submitters_on_email" diff --git a/lib/submitters/submit_values.rb b/lib/submitters/submit_values.rb index 8775f26e..4e6d201d 100644 --- a/lib/submitters/submit_values.rb +++ b/lib/submitters/submit_values.rb @@ -55,6 +55,7 @@ module Submitters def assign_completed_attributes(submitter, request, validate_required: true) submitter.completed_at = Time.current + submitter.changes_requested_at = nil submitter.ip = request.remote_ip submitter.ua = request.user_agent submitter.timezone = request.params[:timezone] diff --git a/spec/jobs/send_form_changes_requested_webhook_request_job_spec.rb b/spec/jobs/send_form_changes_requested_webhook_request_job_spec.rb new file mode 100644 index 00000000..787f8986 --- /dev/null +++ b/spec/jobs/send_form_changes_requested_webhook_request_job_spec.rb @@ -0,0 +1,61 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe SendFormChangesRequestedWebhookRequestJob do + let(:account) { create(:account) } + let(:user) { create(:user, account: account) } + let(:template) { create(:template, account: account, author: user) } + let(:submission) { create(:submission, template: template, created_by_user: user) } + let(:submitter) do + create( + :submitter, submission: submission, uuid: template.submitters.first['uuid'], changes_requested_at: Time.current + ) + end + let(:webhook_url) { create(:webhook_url, account: account, events: ['form.changes_requested']) } + + before do + create(:encrypted_config, key: EncryptedConfig::ESIGN_CERTS_KEY, + value: GenerateCertificate.call.transform_values(&:to_pem)) + end + + describe '#perform' do + before do + stub_request(:post, webhook_url.url).to_return(status: 200) + end + + it 'sends a webhook request' do + described_class.new.perform('submitter_id' => submitter.id, 'webhook_url_id' => webhook_url.id) + + expect(WebMock).to have_requested(:post, webhook_url.url).with( + body: { + 'event_type' => 'form.changes_requested', + 'timestamp' => /.*/, + 'data' => JSON.parse(Submitters::SerializeForWebhook.call(submitter.reload).to_json) + }, + headers: { + 'Content-Type' => 'application/json', + 'User-Agent' => 'DocuSeal.com Webhook' + } + ).once + end + + it "doesn't send a webhook request if the event is not in the webhook's events" do + webhook_url.update!(events: ['form.completed']) + + described_class.new.perform('submitter_id' => submitter.id, 'webhook_url_id' => webhook_url.id) + + expect(WebMock).not_to have_requested(:post, webhook_url.url) + end + + it 'retries on failure' do + stub_request(:post, webhook_url.url).to_return(status: 500) + + expect do + described_class.new.perform('submitter_id' => submitter.id, 'webhook_url_id' => webhook_url.id) + end.to change(described_class.jobs, :size).by(1) + + expect(WebMock).to have_requested(:post, webhook_url.url).once + end + end +end diff --git a/spec/mailers/submitter_mailer_spec.rb b/spec/mailers/submitter_mailer_spec.rb new file mode 100644 index 00000000..4be88fb9 --- /dev/null +++ b/spec/mailers/submitter_mailer_spec.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe SubmitterMailer, type: :mailer do + let(:account) { create(:account) } + let(:user) { create(:user, account: account, first_name: 'John', last_name: 'Doe') } + let(:template) { create(:template, account: account, author: user) } + let(:submission) { create(:submission, template: template, account: account, created_by_user: user) } + let(:submitter) do + create(:submitter, submission: submission, account: account, email: 'test@example.com', name: 'Jane Smith') + end + + describe '#changes_requested_email' do + let(:reason) { 'Please fix the signature field' } + let(:mail) { described_class.changes_requested_email(submitter, user, reason) } + + it 'sets the correct email attributes' do + expect(mail.to).to eq(['test@example.com']) + expect(mail.subject).to include('Changes requested') + expect(mail.from).to be_present + end + + it 'includes the reason in the email body' do + expect(mail.body.encoded).to include(reason) + end + + it 'includes the user name in the email body' do + expect(mail.body.encoded).to include('John Doe') + end + + it 'includes the submitter name in the greeting' do + expect(mail.body.encoded).to include('Jane Smith') + end + + it 'includes resubmit instructions' do + expect(mail.body.encoded).to include('resubmit') + end + end +end diff --git a/spec/models/submitter_spec.rb b/spec/models/submitter_spec.rb index d595aca0..7618dc07 100644 --- a/spec/models/submitter_spec.rb +++ b/spec/models/submitter_spec.rb @@ -70,6 +70,27 @@ RSpec.describe Submitter do expect(submitter.status).to eq('declined') end end + + context 'when submitter has changes requested' do + before { submitter.update!(changes_requested_at: Time.current) } + + it 'returns changes_requested' do + expect(submitter.status).to eq('changes_requested') + end + end + + context 'when submitter has changes requested but is also completed' do + before do + submitter.update!( + completed_at: Time.current, + changes_requested_at: Time.current + ) + end + + it 'returns changes_requested (changes_requested takes precedence over completed)' do + expect(submitter.status).to eq('changes_requested') + end + end end describe '#export_submission_on_status_change' do diff --git a/spec/requests/submitters_request_changes_controller_spec.rb b/spec/requests/submitters_request_changes_controller_spec.rb new file mode 100644 index 00000000..885ca8da --- /dev/null +++ b/spec/requests/submitters_request_changes_controller_spec.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe SubmittersRequestChangesController, type: :controller do + let(:account) { create(:account) } + let(:user) { create(:user, account: account) } + let(:template) { create(:template, account: account, author: user) } + let(:submission) { create(:submission, template: template, account: account, created_by_user: user) } + let(:submitter) { create(:submitter, submission: submission, account: account, completed_at: 1.hour.ago) } + + before do + sign_in user + end + + describe 'GET #request_changes' do + it 'renders the request changes modal' do + get :request_changes, params: { slug: submitter.slug }, xhr: true + expect(response).to have_http_status(:ok) + end + end + + describe 'POST #request_changes' do + context 'when user can request changes' do + it 'updates submitter and sends notifications' do + expect do + post :request_changes, params: { slug: submitter.slug, reason: 'Please fix the signature' } + end.to change { submitter.reload.changes_requested_at }.from(nil) + .and change { submitter.reload.completed_at }.to(nil) + + expect(response).to redirect_to(submission_path(submission)) + end + + it 'creates submission event' do + expect do + post :request_changes, params: { slug: submitter.slug, reason: 'Fix this' } + end.to change(SubmissionEvent, :count).by(1) + + event = SubmissionEvent.last + expect(event.event_type).to eq('request_changes') + expect(event.data['reason']).to eq('Fix this') + end + end + + context 'when user cannot request changes' do + let(:other_user) { create(:user, account: account) } + + before { sign_in other_user } + + it 'redirects with alert' do + post :request_changes, params: { slug: submitter.slug, reason: 'Fix this' } + expect(response).to redirect_to(root_path) + end + end + end +end diff --git a/spec/services/export_submission_service_spec.rb b/spec/services/export_submission_service_spec.rb index 441fb6e2..0bbc2900 100644 --- a/spec/services/export_submission_service_spec.rb +++ b/spec/services/export_submission_service_spec.rb @@ -191,6 +191,30 @@ RSpec.describe ExportSubmissionService do service.call end + context 'when one submitter has changes requested' do + before do + submission.submitters.first.update!(name: 'John Doe', email: 'john@example.com', + changes_requested_at: Time.current) + submission.submitters << create( + :submitter, + submission: submission, + account: account, + name: 'Jane Smith', + email: 'jane@example.com', + completed_at: Time.current, + uuid: SecureRandom.uuid + ) + end + + it 'sets overall status to changes_requested' do + allow(request_double).to receive(:body=) do |body| + parsed_body = JSON.parse(body) + expect(parsed_body['status']).to eq('changes_requested') + end + service.call + end + end + context 'when template is nil' do before do allow(submission).to receive(:template).and_return(nil)