Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .database_consistency.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# Invoice#human_id is populated deterministically by a before_create callback
# (Invoice#set_human_id) rather than being set directly, so it is never nil at
# INSERT time even though the column is NOT NULL. Adding a presence validator
# would run before that callback and would break specs built with
# FactoryBot's build_stubbed (which never runs on: :create callbacks).
Invoice:
human_id:
NullConstraintChecker:
enabled: false
2 changes: 1 addition & 1 deletion Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ group :development, :test do
gem 'brakeman', '~> 8.0.1', require: false
gem 'bullet', '~> 8.1'
gem 'colorize', '~> 1.1.0'
gem 'database_consistency', '~> 2.1.1'
gem 'database_consistency', '~> 3.0.12'
gem 'dotenv-rails', '~> 3.2.0'
gem 'guard-livereload', '~> 2.5.2'
gem 'guard-rspec', '~> 4.7.3', require: false
Expand Down
8 changes: 4 additions & 4 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -119,14 +119,14 @@ GEM
coderay (1.1.3)
colorize (1.1.0)
concurrent-ruby (1.3.8)
connection_pool (2.5.5)
connection_pool (3.0.2)
crass (1.0.7)
cssbundling-rails (1.4.3)
railties (>= 6.0.0)
dartsass-rails (0.5.1)
railties (>= 6.0.0)
sass-embedded (~> 1.63)
database_consistency (2.1.1)
database_consistency (3.0.12)
activerecord (>= 3.2)
date (3.5.1)
devise (5.0.4)
Expand Down Expand Up @@ -218,7 +218,7 @@ GEM
domain_name (~> 0.5)
http-form_data (2.3.0)
http_parser.rb (0.8.0)
i18n (1.14.8)
i18n (1.15.2)
concurrent-ruby (~> 1.0)
io-console (0.8.2)
irb (1.16.0)
Expand Down Expand Up @@ -621,7 +621,7 @@ DEPENDENCIES
colorize (~> 1.1.0)
cssbundling-rails (~> 1.4, >= 1.4.3)
dartsass-rails (~> 0.5.1)
database_consistency (~> 2.1.1)
database_consistency (~> 3.0.12)
devise (~> 5.0.0)
devise-i18n (~> 1.15.0)
dotenv-rails (~> 3.2.0)
Expand Down
7 changes: 7 additions & 0 deletions app/jobs/payment_poll_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,13 @@ class PaymentPollJob < ApplicationJob

def perform
Payment.not_completed.each do |payment|
if payment.mollie_id.nil?
# Mollie::Payment.create can fail to return an id (e.g. the API call errored after
# the record was created), leaving the payment stuck in `not_completed` forever
payment.update(status: 'failed') if payment.created_at < 15.minutes.ago
next
end

payment.update(status: payment.mollie_payment.status)
rescue ActiveRecord::StaleObjectError
# If the payment has not changed to paid with this concurrent update,
Expand Down
1 change: 1 addition & 0 deletions app/models/activity.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ class Activity < ApplicationRecord

has_many :orders, dependent: :destroy
has_many :credit_mutations, dependent: :destroy
has_many :invoices, dependent: :destroy
has_many :ordering_users, through: :orders, source: :user
belongs_to :price_list
belongs_to :created_by, class_name: 'User'
Expand Down
3 changes: 3 additions & 0 deletions app/models/invoice.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,11 @@ class Invoice < ApplicationRecord
belongs_to :user
belongs_to :activity
has_many :rows, class_name: 'InvoiceRow', dependent: :destroy
has_many :payments, dependent: :destroy
accepts_nested_attributes_for :rows

validates :token, uniqueness: true

validate :activity_is_locked

before_create :set_human_id
Expand Down
8 changes: 5 additions & 3 deletions app/models/order_row.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,20 @@ class OrderRow < ApplicationRecord
belongs_to :product

validates :product_count, presence: true, numericality: { only_integer: true, greater_than_or_equal_to: 0 }
validates :price_per_product, numericality: { greater_than: 0, allow_nil: true }
validates :price_per_product, presence: true, numericality: { greater_than: 0 }
validates :product, inclusion: { in: :available_products }

validate :no_changes_of_product_count_allowed
validate :no_changes_of_price_per_product_allowed

before_create :copy_product_price
before_validation :copy_product_price, on: :create

before_destroy -> { throw(:abort) }

def copy_product_price
self.price_per_product = order.activity.price_list.product_price_for(product).price
return if order.blank? || product.blank?

self.price_per_product = order.activity.price_list.product_price_for(product)&.price
end

def no_changes_of_product_count_allowed
Expand Down
1 change: 1 addition & 0 deletions app/models/product.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ class Product < ApplicationRecord

has_many :product_prices, dependent: :destroy
has_many :price_lists, through: :product_prices, dependent: :restrict_with_error
has_many :order_rows, dependent: :destroy

attribute :color, :string, default: '#f8f9fa'

Expand Down
2 changes: 2 additions & 0 deletions app/models/roles_users.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,7 @@ class RolesUsers < ApplicationRecord
belongs_to :user
belongs_to :role

validates :user_id, uniqueness: { scope: %i[role_id created_at] }

delegate :name, to: :role
end
3 changes: 2 additions & 1 deletion app/models/sofia_account.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@ class SofiaAccount < OmniAuth::Identity::Models::ActiveRecord

