Skip to content

Commit

Permalink
Convert page editing user methods into AR relations
Browse files Browse the repository at this point in the history
This allows to proper eager load the user class.
  • Loading branch information
tvdeyen committed Oct 28, 2019
1 parent 8c327a4 commit 24bef77
Show file tree
Hide file tree
Showing 4 changed files with 106 additions and 144 deletions.
46 changes: 45 additions & 1 deletion app/models/alchemy/page.rb
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,24 @@ class Page < BaseRecord

belongs_to :language, optional: true

belongs_to :creator,
primary_key: Alchemy.user_class.primary_key,
class_name: Alchemy.user_class_name,
foreign_key: :creator_id,
optional: true

belongs_to :updater,
primary_key: Alchemy.user_class.primary_key,
class_name: Alchemy.user_class_name,
foreign_key: :updater_id,
optional: true

belongs_to :locker,
primary_key: Alchemy.user_class.primary_key,
class_name: Alchemy.user_class_name,
foreign_key: :locked_by,
optional: true

has_one :site, through: :language
has_many :site_languages, through: :site, source: :languages
has_many :folded_pages
Expand Down Expand Up @@ -129,7 +147,6 @@ class Page < BaseRecord
include Alchemy::Page::PageScopes
include Alchemy::Page::PageNatures
include Alchemy::Page::PageNaming
include Alchemy::Page::PageUsers
include Alchemy::Page::PageElements

# site_name accessor
Expand Down Expand Up @@ -488,6 +505,33 @@ def public_until
attribute_fixed?(:public_until) ? fixed_attributes[:public_until] : self[:public_until]
end

# Returns the name of the creator of this page.
#
# If no creator could be found or associated user model
# does not respond to +#name+ it returns +'unknown'+
#
def creator_name
creator.try(:name) || Alchemy.t('unknown')
end

# Returns the name of the last updater of this page.
#
# If no updater could be found or associated user model
# does not respond to +#name+ it returns +'unknown'+
#
def updater_name
updater.try(:name) || Alchemy.t('unknown')
end

# Returns the name of the user currently editing this page.
#
# If no locker could be found or associated user model
# does not respond to +#name+ it returns +'unknown'+
#
def locker_name
locker.try(:name) || Alchemy.t('unknown')
end

private

def set_fixed_attributes
Expand Down
60 changes: 0 additions & 60 deletions app/models/alchemy/page/page_users.rb

This file was deleted.

5 changes: 2 additions & 3 deletions spec/features/admin/dashboard_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,16 +31,15 @@
end

context 'When locked by another user' do
let(:other_user) { create(:alchemy_dummy_user, :as_admin, name: "Sue Smith") }
let(:other_user) { create(:alchemy_dummy_user, :as_admin) }

it "shows the name of the user who locked the page" do
a_page.lock_to!(other_user)
allow(user.class).to receive(:find_by).and_return(other_user)
visit admin_dashboard_path
locked_pages_widget = all('div[@class="widget"]').first
expect(locked_pages_widget).to have_content "Currently locked pages"
expect(locked_pages_widget).to have_content a_page.name
expect(locked_pages_widget).to have_content "Sue Smith"
expect(locked_pages_widget).to have_content other_user.name
end
end
end
Expand Down
139 changes: 59 additions & 80 deletions spec/models/alchemy/page_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2060,153 +2060,132 @@ module Alchemy
end
end

context 'indicate page editors' do
let(:page) { Page.new }
describe 'page editor methods' do
let(:user) { create(:alchemy_dummy_user, :as_editor) }

describe '#creator' do
before { page.update(creator_id: user.id) }
let(:page) { Page.new(creator: user) }
subject(:creator) { page.creator }

it "returns the user that created the page" do
expect(page.creator).to eq(user)
is_expected.to eq(user)
end

context 'with user class having a different primary key' do
before do
allow(Alchemy.user_class)
.to receive(:primary_key)
.and_return('user_id')

allow(page)
.to receive(:creator_id)
.and_return(1)
end

it "returns the user that created the page" do
expect(Alchemy.user_class)
.to receive(:find_by)
.with({'user_id' => 1})

page.creator
end
it 'uses the primary key defined on user class' do
expect(Alchemy.user_class).to receive(:primary_key).at_least(:once) { :id }
subject
end
end

