On Writing Software Wrong: Ruby on Rails

On Writing Software Wrong: Ruby on Rails

To scope or not to scope......

·

4 min read

If you are a loyal reader of DHH's Rails series: On Writing Software Well. You will agree it's a "gem"(no pun intended) on how to write good code.

Lately, I have had the privilege of migrating a 3-year legacy system built in Nodejs to Rails. It's been an awesome learning experience for me and one in which I have come to learn a lot about how not to build software which in turn has made me a better developer.

Today, we will be looking at scopes and just how using "default_scopes" can be a two-edged sword.

For this project, I had a user God-Object from which other models derive from.

module Hr
  class User < ApplicationRecord
    validates_presence_of :email
    validates_format_of :email, with: URI::MailTo::EMAIL_REGEXP
    before_create :convert_email_to_lowercase
    has_one :personal_detail, class_name: 'Hr::PersonalDetail', foreign_key: 'hr_user_id', dependent: :destroy
    has_one :employee, class_name: 'Hr::Employee', foreign_key: 'hr_user_id', dependent: :destroy
    has_many :shifts, class_name: 'Hr::Shift', foreign_key: 'hr_user_id', dependent: :destroy
    has_many :account_salary_slips, class_name: 'Account::SalarySlip', foreign_key: 'hr_user_id', dependent: :destroy
    has_many :deductions, class_name: 'Hr::Deduction', foreign_key: 'hr_user_id', dependent: :destroy
    has_many :role_assignments, class_name: 'Admin::RoleAssignment', foreign_key: 'hr_user_id', dependent: :destroy
    has_many :roles, through: :role_assignments, source: 'role', dependent: :destroy
    has_many :account_salary_slips, class_name: 'Account::SalarySlip', foreign_key: 'hr_user_id', dependent: :destroy
    has_many :account_bank_details, class_name: 'Account::BankDetail', foreign_key: 'hr_user_id', dependent: :destroy
    has_many :hr_leaves, class_name: 'Hr::Leave', foreign_key: 'hr_user_id', dependent: :destroy

    default_scope { where(status: :active) }

    enum status: %i[deactivated active left terminated]
    def convert_email_to_lowercase
      self.email = email.downcase.strip
    end

A lot seems to be happening but our main concern is this line:

default_scope { where(status: :active) }

Firstly what are scopes?

"Scopes are custom queries that you define inside your Rails models with the scope method." - https://www.rubyguides.com/2019/10/scopes-in-ruby-on-rails/

This is Rails' way of saving us the need to expressly write custom queries and more importantly have reusable code which keeps things DRY and simple. What are default scopes? these are no-named scopes we intend to be executed each time our model is called or interacted with. For instance, Hr::User.all will execute a query to get all users that have a status of "active".

While this is what I had intended, I did not realize its side-effect. The side-effect?
Well, part of building this new system required importing existing data so I created a seed file, a nice trick I learned while working with AIRL labs. My seeder looked like this:


#..... sample code section
designation = Hr::Designation.find_by_old_id(employee_row['designation'])
employee_designation_id = designation.nil? ? default_designation.id : designation.id

ActiveRecord::Base.transaction do
  user = Hr::User.create!({
                            email: employee_row['company_email'],
                            status: employee_row['status']
                          })

  Hr::Employee.create!({
                         ogid: employee_row['ogid'],
                         old_erp_id: employee_row['_id'],
                         hr_user_id: user.id,
                         date_of_joining: employee_row['date_of_joining'],
                         remote: employee_row['remote'],
                         operation_branch_id: branch.id,
                         operation_office_id: office[rand(0..total_offices - 1)].id,
                         hr_designation_id: employee_designation_id
                       })

What is basically happening is I am reading from a CSV import and then proceeding to create employees after creating my user. Remember, an employee is a type of User.

The issue arose when I tried to create an employee with a user whose status from the CSV was "terminated". What happened was when calling Hr::Employee.create(), I passed the hr_user_id which is a foreign key to the hr_user table. Active Record needs to ensure that the user exists before creating an employee with that user ID.

To achieve this, it calls the User model to find a user with that ID, however, because of the default scope, the User model will execute finding a user with that ID who has a status of active. Now such a user does not exist because the user during the current iteration was terminated so his record was something like this:

{id: 2, email: demo@gmail.com, status: terminated, created_at: <date>, updated_at: <date>}

Clearly, This was not my intention, a side effect I did not intend. The consequence of this was all models that belong to the user model will only ever get records of users who are active. This would have been a difficult bug to catch because it affects insertions where the user_id is a foreign key. Imagine telling users they cannot create a leave application because they do not exist even when they are logged in.

This also affects deletes. Calling User.destroy_all, will only destroy "active" users. Again another unintended side effect. This Stackoverflow post gives more insight.

To conclude, as with all things, it's always important to know the side effects just as we attempt to utilize their seeming benefits. See you in the next series!!