Merge pull request #11 from CareerPlug/CP-10319-approve-reject-changes

CP-10319 changes requested feature

.rubocop.yml

@@ -6,6 +6,7 @@ require:
 AllCops:
   NewCops: enable
   Exclude:
+    - db/migrate/**/*
     - db/schema.rb
     - node_modules/**/*
     - bin/*

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? || request.head?
+      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

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

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

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
 

app/models/submitter.rb

@@ -4,27 +4,28 @@
 #
 # Table name: submitters
 #
-#  id            :bigint           not null, primary key
+#  id                   :bigint           not null, primary key
-#  completed_at  :datetime
+#  changes_requested_at :datetime
-#  declined_at   :datetime
+#  completed_at         :datetime
-#  email         :string
+#  declined_at          :datetime
-#  ip            :string
+#  email                :string
-#  metadata      :text             not null
+#  ip                   :string
-#  name          :string
+#  metadata             :text             not null
-#  opened_at     :datetime
+#  name                 :string
-#  phone         :string
+#  opened_at            :datetime
-#  preferences   :text             not null
+#  phone                :string
-#  sent_at       :datetime
+#  preferences          :text             not null
-#  slug          :string           not null
+#  sent_at              :datetime
-#  timezone      :string
+#  slug                 :string           not null
-#  ua            :string
+#  timezone             :string
-#  uuid          :string           not null
+#  ua                   :string
-#  values        :text             not null
+#  uuid                 :string           not null
-#  created_at    :datetime         not null
+#  values               :text             not null
-#  updated_at    :datetime         not null
+#  created_at           :datetime         not null
-#  account_id    :integer          not null
+#  updated_at           :datetime         not null
-#  external_id   :string
+#  account_id           :integer          not null
-#  submission_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?

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')

app/views/submissions/show.html.erb

@@ -180,6 +180,8 @@
               <span>
                 <% 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 @@
                 </span>
               </div>
             <% end %>
+            <% if submitter&.changes_requested_at? %>
+              <div class="flex items-center space-x-1 mt-1">
+                <span>
+                  Changes requested:
+                  <%= simple_format(h(submitter.submission_events.find_by(event_type: :request_changes).data['reason'])) %>
+                </span>
+              </div>
+            <% 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? %>
               <div class="mt-2 mb-1">
                 <%= 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 } %>
               </div>
             <% end %>
+            <% if signed_in? && submitter && submitter.completed_at? && !submitter.declined_at? && !submitter.changes_requested_at? && current_user == @submission.created_by_user %>
+              <div class="mt-2 mb-1">
+                <%= link_to 'Request Changes', request_changes_submitter_path(submitter.slug),
+                            class: 'btn btn-sm btn-warning w-full',
+                            data: { turbo_frame: :modal } %>
+              </div>
+            <% end %>
           </div>
         </div>
         <div class="px-1.5 mb-4">

app/views/submitter_mailer/changes_requested_email.html.erb

@@ -0,0 +1,10 @@
+<p>Hello <%= @submitter.name || @submitter.email %>,</p>
+
+<p><%= [@user.first_name, @user.last_name].join(' ').strip.presence || @user.email %> has requested changes to your submission for "<%= @submission.name || @submission.template.name %>".</p>
+
+<p><strong>Message from <%= [@user.first_name, @user.last_name].join(' ').strip.presence || @user.email %>:</strong></p>
+<%= simple_format(h(@reason)) %>
+
+<p>To make the requested changes, please log in to your account and resubmit the form.</p>
+
+<p>If you have any questions, please reply to this email.</p>

app/views/submitters_request_changes/request_changes.html.erb

@@ -0,0 +1,34 @@
+<turbo-frame id="modal" target="_top">
+  <div class="modal modal-open">
+    <div class="modal-box relative">
+      <label class="btn btn-sm btn-circle absolute right-2 top-2" onclick="document.getElementById('modal').innerHTML = ''">✕</label>
+      <h3 class="text-lg font-bold mb-4">Request Changes</h3>
+
+      <p class="mb-4">
+        Request changes from <strong><%= @submitter.name || @submitter.email %></strong> for this submission.
+        They will receive an email with your message and be able to resubmit the form.
+      </p>
+
+      <%= form_for '', url: request_changes_submitter_path(@submitter.slug), method: :post do |f| %>
+        <div class="form-control mt-2">
+          <label class="label">
+            <span class="label-text">Message (required)</span>
+          </label>
+          <%= 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' %>
+        </div>
+
+        <div class="modal-action">
+          <label class="btn btn-ghost" onclick="document.getElementById('modal').innerHTML = ''">Cancel</label>
+          <toggle-submit dir="auto">
+            <%= f.button 'Request Changes', class: 'btn btn-warning' %>
+          </toggle-submit>
+        </div>
+      <% end %>
+    </div>
+  </div>
+</turbo-frame>

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

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

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

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"

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]

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

spec/mailers/submitter_mailer_spec.rb

@@ -0,0 +1,47 @@
+# 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',
+      uuid: template.submitters.first['uuid']
+    )
+  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

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

spec/requests/request_changes_spec.rb

@@ -0,0 +1,73 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+describe 'Request Changes' 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) do
+    create(
+      :submitter,
+      submission: submission,
+      account: account,
+      completed_at: 1.hour.ago,
+      uuid: template.submitters.first['uuid']
+    )
+  end
+
+  before do
+    sign_in user
+  end
+
+  describe 'GET /submitters/:slug/request_changes' do
+    it 'renders the request changes modal when xhr request' do
+      get "/submitters/#{submitter.slug}/request_changes",
+          headers: { 'X-Requested-With' => 'XMLHttpRequest' }
+
+      expect(response).to have_http_status(:ok)
+    end
+  end
+
+  describe 'POST /submitters/:slug/request_changes' do
+    context 'when user can request changes' do
+      it 'updates submitter and sends notifications' do
+        expect do
+          post "/submitters/#{submitter.slug}/request_changes",
+               params: { 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 have_http_status(:found)
+      end
+
+      it 'creates submission event' do
+        expect do
+          post "/submitters/#{submitter.slug}/request_changes",
+               params: { 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 do
+        sign_out user
+        sign_in other_user
+      end
+
+      it 'redirects with alert' do
+        post "/submitters/#{submitter.slug}/request_changes",
+             params: { reason: 'Fix this' }
+
+        expect(response).to have_http_status(:found)
+      end
+    end
+  end
+end

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)