Update parameters used for API requests

Author: ozovalihasan

app/controllers/api/v1/pushes_controller.rb

@@ -153,6 +153,10 @@ def create
     # when creating file pushes / uploading attachments.
     authenticate_user! if requires_authentication_for_create?(permitted_params)
 
+    # Extract notify_by_email params before creating the Push
+    # to avoid ActiveModel::UnknownAttributeError
+    permitted_notify_by_email_params = permitted_params.delete(:notify_by_email)
+
     @push = Push.new(permitted_params)
 
     if !permitted_params[:kind].present?
@@ -173,15 +177,17 @@ def create
 
     if user_signed_in?
       @push.user = current_user
-      @push.notify_by_email_creator = current_user if permitted_params[:notify_by_email_recipients].present?
     end
 
+    # Handle nested notify_by_email params
+    set_notify_by_email(@push, permitted_notify_by_email_params) if permitted_notify_by_email_params.present?
+
     assign_deletable_by_viewer(@push, permitted_params)
     assign_retrieval_step(@push, permitted_params)
 
     if @push.save
       log_creation(@push)
-      log_creation_email_send(@push) if permitted_params[:notify_by_email_recipients].present?
+      log_creation_email_send(@push) if permitted_notify_by_email_params.present?
 
       render template: "pushes/show", status: :created
     else
@@ -484,6 +490,13 @@ def set_push
     end
   end
 
+  def set_notify_by_email(push, notify_by_email_params, required: false)
+    push.notify_by_email_recipients = notify_by_email_params[:recipients]
+    push.notify_by_email_locale = notify_by_email_params[:locale]
+    push.notify_by_email_creator = current_user if user_signed_in?
+    push.notify_by_email_required = required
+  end
+
   def push_params
     if request.path.start_with?("/f")
       params.require(:file_push).permit(:name, :expire_after_days, :expire_after_views, :deletable_by_viewer,

app/controllers/api/v2/pushes_controller.rb

@@ -6,10 +6,7 @@ class Api::V2::PushesController < Api::V1::PushesController
   before_action :set_push, only: %i[show preview audit destroy notify_by_email]
 
   def notify_by_email
-    @push.notify_by_email_recipients = params[:recipients]
-    @push.notify_by_email_locale = params[:locale]
-    @push.notify_by_email_creator = current_user if user_signed_in?
-    @push.notify_by_email_required = true
+    set_notify_by_email(@push, notify_by_email_params, required: true)
 
     if @push.valid?
       log_creation_email_send(@push)
@@ -25,9 +22,20 @@ def force_json_format
     request.format = :json
   end
 
+  def set_notify_by_email(push, permitted_params, required: false)
+    push.notify_by_email_recipients = permitted_params[:recipients]
+    push.notify_by_email_locale = permitted_params[:locale]
+    push.notify_by_email_creator = current_user if user_signed_in?
+    push.notify_by_email_required = required
+  end
+
+  def notify_by_email_params
+    params.permit(:recipients, :locale)
+  end
+
   def push_params
     permitted = params.require(:push).permit(:name, :kind, :expire_after_days, :expire_after_views,
-      :deletable_by_viewer, :retrieval_step, :payload, :note, :passphrase, :notify_by_email_recipients, :notify_by_email_locale, files: [])
+      :deletable_by_viewer, :retrieval_step, :payload, :note, :passphrase, notify_by_email: [:recipients, :locale], files: [])
 
     # For v2 requests, file uploads imply a file push unless kind is explicit.
     if permitted[:kind].blank? && permitted[:files].present?

app/controllers/concerns/log_events.rb

@@ -34,7 +34,6 @@ def log_creation_email_send(push)
     notify_by_email = audit_log.build_notify_by_email(recipients: recipients, locale: locale)
 
     audit_log.save!
-
     SendPushCreatedEmailJob.perform_later(notify_by_email.id)
   end
 

app/views/pages/api.html.erb

@@ -115,8 +115,9 @@
                 <tr><td><code>passphrase</code></td><td>string</td><td><%= _("No") %></td><td><%= _("Requires this passphrase to retrieve the payload.") %></td></tr>
                 <tr><td><code>name</code></td><td>string</td><td><%= _("No") %></td><td><%= _("Optional label shown to the owner.") %></td></tr>
                 <tr><td><code>note</code></td><td>string</td><td><%= _("No") %></td><td><%= _("Optional owner-only note.") %></td></tr>
-                <tr><td><code>notify_by_email_recipients</code></td><td>string</td><td><%= _("No") %></td><td><%= _("Optional recipients for the push creation email. Comma-separated list of email addresses. Maximum 5 emails.") %></td></tr>
-                <tr><td><code>notify_by_email_locale</code></td><td>string</td><td><%= _("No") %></td><td><%= _("Optional locale for the push creation email. Defaults to #{I18n.default_locale}. Available locales: #{I18n.available_locales.map(&:to_s).join(", ")}") %></td></tr>
+                <tr><td><code>notify_by_email</code></td><td>object</td><td><%= _("No") %></td><td><%= _("Optional recipients for the push creation email.") %></td></tr>
+                <tr><td><code>notify_by_email.recipients</code></td><td>string</td><td><%= _("No") %></td><td><%= _("Optional recipients for the push creation email. Comma-separated list of email addresses. Maximum 5 emails.") %></td></tr>
+                <tr><td><code>notify_by_email.locale</code></td><td>string</td><td><%= _("No") %></td><td><%= _("Optional locale for the push creation email. Defaults to #{I18n.default_locale}. Available locales: #{I18n.available_locales.map(&:to_s).join(", ")}") %></td></tr>
               </tbody>
             </table>
           </div>
