Add validation for multiple attributes on Refund and Activity

app/legacy_lib/insert_refunds.rb

@@ -29,36 +29,34 @@ def self.modern_refund(charge, h)
     results = InsertRefunds.perform_stripe_refund(nonprofit_id: charge["nonprofit_id"], refund_data: refund_data, charge_date: charge["created_at"])
 
     Refund.transaction do
-      refund = Refund.create!({amount: h["amount"],
-        comment: h["comment"],
-        reason: h["reason"],
-        stripe_refund_id: results[:stripe_refund].id,
-        charge_id: charge["id"]})
-
-      refund.create_misc_refund_info(is_modern: true, stripe_application_fee_refund_id: results[:stripe_app_fee_refund]&.id)
-
-      gross = -h["amount"]
-      fees = (results[:stripe_app_fee_refund] && results[:stripe_app_fee_refund].amount) || 0
-      net = gross + fees
+      gross_amount = -h["amount"]
+      fee_total = results[:stripe_app_fee_refund]&.amount || 0
+      net_amount = gross_amount + fee_total
+      date = Time.current
 
-      # Create a corresponding./run  negative payment record
       payment = Payment.create!({
-        gross_amount: gross,
-        fee_total: fees,
-        net_amount: net,
+        gross_amount:,
+        fee_total:,
+        net_amount:,
         kind: "Refund",
         towards: original_payment.towards,
-        date: refund.created_at,
+        date:,
         nonprofit_id: charge["nonprofit_id"],
         supporter_id: charge["supporter_id"]
       })
 
-      InsertActivities.for_refunds([payment.id])
+      refund = Refund.create!(
+        amount: h["amount"],
+        comment: h["comment"],
+        reason: h["reason"],
+        stripe_refund_id: results[:stripe_refund].id,
+        charge_id: charge["id"],
+        payment: payment
+      )
 
-      # Update the refund to have the above payment_id
-      refund.payment = payment
-      refund.save!
+      refund.create_misc_refund_info(is_modern: true, stripe_application_fee_refund_id: results[:stripe_app_fee_refund]&.id)
 
+      InsertActivities.for_refunds([payment.id])
       # Update original payment to increment its refund_total for any future refund attempts
       original_payment.refund_total += h["amount"].to_i
       original_payment.save!

app/models/activity.rb

@@ -1,6 +1,7 @@
 # License: AGPL-3.0-or-later WITH Web-Template-Output-Additional-Permission-3.0-or-later
 class Activity < ApplicationRecord
-  belongs_to :attachment, polymorphic: true
-  belongs_to :supporter
-  belongs_to :nonprofit
+  belongs_to :attachment, polymorphic: true, optional: false
+  belongs_to :supporter, optional: false
+  belongs_to :nonprofit, optional: false
+  belongs_to :user, optional: true
 end

app/models/refund.rb

@@ -15,8 +15,9 @@ class Refund < ApplicationRecord
 
   attr_accessor :failure_message
 
-  belongs_to :charge
-  belongs_to :payment
+  belongs_to :charge, optional: false
+  belongs_to :payment, optional: false
+  belongs_to :user, optional: true
   has_one :subtransaction_payment, through: :payment
   has_one :misc_refund_info
   has_one :nonprofit, through: :charge
@@ -27,6 +28,8 @@ class Refund < ApplicationRecord
 
   has_many :manual_balance_adjustments, as: :entity
 
+  validates :amount, presence: true, numericality: {only_integer: true, greater_than: 0}
+
   def original_payment
     charge&.payment
   end

spec/models/activity_spec.rb

@@ -0,0 +1,13 @@
+# License: AGPL-3.0-or-later WITH Web-Template-Output-Additional-Permission-3.0-or-later
+require "rails_helper"
+RSpec.describe Activity, type: :model do
+  context "validation" do
+    it { is_expected.to belong_to(:attachment).required(true) }
+    it { is_expected.to belong_to(:supporter).required(true) }
+
+    it { is_expected.to belong_to(:nonprofit).required(true) }
+
+    it { is_expected.to belong_to(:user).required(false) }
+
+  end
+end

spec/models/refund_spec.rb

@@ -2,15 +2,19 @@
 require "rails_helper"
 
 RSpec.describe Refund, type: :model do
-  it { is_expected.to belong_to(:charge) }
-  it { is_expected.to belong_to(:payment) }
+  it { is_expected.to belong_to(:charge).required(true) }
+  it { is_expected.to belong_to(:payment).required(true) }
+  it { is_expected.to belong_to(:user).required(false) }
   it { is_expected.to have_one(:subtransaction_payment).through(:payment) }
   it { is_expected.to have_one(:misc_refund_info) }
 
   it { is_expected.to have_one(:nonprofit).through(:charge) }
   it { is_expected.to have_one(:supporter).through(:charge) }
   it { is_expected.to have_many(:manual_balance_adjustments) }
 
+  it { is_expected.to validate_presence_of(:amount) }
+  it { is_expected.to validate_numericality_of(:amount).only_integer.is_greater_than(0) }
+
   describe "#from_donation?" do
     it "is true when refund is associated with a donation" do
       expect(build(:refund, :from_donation).from_donation?).to eq true