belongs_to :user

validates :user, uniqueness: true # rubocop:disable Rails/UniqueValidationWithoutIndex
validates :user, uniqueness: true
validates :username, presence: true, uniqueness: true
validates :password_digest, presence: true
# the presence of :password is already checked by omniauth-sofia-account itself
validates :password, length: { minimum: 12, maximum: 128 }, allow_nil: true

Expand Down
6 changes: 6 additions & 0 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ class User < ApplicationRecord # rubocop:disable Metrics/ClassLength
has_many :order_rows, through: :orders, dependent: :destroy
has_many :credit_mutations, dependent: :destroy
has_many :activities, dependent: :destroy, foreign_key: 'created_by_id', inverse_of: :created_by
has_many :payments, dependent: :destroy
has_many :invoices, dependent: :destroy

has_many :roles_users, class_name: 'RolesUsers', dependent: :destroy
has_many :roles, through: :roles_users
Expand All @@ -13,6 +15,10 @@ class User < ApplicationRecord # rubocop:disable Metrics/ClassLength
validate :no_deactivation_when_nonzero_credit
validates :email, format: { with: Devise.email_regexp }, allow_blank: true
validates :email, presence: true, if: ->(user) { !user.deactivated && user.sofia_account.present? }
validates :email, uniqueness: {
case_sensitive: false,
conditions: -> { where(deleted_at: nil).where.not(email: nil).where('provider IS DISTINCT FROM ?', 'amber_oauth2') }
}, if: ->(user) { user.email.present? && user.deleted_at.nil? && user.provider != 'amber_oauth2' }

scope :in_amber, -> { where(provider: 'amber_oauth2') }
scope :sofia_account, -> { where(provider: 'sofia_account') }
Expand Down
3 changes: 1 addition & 2 deletions bin/ci.sh
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@ if [ "${TYPE}" = "lint" ] || [ "${TYPE}" = "" ]; then
gem install bundler-audit
bundle-audit update && bundle-audit check --ignore CVE-2015-9284 || true
RAILS_ENV=test bundle exec rails db:create db:environment:set db:schema:load
# uncomment when it does not fail anymore :)
# bundle exec database_consistency
RAILS_ENV=test bundle exec database_consistency

echo "--- :eslint: Yarn lint"
yarn install # Why do I need to do this again? This was done in Dockerfile, rite?
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
class EnforceDatabaseConsistencyConstraints < ActiveRecord::Migration[7.2]
def change
remove_redundant_indexes
fix_sofia_accounts_unique_index
add_missing_foreign_keys
add_missing_not_null_constraints
add_missing_check_constraints
add_email_uniqueness_index
end

private

def remove_redundant_indexes
# Both are fully covered by the composite index index_roles_users_on_user_id_and_role_id_and_created_at
remove_index :roles_users, :user_id, name: 'index_roles_users_on_user_id'
# Both are fully covered by the composite index index_product_prices_on_product_id_and_price_list_id
remove_index :product_prices, :product_id, name: 'index_product_prices_on_product_id'
end

def fix_sofia_accounts_unique_index
remove_index :sofia_accounts, :user_id, name: 'index_sofia_accounts_on_user_id'
add_index :sofia_accounts, :user_id, unique: true
end

def add_missing_foreign_keys
add_foreign_key :roles_users, :users
add_foreign_key :roles_users, :roles
add_foreign_key :product_prices, :products
add_foreign_key :product_prices, :price_lists
add_foreign_key :payments, :users
add_foreign_key :payments, :invoices
add_foreign_key :order_rows, :orders
add_foreign_key :order_rows, :products
add_foreign_key :orders, :activities
add_foreign_key :orders, :users
add_foreign_key :invoice_rows, :invoices
add_foreign_key :invoices, :users
add_foreign_key :invoices, :activities
add_foreign_key :credit_mutations, :users
add_foreign_key :credit_mutations, :activities
add_foreign_key :activities, :price_lists
end

def add_missing_not_null_constraints
change_column_null :roles, :role_type, false

change_table :product_prices, bulk: true do |t|
t.change_null :product_id, false
t.change_null :price_list_id, false
t.change_null :price, false
end

change_column_null :payments, :amount, false
change_column_null :orders, :created_by_id, false
change_column_null :invoice_rows, :invoice_id, false
change_column_null :credit_mutations, :created_by_id, false

change_table :activities, bulk: true do |t|
t.change_null :price_list_id, false
t.change_null :created_by_id, false
end

change_column_null :users, :name, false
end

def add_missing_check_constraints
add_check_constraint :order_rows, 'product_count >= 0', name: 'order_rows_product_count_check'
add_check_constraint :order_rows, 'price_per_product > 0', name: 'order_rows_price_per_product_check'
add_check_constraint :credit_mutations, 'amount <= 5000', name: 'credit_mutations_amount_check'
end

def add_email_uniqueness_index
# Only sofia_account and manually created users sign up with their own email address.
# Amber-synced accounts (provider: 'amber_oauth2') are managed externally and are excluded.
add_index :users, 'lower(email)',
name: 'index_users_on_lower_email_for_sofia_and_manual',
unique: true,
where: "deleted_at IS NULL AND email IS NOT NULL AND provider IS DISTINCT FROM 'amber_oauth2'"
end
end
Loading
Loading