@@ -127,7 +128,11 @@
     "expire_after_views": 5,
     "passphrase": "optional-passphrase",
     "deletable_by_viewer": true,
-    "retrieval_step": true
+    "retrieval_step": true,
+    "notify_by_email": {
+      "recipients": "email1@example.com, email2@example.com",
+      "locale": "en"
+    }
   }
 }</code></pre>
           <p class="mb-2 mt-3"><strong><%= _("cURL example (JSON body):") %></strong></p>
@@ -231,7 +236,7 @@
   -H "Authorization: Bearer YOUR_API_TOKEN" \
   -H "Content-Type: application/json" \
   -d '{
-    "recipients": "recipient1@example.com,recipient2@example.com",
+    "recipients": "email1@example.com, email2@example.com",
     "locale": "en"
   }'</code></pre>
         </div>

app/views/pushes/audit.json.jbuilder

@@ -11,6 +11,7 @@ json.logs @audit_logs do |audit_log|
       json.recipients audit_log.notify_by_email.recipients
       json.locale audit_log.notify_by_email.locale
       json.status audit_log.notify_by_email.status
+      json.error_message audit_log.notify_by_email.error_message if audit_log.notify_by_email.error_message.present?
       json.successful_sends audit_log.notify_by_email.successful_sends
       json.proceed_at audit_log.notify_by_email.proceed_at
     end

test/integration/api/api_v2_pushes_test.rb

@@ -166,9 +166,12 @@ def test_audit_includes_notify_by_email_details_for_completed_notify_by_email
     notify_by_email = notify_by_emails(:one)
     owner = users(:giuliana)
 
-    travel_to Time.zone.local(2026, 1, 1, 1, 0, 0) do
-      SendPushCreatedEmailJob.perform_now(notify_by_email.id)
+    Settings.stub(:notify_by_email_available?, true) do
+      travel_to Time.zone.local(2026, 1, 1, 1, 0, 0) do
+        SendPushCreatedEmailJob.perform_now(notify_by_email.id)
+      end
     end
+
     get "/api/v2/pushes/#{push.url_token}/audit",
       headers: bearer_headers(owner),
       as: :json
@@ -389,8 +392,10 @@ def test_create_with_notify_by_email_params_adds_a_job_to_the_queue
         params: {
           push: {
             payload: "some-secret",
-            notify_by_email_recipients: "recipient@example.com",
-            notify_by_email_locale: "en"
+            notify_by_email: {
+              recipients: "recipient@example.com",
+              locale: "en"
+            }
           }
         },
         headers: bearer_headers(user),
@@ -420,8 +425,10 @@ def test_create_with_notify_by_email_params_fails_when_email_service_is_not_conf
       params: {
         push: {
           payload: "some-secret",
-          notify_by_email_recipients: "recipient@example.com",
-          notify_by_email_locale: "en"
+          notify_by_email: {
+            recipients: "recipient@example.com",
+            locale: "en"
+          }
         }
       },
       headers: bearer_headers(user),
@@ -441,8 +448,10 @@ def test_create_with_notify_by_email_params_fails_when_user_is_not_signed_in
       params: {
         push: {
           payload: "some-secret",
-          notify_by_email_recipients: "recipient@example.com",
-          notify_by_email_locale: "en"
+          notify_by_email: {
+            recipients: "recipient@example.com",
+            locale: "en"
+          }
         }
       },
       as: :json

test/unit/send_push_created_email_job_test.rb

@@ -12,7 +12,9 @@ class SendPushCreatedEmailJobTest < ActiveJob::TestCase
 
   test "sends email to specified recipient" do
     mails = capture_emails do
-      SendPushCreatedEmailJob.perform_now(@notify_by_email.id)
+      Settings.stub(:notify_by_email_available?, true) do
+        SendPushCreatedEmailJob.perform_now(@notify_by_email.id)
+      end
     end
 
     mail = mails.first
@@ -21,7 +23,9 @@ class SendPushCreatedEmailJobTest < ActiveJob::TestCase
   end
 
   test "perform update notify_by_email status to completed after sending" do
-    SendPushCreatedEmailJob.perform_now(@notify_by_email.id)
+    Settings.stub(:notify_by_email_available?, true) do
+      SendPushCreatedEmailJob.perform_now(@notify_by_email.id)
+    end
 
     @notify_by_email.reload
     assert_equal "completed", @notify_by_email.status
@@ -33,7 +37,9 @@ class SendPushCreatedEmailJobTest < ActiveJob::TestCase
     failing_mail.expect(:deliver_now, -> { raise StandardError, "test error" })
 
     PushCreatedMailer.stub(:with, failing_mail) do
-      SendPushCreatedEmailJob.perform_now(@notify_by_email.id)
+      Settings.stub(:notify_by_email_available?, true) do
+        SendPushCreatedEmailJob.perform_now(@notify_by_email.id)
+      end
     end
 
     @notify_by_email.reload