I'm working on Ruby on Rails application. It's an ecommerce platform for print creators. My minimum viable product is working, and it's time to roll up my sleeves and refactor the code to get it production-ready.

What needs to change about my Reader Model?

The basic authentication model is the Reader. It represents what you might consider to be a User in other applications.

This model is built with Devise. When I first wrote it, I also used it to store profile information for each reader.

After some great conversations in the Ruby on Rails Link Slack, I've come to understand that my authentication model shouldn't be responsible for managing user data. It sort of violates the single responsibility principle. And practically speaking, every attribute follows the current_reader object in controllers and views. So if I have a logged-in Reader on the home page, the view knows all of its attributes. In my case, that only includes a first_name and last_name column, but this approach can lead to problems down the road.

If I continue to add reader data in the Reader model, I will be loading a lot of unnecessary data in each request with the current_reader object.

I want to contain these attributes in a new class called ReaderProfile, of which each Reader object will have_one.

The Current Reader Model

This is my current ApplicationRecord subclass:

# frozen_string_literal: true

class Reader < ApplicationRecord
  # Include default devise modules. Others available are:
  # :confirmable, :lockable, :timeoutable, :trackable and :omniauthable
  devise :database_authenticatable, :registerable,
         :recoverable, :rememberable, :validatable
  has_many :works, :through => :purchases
  has_many :followings
  has_many :purchases
  has_many :reading_list_items
  has_many :works_to_read, through: :reading_list_items, source: :work
  has_many :reviews
end

And the relevant schema from db/schema.rb.

create_table "readers", force: :cascade do |t|
    t.string "email", default: "", null: false
    t.string "encrypted_password", default: "", null: false
    t.string "reset_password_token"
    t.datetime "reset_password_sent_at"
    t.datetime "remember_created_at"
    t.datetime "created_at", precision: 6, null: false
    t.datetime "updated_at", precision: 6, null: false
    t.string "first_name"
    t.string "last_name"
    t.index ["email"], name: "index_readers_on_email", unique: true
    t.index ["reset_password_token"], name: "index_readers_on_reset_password_token", unique: true
end

The Current Reader Tests

I'm using RSpec to test my Rails application, along with FactoryBot for my factories.

I have unit tests in spec/models/reader_spec.rb. Here's what I test for:

# frozen_string_literal: true

require 'rails_helper'

RSpec.describe Reader, type: :model do
  it 'has a valid factory' do
    expect(build(:reader)).to be_valid
  end
  describe 'associations' do
    it { should have_many(:followings) }
    it { should have_many(:purchases) }
    it { should have_many(:reading_list_items) }
    it { should have_many(:reviews) }
    it { should have_many(:works) }
  end
end

The ReaderProfile Model

I can create my new ReaderProfile model with the command:

rails g model ReaderProfile first_name:string last_name:string reader:references

This creates the relevant ActiveRecord class, Rails migration, and tests for the class.

Testing the ReaderProfile

I'm going to make first_name and last_name information optional for readers, so all I want to do is make sure I've got a test in spec/models/reader_profile_spec.rb that checks for a valid factory and it belongs to a Reader object. Here's what that unit test looks like:

RSpec.describe ReaderProfile, type: :model do
  it 'has a valid factory' do
    expect(build(:reader_profile)).to be_valid
  end
end

My factory in spec/factories/reader_profiles.rb looks like:

FactoryBot.define do
  factory :reader_profile do
    first_name { "MyString" }
    last_name { "MyString" }
    reader { create(:reader) }
  end
end

I add the belongs_to code in my ReaderProfile model like so:

# app/models/reader_profile.rb
class ReaderProfile < ApplicationRecord
    belongs_to :reader
end

And when I run rspec spec/models/reader_profile_spec.rb, my tests pass.

Adding to the Reader Tests

I want every Reader object to have one ReaderProfile. To test that, I add the following to spec/models/reader_spec.rb:

it { should have_one(:reader_profile) }

And I update the Reader model to look like:

# frozen_string_literal: true

class Reader < ApplicationRecord
  # Include default devise modules. Others available are:
  # :confirmable, :lockable, :timeoutable, :trackable and :omniauthable
  devise :database_authenticatable, :registerable,
         :recoverable, :rememberable, :validatable
  has_many :works, :through => :purchases
  has_many :followings
  has_many :purchases
  has_many :reading_list_items
  has_many :works_to_read, through: :reading_list_items, source: :work
  has_many :reviews
  has_one :reader_profile
end

When I run rspec spec/models/reader_spec, everything passes.

Updating the impacted Views and Controllers

I've been focusing on the model layer of this refactoring. It's worth noting that I have current_reader.first_name and current_reader.last_name strewn through my application. I'll need to find-and-replace those instances to current_reader.reader_profile.first_name and current_reader.reader_profile.last_name.

I'll also need to drop the first_name and last_name parameters from the ReadersController, and create endpoints for readers to create, update, and delete their profiles.

Conclusion

Aside from my remaining TODOs, I've made good progress today. Now I have a ReaderProfile model to encapsulate personal information associated with each Reader. This makes it easier to add more pieces of Reader data without bloating the authentication class. It also means I can extend my authentication class to include different types of profiles if I like. Here's what my refactoring is really saying:

  • The Reader model handles authentication and authorization for readers. When a reader signs in, they can access their associated ReaderProfile.
  • The ReaderProfile model stores personal information about the Reader it belongs to.