describe '#updater' do
before { page.update(updater_id: user.id) }
let(:page) { Page.new(updater: user) }
subject(:updater) { page.updater }

it "returns the user that updated the page" do
expect(page.updater).to eq(user)
is_expected.to eq(user)
end

context 'with user class having a different primary key' do
before do
allow(Alchemy.user_class)
.to receive(:primary_key)
.and_return('user_id')

allow(page)
.to receive(:updater_id)
.and_return(1)
end

it "returns the user that updated the page" do
expect(Alchemy.user_class)
.to receive(:find_by)
.with({'user_id' => 1})

page.updater
end
it 'uses the primary key defined on user class' do
expect(Alchemy.user_class).to receive(:primary_key).at_least(:once) { :id }
subject
end
end

describe '#locker' do
before { page.update(locked_by: user.id) }
let(:page) { Page.new(locker: user) }
subject(:locker) { page.locker }

it "returns the user that locked the page" do
expect(page.locker).to eq(user)
it "returns the user that updated the page" do
is_expected.to eq(user)
end

context 'with user class having a different primary key' do
before do
allow(Alchemy.user_class)
.to receive(:primary_key)
.and_return('user_id')
it 'uses the primary key defined on user class' do
expect(Alchemy.user_class).to receive(:primary_key).at_least(:once) { :id }
subject
end
end

allow(page)
.to receive(:locked_by)
.and_return(1)
context 'with user class having a name accessor' do
let(:user) { build(:alchemy_dummy_user, name: 'Paul Page') }

describe '#creator_name' do
let(:page) { Page.new(creator: user) }

it "returns the name of the creator" do
expect(page.creator_name).to eq('Paul Page')
end
end

it "returns the user that locked the page" do
expect(Alchemy.user_class)
.to receive(:find_by)
.with({'user_id' => 1})
describe '#updater_name' do
let(:page) { Page.new(updater: user) }

page.locker
it "returns the name of the updater" do
expect(page.updater_name).to eq('Paul Page')
end
end
end

context 'with user that can not be found' do
it 'does not raise not found error' do
%w(creator updater locker).each do |user_type|
expect {
page.send(user_type)
}.to_not raise_error
describe '#locker_name' do
let(:page) { Page.new(locker: user) }

it "returns the name of the current page editor" do
expect(page.locker_name).to eq('Paul Page')
end
end
end

context 'with user class having a name accessor' do
let(:user) { double(name: 'Paul Page') }
context 'with user class returning nil for name' do
let(:user) { Alchemy.user_class.new }

describe '#creator_name' do
before { allow(page).to receive(:creator).and_return(user) }
let(:page) { Page.new(creator: user) }

it "returns the name of the creator" do
expect(page.creator_name).to eq('Paul Page')
it "returns unknown" do
expect(page.creator_name).to eq('unknown')
end
end

describe '#updater_name' do
before { allow(page).to receive(:updater).and_return(user) }
let(:page) { Page.new(updater: user) }

it "returns the name of the updater" do
expect(page.updater_name).to eq('Paul Page')
it "returns unknown" do
expect(page.updater_name).to eq('unknown')
end
end

describe '#locker_name' do
before { allow(page).to receive(:locker).and_return(user) }
let(:page) { Page.new(locker: user) }

it "returns the name of the current page editor" do
expect(page.locker_name).to eq('Paul Page')
it "returns unknown" do
expect(page.locker_name).to eq('unknown')
end
end
end

context 'with user class not having a name accessor' do
context 'with user class not responding to name' do
let(:user) { Alchemy.user_class.new }

before do
expect(user).to receive(:respond_to?).with(:name) { false }
end

describe '#creator_name' do
before { allow(page).to receive(:creator).and_return(user) }
let(:page) { Page.new(creator: user) }

it "returns unknown" do
expect(page.creator_name).to eq('unknown')
end
end

describe '#updater_name' do
before { allow(page).to receive(:updater).and_return(user) }
let(:page) { Page.new(updater: user) }

it "returns unknown" do
expect(page.updater_name).to eq('unknown')
end
end

describe '#locker_name' do
before { allow(page).to receive(:locker).and_return(user) }
let(:page) { Page.new(locker: user) }

it "returns unknown" do
expect(page.locker_name).to eq('unknown')
Expand Down

0 comments on commit 24bef77

Please sign in to comment.