Apply SOLID and Clean Code principles

app/models/amount.rb

@@ -5,4 +5,13 @@
 
 # a simple object for storing values. Likely will be replaced with Money from ruby-money
 # in future
-Amount = Struct.new(:cents, :currency)
+require 'modules/amount_creator'
+class Amount < ApplicationRecord
+    include AmountCreator
+
+    def initialize(*args)
+        @amount = args[0]
+        @currency = args[1]
+    end
+end
+

app/models/amount_management.rb

@@ -0,0 +1,17 @@
+require 'modules/amount_creator'
+
+class AmountManagement < ApplicationRecord
+    include AmountCreator
+    include Model::Amount
+
+    delegate :initialize, to: :amount
+
+    def self.generate_amount_as_money(amount, currency)
+        AmountCreator.initialize([amount, currency])
+    end
+
+    def self.net_amount(target)
+		target.sum(&:net_amount)
+	end
+
+end

app/models/campaign.rb

@@ -60,7 +60,7 @@ class Campaign < ApplicationRecord
   has_one_attached_with_sizes(:main_image, {normal: [524, 360], thumb: [180,150]})
   has_one_attached_with_sizes(:background_image, {normal: [1000, 600]})
 
-  has_one_attached_with_default(:main_image, Houdini.defaults.image.profile, 
+  has_one_attached_with_default(:main_image, Houdini.defaults.image.profile,
     filename: "main_image_#{SecureRandom.uuid}#{Pathname.new(Houdini.defaults.image.profile).extname}")
 
   has_many :donations
@@ -119,23 +119,18 @@ def set_defaults
   end
 
   def parse_video_id
+    self.vimeo_video_id = nil
+    self.youtube_video_id = nil
     if video_url.include? 'vimeo'
-      self.vimeo_video_id = video_url.split('/').last
-      self.youtube_video_id = nil
+        self.vimeo_video_id = video_url.split('/').last
     elsif video_url.include? 'youtube'
-      match = video_url.match(/\?v=(.+)/)
-      return if match.nil?
+        match = video_url.match(/\?v=(.+)/)
+        return if match.nil?
 
-      self.youtube_video_id = match[1].split('&').first
-      self.vimeo_video_id = nil
+        self.youtube_video_id = match[1].split('&').first
     elsif video_url.include? 'youtu.be'
-      self.youtube_video_id = video_url.split('/').last
-      self.vimeo_video_id = nil
-    elsif video_url.blank?
-      self.vimeo_video_id = nil
-      self.youtube_video_id = nil
+        self.youtube_video_id = video_url.split('/').last
     end
-    self
   end
 
   def total_raised

app/models/modules/amount_creator.rb

@@ -0,0 +1,5 @@
+module AmountCreator
+    def self.initialize(*args)
+        raise 'Not implemented yet'
+    end
+end

app/models/offline_transaction.rb

@@ -5,19 +5,9 @@
 # rubocop:disable Metrics/BlockLength, Metrics/AbcSize, Metrics/MethodLength
 class OfflineTransaction < ApplicationRecord
 	include Model::Subtransactable
-	delegate :created, to: :subtransaction
-
-	def amount_as_money
-		Amount.new(amount || 0, nonprofit.currency)
-	end
+	include Model::AmountManagement
 
-	def net_amount
-		payments.sum(&:net_amount)
-	end
-
-	def net_amount_as_money
-		Amount.new(net_amount || 0, nonprofit.currency)
-	end
+	delegate :created, to: :subtransaction
 
 	concerning :JBuilder do
 		included do

app/models/offline_transaction_charge.rb

@@ -6,22 +6,12 @@
 # rubocop:disable Metrics/BlockLength, Metrics/AbcSize
 class OfflineTransactionCharge < ApplicationRecord
 	include Model::SubtransactionPaymentable
+	include Model::AmountManagement
+
 	belongs_to :payment
 
 	delegate :gross_amount, :net_amount, :fee_total, to: :payment
 
-	def gross_amount_as_money
-		Amount.new(gross_amount || 0, currency)
-	end
-
-	def net_amount_as_money
-		Amount.new(net_amount || 0, currency)
-	end
-
-	def fee_total_as_money
-		Amount.new(fee_total || 0, currency)
-	end
-
 	def created
 		payment.date
 	end

app/models/offline_transaction_refund.rb

@@ -4,24 +4,14 @@
 # Full license explanation at https://github.com/houdiniproject/houdini/blob/main/LICENSE
 class OfflineTransactionRefund < ApplicationRecord
 	include Model::SubtransactionPaymentable
+	include Model::AmountManagement
+
 	belongs_to :payment
 
 	delegate :gross_amount, :net_amount, :fee_total, to: :payment
 
 	delegate :currency, to: :nonprofit
 
-	def gross_amount_as_money
-		Amount.new(gross_amount || 0, currency)
-	end
-
-	def net_amount_as_money
-		Amount.new(net_amount || 0, currency)
-	end
-
-	def fee_total_as_money
-		Amount.new(fee_total || 0, currency)
-	end
-
 	def created
 		payment.date
 	end

app/models/stripe_transaction.rb

@@ -4,11 +4,9 @@
 # Full license explanation at https://github.com/houdiniproject/houdini/blob/main/LICENSE
 class StripeTransaction < ApplicationRecord
 	include Model::Subtransactable
-	delegate :created, to: :subtransaction
+	include Model::AmountManagement
 
-	def net_amount
-		payments.sum(&:net_amount)
-	end
+	delegate :created, to: :subtransaction
 
 	concerning :JBuilder do # rubocop:disable Metrics/BlockLength
 		included do

app/models/transaction.rb

@@ -4,6 +4,7 @@
 # Full license explanation at https://github.com/houdiniproject/houdini/blob/main/LICENSE
 class Transaction < ApplicationRecord
 	include Model::CreatedTimeable
+	include Model::AmountManagement
 
 	belongs_to :supporter
 	has_one :nonprofit, through: :supporter
@@ -19,10 +20,6 @@ class Transaction < ApplicationRecord
 
 	validates :supporter, presence: true
 
-	def amount_as_money
-    Amount.new(amount||0, nonprofit.currency)
-  end
-
 	concerning :JBuilder do
 		include Model::Houidable
 		include Model::Jbuilder

db/migrate/20220407105656_create_amount_management.rb

@@ -0,0 +1,12 @@
+# frozen_string_literal: true
+
+# License: AGPL-3.0-or-later WITH WTO-AP-3.0-or-later
+# Full license explanation at https://github.com/houdiniproject/houdini/blob/main/LICENSE
+class CreateAmountManagement < ActiveRecord::Migration[6.1]
+  def change
+    create_table :amount_managements do |t|
+
+      t.timestamps
+    end
+  end
+end

db/migrate/20220407130650_create_amount.rb

@@ -0,0 +1,14 @@
+# frozen_string_literal: true
+
+# License: AGPL-3.0-or-later WITH WTO-AP-3.0-or-later
+# Full license explanation at https://github.com/houdiniproject/houdini/blob/main/LICENSE
+class CreateAmount < ActiveRecord::Migration[6.1]
+  def change
+    create_table :amounts do |t|
+      t.integer :amount
+      t.string :currency
+
+      t.timestamps
+    end
+  end
+end

db/migrate/20220407170352_change_amount.rb

@@ -0,0 +1,8 @@
+# frozen_string_literal: true
+
+# License: AGPL-3.0-or-later WITH WTO-AP-3.0-or-later
+# Full license explanation at https://github.com/houdiniproject/houdini/blob/main/LICENSE
+class ChangeAmount < ActiveRecord::Migration[6.1]
+  def change
+  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.define(version: 2021_11_01_214501) do
+ActiveRecord::Schema.define(version: 2022_04_07_130650) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "pg_stat_statements"
@@ -65,6 +65,18 @@
     t.index ["supporter_id"], name: "index_activities_on_supporter_id"
   end
 
+  create_table "amount_managements", force: :cascade do |t|
+    t.datetime "created_at", precision: 6, null: false
+    t.datetime "updated_at", precision: 6, null: false
+  end
+
+  create_table "amounts", force: :cascade do |t|
+    t.integer "amount"
+    t.string "currency"
+    t.datetime "created_at", precision: 6, null: false
+    t.datetime "updated_at", precision: 6, null: false
+  end
+
   create_table "bank_accounts", id: :serial, force: :cascade do |t|
     t.string "name", limit: 255
     t.string "account_number", limit: 255