lino.jorzick/greenlight
1
0
Fork 0
forked from External/greenlight
Code Issues Pull Requests Projects Releases Wiki Activity

GRN2-xx: Switch the relation between users and roles to make queries cleaner and faster (#1299)

Browse Source 
* First steps

* Fixes in account creation flow

* Fixed most testcases

* more test fixes

* Fixed more test cases

* Passing tests and rubocop

* Added rake task to remove rooms
This commit is contained in:
Ahmad Farhat
2020-05-06 15:23:28 -04:00
committed by GitHub
parent 8f454cad0e
commit 467947f1b5
37 changed files with 262 additions and 402 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
spec/controllers/account_activations_controller_spec.rb
View File

@ -70,7 +70,8 @@ describe AccountActivationsController, type: :controller do
it "redirects a pending user to root with a flash" do
@user = create(:user, email_verified: false, provider: "greenlight")
@user.add_role :pending
@user.set_role :pending
@user.reload
get :edit, params: { token: @user.create_activation_token }

28
spec/controllers/admins_controller_spec.rb
View File

@ -25,7 +25,7 @@ describe AdminsController, type: :controller do
@user = create(:user, provider: "provider1")
@admin = create(:user, provider: "provider1")
@admin.add_role :admin
@admin.set_role :admin
end
describe "User Roles" do
@ -78,7 +78,7 @@ describe AdminsController, type: :controller do
context "POST #unban" do
it "unbans the user from the application" do
@request.session[:user_id] = @admin.id
@user.add_role :denied
@user.set_role :denied
expect(@user.has_role?(:denied)).to eq(true)
@ -153,7 +153,7 @@ describe AdminsController, type: :controller do
it "approves a pending user" do
@request.session[:user_id] = @admin.id
@user.add_role :pending
@user.set_role :pending
post :approve, params: { user_uid: @user.uid }
@ -167,7 +167,7 @@ describe AdminsController, type: :controller do
it "sends the user an email telling them theyre approved" do
@request.session[:user_id] = @admin.id
@user.add_role :pending
@user.set_role :pending
params = { user_uid: @user.uid }
expect { post :approve, params: params }.to change { ActionMailer::Base.deliveries.count }.by(1)
end
@ -245,7 +245,7 @@ describe AdminsController, type: :controller do
Role.create_new_role("test", "greenlight").update_all_role_permissions(can_manage_users: true)
@user2 = create(:user)
@user2.add_role(:test)
@user2.set_role(:test)
# Random manage user action test
@ -266,7 +266,7 @@ describe AdminsController, type: :controller do
Role.create_new_role("test", "greenlight").update_all_role_permissions(can_manage_users: false)
@user2 = create(:user)
@user2.add_role(:test)
@user2.set_role(:test)
# Random manage user action test
@ -450,7 +450,7 @@ describe AdminsController, type: :controller do
@request.session[:user_id] = @admin.id
@admin.add_role :super_admin
@admin.set_role :super_admin
@admin.update_attribute(:provider, "greenlight")
@user2 = create(:user, provider: "provider1")
@user3 = create(:user, provider: "provider1")
@ -479,7 +479,7 @@ describe AdminsController, type: :controller do
it "changes the log level" do
@request.session[:user_id] = @admin.id
@admin.add_role :super_admin
@admin.set_role :super_admin
expect(Rails.logger.level).to eq(0)
post :log_level, params: { value: 2 }
@ -492,7 +492,7 @@ describe AdminsController, type: :controller do
Role.create_new_role("test", "greenlight").update_all_role_permissions(can_edit_site_settings: true)
@user2 = create(:user)
@user2.add_role(:test)
@user2.set_role(:test)
# Random edit site settings action test
@ -510,7 +510,7 @@ describe AdminsController, type: :controller do
Role.create_new_role("test", "greenlight").update_all_role_permissions(can_manage_users: true)
@user2 = create(:user)
@user2.add_role(:test)
@user2.set_role(:test)
# Random edit site settings action test
@ -610,7 +610,7 @@ describe AdminsController, type: :controller do
new_role2 = Role.create_new_role("test2", "provider1")
new_role2.update_permission("can_edit_roles", "true")
@user.roles << new_role2
@user.role = new_role2
@user.save!
@request.session[:user_id] = @user.id
@ -657,7 +657,7 @@ describe AdminsController, type: :controller do
new_role2 = Role.create(name: "test2", priority: 2, provider: "provider1")
new_role2.update_permission("can_edit_roles", "true")
@user.roles << new_role2
@user.role = new_role2
@user.save!
@request.session[:user_id] = @user.id
@ -743,7 +743,7 @@ describe AdminsController, type: :controller do
Role.create_new_role("test", "greenlight").update_all_role_permissions(can_edit_roles: true)
@user2 = create(:user)
@user2.add_role(:test)
@user2.set_role(:test)
# Random edit roles action test
@ -764,7 +764,7 @@ describe AdminsController, type: :controller do
Role.create_new_role("test", "greenlight").update_all_role_permissions(can_manage_users: false)
@user2 = create(:user)
@user2.add_role(:test)
@user2.set_role(:test)
# Random edit roles action test

4
spec/controllers/application_controller_spec.rb
View File

@ -43,7 +43,7 @@ describe ApplicationController do
end
it "redirects a banned user to a 401 and logs them out" do
@user.add_role :denied
@user.set_role :denied
@request.session[:user_id] = @user.id
get :index
@ -53,7 +53,7 @@ describe ApplicationController do
end
it "redirects a pending user to a 401 and logs them out" do
@user.add_role :pending
@user.set_role :pending
@request.session[:user_id] = @user.id
get :index

30
spec/controllers/rooms_controller_spec.rb
View File

@ -64,7 +64,7 @@ describe RoomsController, type: :controller do
end
it "should render cant_create_rooms if user doesn't have permission to create rooms" do
user_role = @user.highest_priority_role
user_role = @user.role
user_role.update_permission("can_create_rooms", "false")
user_role.save!
@ -117,7 +117,7 @@ describe RoomsController, type: :controller do
it "redirects to admin if user is a super_admin" do
@request.session[:user_id] = @owner.id
@owner.add_role :super_admin
@owner.set_role :super_admin
get :show, params: { room_uid: @owner.main_room, search: :none }
@ -140,7 +140,7 @@ describe RoomsController, type: :controller do
it "redirects to root if owner is pending" do
@request.session[:user_id] = @owner.id
@owner.add_role :pending
@owner.set_role :pending
get :show, params: { room_uid: @owner.main_room, search: :none }
@ -149,7 +149,7 @@ describe RoomsController, type: :controller do
it "redirects to root if owner is banned" do
@request.session[:user_id] = @owner.id
@owner.add_role :denied
@owner.set_role :denied
get :show, params: { room_uid: @owner.main_room, search: :none }
@ -406,7 +406,7 @@ describe RoomsController, type: :controller do
it "redirects to root if owner is pending" do
@request.session[:user_id] = @owner.id
@owner.add_role :pending
@owner.set_role :pending
post :join, params: { room_uid: @room }
@ -415,7 +415,7 @@ describe RoomsController, type: :controller do
it "redirects to root if owner is banned" do
@request.session[:user_id] = @owner.id
@owner.add_role :denied
@owner.set_role :denied
post :join, params: { room_uid: @room }
@ -456,7 +456,7 @@ describe RoomsController, type: :controller do
it "allows admin to delete room" do
@admin = create(:user)
@admin.add_role :admin
@admin.set_role :admin
@request.session[:user_id] = @admin.id
expect do
@ -468,7 +468,7 @@ describe RoomsController, type: :controller do
it "does not allow admin to delete a users home room" do
@admin = create(:user)
@admin.add_role :admin
@admin.set_role :admin
@request.session[:user_id] = @admin.id
expect do
@ -483,7 +483,7 @@ describe RoomsController, type: :controller do
allow_any_instance_of(User).to receive(:admin_of?).and_return(false)
@admin = create(:user)
@admin.add_role :admin
@admin.set_role :admin
@request.session[:user_id] = @admin.id
expect do
@ -527,7 +527,7 @@ describe RoomsController, type: :controller do
it "redirects to join path if admin" do
@admin = create(:user)
@admin.add_role :admin
@admin.set_role :admin
@request.session[:user_id] = @admin.id
post :start, params: { room_uid: @user.main_room }
@ -538,7 +538,7 @@ describe RoomsController, type: :controller do
it "redirects to root path if not admin of current user" do
allow_any_instance_of(User).to receive(:admin_of?).and_return(false)
@admin = create(:user)
@admin.add_role :admin
@admin.set_role :admin
@request.session[:user_id] = @admin.id
post :start, params: { room_uid: @user.main_room }
@ -587,7 +587,7 @@ describe RoomsController, type: :controller do
it "allows admin to update room settings" do
@admin = create(:user)
@admin.add_role :admin
@admin.set_role :admin
@request.session[:user_id] = @admin.id
room_params = { "mute_on_join": "1", "name": @secondary_room.name }
@ -603,7 +603,7 @@ describe RoomsController, type: :controller do
it "does not allow admins from a different context to update room settings" do
allow_any_instance_of(User).to receive(:admin_of?).and_return(false)
@admin = create(:user)
@admin.add_role :admin
@admin.set_role :admin
@request.session[:user_id] = @admin.id
room_params = { "mute_on_join": "1", "name": @secondary_room.name }
@ -743,7 +743,7 @@ describe RoomsController, type: :controller do
it "allows admins to update room access" do
@admin = create(:user)
@admin.add_role :admin
@admin.set_role :admin
@request.session[:user_id] = @admin.id
post :shared_access, params: { room_uid: @room.uid, add: [@user1.uid] }
@ -756,7 +756,7 @@ describe RoomsController, type: :controller do
it "redirects to root path if not admin of current user" do
allow_any_instance_of(User).to receive(:admin_of?).and_return(false)
@admin = create(:user)
@admin.add_role :admin
@admin.set_role :admin
@request.session[:user_id] = @admin.id
post :shared_access, params: { room_uid: @room.uid, add: [] }

10
spec/controllers/sessions_controller_spec.rb
View File

@ -221,7 +221,7 @@ describe SessionsController, type: :controller do
it "redirects to the admins page for admins" do
user = create(:user, provider: "greenlight",
password: "example", password_confirmation: 'example')
user.add_role :super_admin
user.set_role :super_admin
post :create, params: {
session: {
@ -235,7 +235,7 @@ describe SessionsController, type: :controller do
end
it "should migrate old rooms from the twitter account to the new user" do
twitter_user = User.create(name: "Twitter User", email: "user@twitter.com", image: "example.png",
twitter_user = create(:user, name: "Twitter User", email: "user@twitter.com", image: "example.png",
username: "twitteruser", email_verified: true, provider: 'twitter', social_uid: "twitter-user")
room = Room.new(name: "Test")
@ -383,7 +383,7 @@ describe SessionsController, type: :controller do
it "should notify twitter users that twitter is deprecated" do
allow(Rails.configuration).to receive(:allow_user_signup).and_return(true)
twitter_user = User.create(name: "Twitter User", email: "user@twitter.com", image: "example.png",
twitter_user = create(:user, name: "Twitter User", email: "user@twitter.com", image: "example.png",
username: "twitteruser", email_verified: true, provider: 'twitter', social_uid: "twitter-user")
request.env["omniauth.auth"] = OmniAuth.config.mock_auth[:twitter]
@ -394,7 +394,7 @@ describe SessionsController, type: :controller do
end
it "should migrate rooms from the twitter account to the google account" do
twitter_user = User.create(name: "Twitter User", email: "user@twitter.com", image: "example.png",
twitter_user = create(:user, name: "Twitter User", email: "user@twitter.com", image: "example.png",
username: "twitteruser", email_verified: true, provider: 'twitter', social_uid: "twitter-user")
room = Room.new(name: "Test")
@ -419,7 +419,7 @@ describe SessionsController, type: :controller do
allow(Rails.configuration).to receive(:enable_email_verification).and_return(true)
@user = create(:user, provider: "greenlight")
@admin = create(:user, provider: "greenlight", email: "test@example.com")
@admin.add_role :admin
@admin.set_role :admin
end
it "should notify admin on new user signup with approve/reject registration" do

86
spec/controllers/users_controller_spec.rb
View File

@ -75,7 +75,7 @@ describe UsersController, type: :controller do
controller.instance_variable_set(:@user_domain, "provider1")
user = create(:user, provider: "provider1")
user.add_role :admin
user.set_role :admin
user2 = create(:user, provider: "provider1")
@request.session[:user_id] = user.id
@ -174,7 +174,7 @@ describe UsersController, type: :controller do
allow(Rails.configuration).to receive(:allow_user_signup).and_return(true)
@user = create(:user, provider: "greenlight")
@admin = create(:user, provider: "greenlight", email: "test@example.com")
@admin.add_role :admin
@admin.set_role :admin
end
it "should notify admins that user signed up" do
@ -232,7 +232,7 @@ describe UsersController, type: :controller do
allow(Rails.configuration).to receive(:allow_user_signup).and_return(true)
@user = create(:user, provider: "greenlight")
@admin = create(:user, provider: "greenlight", email: "test@example.com")
@admin.add_role :admin
@admin.set_role :admin
end
it "allows any user to sign up" do
@ -278,13 +278,13 @@ describe UsersController, type: :controller do
end
end
describe "PATCH #update" do
describe "POST #update" do
it "properly updates user attributes" do
user = create(:user)
@request.session[:user_id] = user.id
params = random_valid_user_params
patch :update, params: params.merge!(user_uid: user)
post :update, params: params.merge!(user_uid: user)
user.reload
expect(user.name).to eql(params[:user][:name])
@ -297,7 +297,7 @@ describe UsersController, type: :controller do
@user = create(:user)
@request.session[:user_id] = @user.id
patch :update, params: invalid_params.merge!(user_uid: @user)
post :update, params: invalid_params.merge!(user_uid: @user)
expect(response).to render_template(:edit)
end
@ -306,7 +306,7 @@ describe UsersController, type: :controller do
user = create(:user)
@request.session[:user_id] = user.id
user_role = user.highest_priority_role
user_role = user.role
user_role.update_permission("can_manage_users", "true")
@ -315,30 +315,7 @@ describe UsersController, type: :controller do
tmp_role = Role.create(name: "test", priority: -4, provider: "greenlight")
params = random_valid_user_params
patch :update, params: params.merge!(user_uid: user, user: { role_ids: tmp_role.id.to_s })
expect(flash[:alert]).to eq(I18n.t("administrator.roles.invalid_assignment"))
expect(response).to render_template(:edit)
end
it "should fail to update roles if a user tries to remove a role with a higher priority than their own" do
user = create(:user)
admin = create(:user)
admin.add_role :admin
@request.session[:user_id] = user.id
user_role = user.highest_priority_role
user_role.update_permission("can_manage_users", "true")
user_role.save!
params = random_valid_user_params
patch :update, params: params.merge!(user_uid: admin, user: { role_ids: "" })
user.reload
post :update, params: params.merge!(user_uid: user, user: { role_id: tmp_role.id.to_s })
expect(flash[:alert]).to eq(I18n.t("administrator.roles.invalid_assignment"))
expect(response).to render_template(:edit)
@ -350,53 +327,30 @@ describe UsersController, type: :controller do
user = create(:user)
admin = create(:user)
admin.add_role :admin
admin.set_role :admin
@request.session[:user_id] = admin.id
tmp_role1 = Role.create(name: "test1", priority: 2, provider: "greenlight")
tmp_role1.update_permission("send_promoted_email", "true")
tmp_role2 = Role.create(name: "test2", priority: 3, provider: "greenlight")
params = random_valid_user_params
params = params.merge!(user_uid: user, user: { role_ids: "#{tmp_role1.id} #{tmp_role2.id}" })
params = params.merge!(user_uid: user, user: { role_id: tmp_role1.id.to_s })
expect { patch :update, params: params }.to change { ActionMailer::Base.deliveries.count }.by(1)
expect { post :update, params: params }.to change { ActionMailer::Base.deliveries.count }.by(1)
user.reload
expect(user.roles.count).to eq(2)
expect(user.highest_priority_role.name).to eq("test1")
expect(response).to redirect_to(admins_path)
end
it "all users must at least have the user role" do
allow(Rails.configuration).to receive(:enable_email_verification).and_return(true)
user = create(:user)
admin = create(:user)
admin.add_role :admin
tmp_role1 = Role.create(name: "test1", priority: 2, provider: "greenlight")
tmp_role1.update_permission("send_demoted_email", "true")
user.roles << tmp_role1
user.save!
@request.session[:user_id] = admin.id
params = random_valid_user_params
params = params.merge!(user_uid: user, user: { role_ids: "" })
expect { patch :update, params: params }.to change { ActionMailer::Base.deliveries.count }.by(1)
expect(user.roles.count).to eq(1)
expect(user.highest_priority_role.name).to eq("user")
expect(user.role.name).to eq("test1")
expect(response).to redirect_to(admins_path)
end
end
end
describe "DELETE #user" do
before { allow(Rails.configuration).to receive(:allow_user_signup).and_return(true) }
before do
allow(Rails.configuration).to receive(:allow_user_signup).and_return(true)
Role.create_default_roles("provider1")
end
it "permanently deletes user" do
user = create(:user)
@ -416,7 +370,7 @@ describe UsersController, type: :controller do
user = create(:user, provider: "provider1")
admin = create(:user, provider: "provider1")
admin.add_role :admin
admin.set_role :admin
@request.session[:user_id] = admin.id
delete :destroy, params: { user_uid: user.uid }
@ -434,7 +388,7 @@ describe UsersController, type: :controller do
user = create(:user, provider: "provider1")
admin = create(:user, provider: "provider1")
admin.add_role :admin
admin.set_role :admin
@request.session[:user_id] = admin.id
delete :destroy, params: { user_uid: user.uid, permanent: "true" }
@ -452,7 +406,7 @@ describe UsersController, type: :controller do
user = create(:user, provider: "provider1")
admin = create(:user, provider: "provider1")
admin.add_role :admin
admin.set_role :admin
@request.session[:user_id] = admin.id
uid = user.main_room.uid
@ -473,7 +427,7 @@ describe UsersController, type: :controller do
user = create(:user, provider: "provider1")
admin = create(:user, provider: "provider2")
admin.add_role :admin
admin.set_role :admin
@request.session[:user_id] = admin.id
delete :destroy, params: { user_uid: user.uid }

1
spec/factories.rb
View File

@ -29,6 +29,7 @@ FactoryBot.define do
accepted_terms { true }
email_verified { true }
activated_at { Time.zone.now }
role { set_role(:user) }
end
factory :room do

45
spec/models/user_spec.rb
View File

@ -170,12 +170,12 @@ describe User, type: :model do
allow_any_instance_of(User).to receive(:greenlight_account?).and_return(true)
@admin = create(:user, provider: @user.provider)
@admin.add_role :admin
@admin.set_role :admin
expect(@admin.admin_of?(@user, "can_manage_users")).to be true
@super_admin = create(:user, provider: "test")
@super_admin.add_role :super_admin
@super_admin.set_role :super_admin
expect(@super_admin.admin_of?(@user, "can_manage_users")).to be true
end
@ -188,32 +188,16 @@ describe User, type: :model do
it "should get the highest priority role" do
@admin = create(:user, provider: @user.provider)
@admin.add_role :admin
@admin.set_role :admin
expect(@admin.highest_priority_role.name).to eq("admin")
end
it "should skip adding the role if the user already has the role" do
@admin = create(:user, provider: @user.provider)
@admin.add_role :admin
@admin.add_role :admin
expect(@admin.roles.count).to eq(2)
expect(@admin.role.name).to eq("admin")
end
it "should add the role if the user doesn't already have the role" do
@admin = create(:user, provider: @user.provider)
@admin.add_role :admin
@admin.set_role :admin
expect(@admin.roles.count).to eq(2)
end
it "should remove the role if the user has the role assigned to them" do
@admin = create(:user, provider: @user.provider)
@admin.add_role :admin
@admin.remove_role :admin
expect(@admin.roles.count).to eq(1)
expect(@admin.has_role?(:admin)).to eq(true)
end
it "has_role? should return false if the user doesn't have the role" do
@ -222,7 +206,7 @@ describe User, type: :model do
it "has_role? should return true if the user has the role" do
@admin = create(:user, provider: @user.provider)
@admin.add_role :admin
@admin.set_role :admin
expect(@admin.has_role?(:admin)).to eq(true)
end
@ -230,8 +214,8 @@ describe User, type: :model do
it "with_role should return all users with the role" do
@admin1 = create(:user, provider: @user.provider)
@admin2 = create(:user, provider: @user.provider)
@admin1.add_role :admin
@admin2.add_role :admin
@admin1.set_role :admin
@admin2.set_role :admin
expect(User.with_role(:admin).count).to eq(2)
end
@ -239,18 +223,11 @@ describe User, type: :model do
it "without_role should return all users without the role" do
@admin1 = create(:user, provider: @user.provider)
@admin2 = create(:user, provider: @user.provider)
@admin1.add_role :admin
@admin2.add_role :admin
@admin1.set_role :admin
@admin2.set_role :admin
expect(User.without_role(:admin).count).to eq(1)
end
it "all_users_with_roles should return all users with at least one role" do
@admin1 = create(:user, provider: @user.provider)
@admin2 = create(:user, provider: @user.provider)
expect(User.all_users_with_roles.count).to eq(3)
end
end
context 'blank email' do

2
spec/spec_helper.rb
View File

@ -108,6 +108,8 @@ RSpec.configure do |config|
<GOOGLE_HD/>
</user>
</response>", headers: {}) if ENV['LOADBALANCER_ENDPOINT']
Role.create_default_roles("greenlight")
end
# rspec-expectations config goes here. You can use an alternate