GRN2-180: First stages of refactoring code for v2.4 (#748)

* Email rescues and authenticator concern

* Application controller and helper clean up

* Moved controller code out of helpers

* More helper and email clean up

* Cleaned up remaining helpers and create omniauth_options

* Controller code clean up

* restructured views structure

* Restructured role code

* Restructured profile and code clean up

* Master merge

* Added bbb server concern to deal with bbb calls

* Bug fixes and changes after changes

* rspec

* More rubocop fixes

spec/controllers/account_activations_controller_spec.rb

@@ -37,7 +37,7 @@ describe AccountActivationsController, type: :controller do
 
       get :show, params: { email: user.email }
 
-      expect(response).to render_template(:verify)
+      expect(response).to render_template(:show)
     end
   end
 

spec/controllers/admins_controller_spec.rb

@@ -91,7 +91,7 @@ describe AdminsController, type: :controller do
     context "POST #invite" do
       before do
         allow(Rails.configuration).to receive(:loadbalanced_configuration).and_return(true)
-        allow_any_instance_of(ApplicationController).to receive(:allow_greenlight_users?).and_return(true)
+        allow_any_instance_of(ApplicationController).to receive(:allow_greenlight_accounts?).and_return(true)
         allow_any_instance_of(User).to receive(:greenlight_account?).and_return(true)
       end
 
@@ -177,7 +177,7 @@ describe AdminsController, type: :controller do
         @request.session[:user_id] = @admin.id
         fake_image_url = "example.com"
 
-        post :branding, params: { url: fake_image_url }
+        post :update_settings, params: { setting: "Branding Image", value: fake_image_url }
 
         feature = Setting.find_by(provider: "provider1").features.find_by(name: "Branding Image")
 
@@ -194,7 +194,7 @@ describe AdminsController, type: :controller do
         @request.session[:user_id] = @admin.id
         primary_color = Faker::Color.hex_color
 
-        post :coloring, params: { color: primary_color }
+        post :coloring, params: { value: primary_color }
 
         feature = Setting.find_by(provider: "provider1").features.find_by(name: "Primary Color")
 
@@ -209,7 +209,7 @@ describe AdminsController, type: :controller do
         @request.session[:user_id] = @admin.id
         primary_color = Faker::Color.hex_color
 
-        post :coloring_lighten, params: { color: primary_color }
+        post :update_settings, params: { setting: "Primary Color Lighten", value: primary_color }
 
         feature = Setting.find_by(provider: "provider1").features.find_by(name: "Primary Color Lighten")
 
@@ -224,7 +224,7 @@ describe AdminsController, type: :controller do
         @request.session[:user_id] = @admin.id
         primary_color = Faker::Color.hex_color
 
-        post :coloring_darken, params: { color: primary_color }
+        post :update_settings, params: { setting: "Primary Color Darken", value: primary_color }
 
         feature = Setting.find_by(provider: "provider1").features.find_by(name: "Primary Color Darken")
 
@@ -243,7 +243,7 @@ describe AdminsController, type: :controller do
 
         @request.session[:user_id] = @admin.id
 
-        post :registration_method, params: { method: "invite" }
+        post :registration_method, params: { value: "invite" }
 
         feature = Setting.find_by(provider: "provider1").features.find_by(name: "Registration Method")
 
@@ -259,7 +259,7 @@ describe AdminsController, type: :controller do
 
         @request.session[:user_id] = @admin.id
 
-        post :registration_method, params: { method: "invite" }
+        post :registration_method, params: { value: "invite" }
 
         expect(flash[:alert]).to be_present
         expect(response).to redirect_to(admin_site_settings_path)
@@ -273,7 +273,7 @@ describe AdminsController, type: :controller do
 
         @request.session[:user_id] = @admin.id
 
-        post :room_authentication, params: { value: "true" }
+        post :update_settings, params: { setting: "Room Authentication", value: "true" }
 
         feature = Setting.find_by(provider: "provider1").features.find_by(name: "Room Authentication")
 
@@ -289,7 +289,7 @@ describe AdminsController, type: :controller do
 
         @request.session[:user_id] = @admin.id
 
-        post :room_limit, params: { limit: 5 }
+        post :update_settings, params: { setting: "Room Limit", value: 5 }
 
         feature = Setting.find_by(provider: "provider1").features.find_by(name: "Room Limit")
 
@@ -305,7 +305,7 @@ describe AdminsController, type: :controller do
 
         @request.session[:user_id] = @admin.id
 
-        post :default_recording_visibility, params: { visibility: "public" }
+        post :update_settings, params: { setting: "Default Recording Visibility", value: "public" }
 
         feature = Setting.find_by(provider: "provider1").features.find_by(name: "Default Recording Visibility")
 
@@ -353,7 +353,7 @@ describe AdminsController, type: :controller do
         post :new_role, params: { role: { name: "admin" } }
 
         expect(response).to redirect_to admin_roles_path
-        expect(flash[:alert]).to eq(I18n.t("administrator.roles.duplicate_name"))
+        expect(flash[:alert]).to eq(I18n.t("administrator.roles.invalid_create"))
       end
 
       it "should fail with empty role name" do
@@ -362,7 +362,7 @@ describe AdminsController, type: :controller do
         post :new_role, params: { role: { name: "    " } }
 
         expect(response).to redirect_to admin_roles_path
-        expect(flash[:alert]).to eq(I18n.t("administrator.roles.empty_name"))
+        expect(flash[:alert]).to eq(I18n.t("administrator.roles.invalid_create"))
       end
 
       it "should create new role and increase user role priority" do
@@ -412,7 +412,7 @@ describe AdminsController, type: :controller do
 
         patch :change_role_order, params: { role: [new_role3.id, new_role2.id] }
 
-        expect(flash[:alert]).to eq(I18n.t("administrator.roles.invalid_update"))
+        expect(flash[:alert]).to eq(I18n.t("administrator.roles.invalid_order"))
         expect(response).to redirect_to admin_roles_path
       end
 
@@ -432,7 +432,7 @@ describe AdminsController, type: :controller do
 
         patch :change_role_order, params: { role: [new_role3.id, new_role2.id] }
 
-        expect(flash[:alert]).to eq(I18n.t("administrator.roles.invalid_update"))
+        expect(flash[:alert]).to eq(I18n.t("administrator.roles.invalid_order"))
         expect(response).to redirect_to admin_roles_path
       end
 
@@ -489,7 +489,7 @@ describe AdminsController, type: :controller do
 
         patch :update_role, params: { role_id: new_role.id, role: { name: "admin" } }
 
-        expect(flash[:alert]).to eq(I18n.t("administrator.roles.duplicate_name"))
+        expect(flash[:alert]).to eq(I18n.t("administrator.roles.invalid_update"))
         expect(response).to redirect_to admin_roles_path(selected_role: new_role.id)
       end
 

spec/controllers/application_controller_spec.rb

@@ -63,6 +63,64 @@ describe ApplicationController do
     end
   end
 
+  context "getters" do
+    it "returns whether user signup is allowed" do
+      allow(Rails.configuration).to receive(:allow_user_signup).and_return(true)
+
+      expect(controller.allow_user_signup?).to eql(true)
+    end
+
+    it "returns whether the default bbb endpoint is being used" do
+      allow(Rails.configuration).to receive(:bigbluebutton_endpoint)
+        .and_return("http://test-install.blindsidenetworks.com/bigbluebutton/api/")
+      allow(Rails.configuration).to receive(:bigbluebutton_endpoint_default)
+        .and_return("http://test-install.blindsidenetworks.com/bigbluebutton/api/")
+
+      expect(controller.bigbluebutton_endpoint_default?).to eql(true)
+    end
+  end
+
+  context "allow_greenlight_accounts" do
+    it "allows if user sign up is turned on" do
+      allow(Rails.configuration).to receive(:loadbalanced_configuration).and_return(false)
+      allow(Rails.configuration).to receive(:allow_user_signup).and_return(true)
+
+      expect(controller.allow_greenlight_accounts?).to eql(true)
+    end
+
+    it "doesn't allow if user sign up is turned off" do
+      allow(Rails.configuration).to receive(:loadbalanced_configuration).and_return(false)
+      allow(Rails.configuration).to receive(:allow_user_signup).and_return(false)
+
+      expect(controller.allow_greenlight_accounts?).to eql(false)
+    end
+
+    it "doesn't allow if user_domain is blank" do
+      allow(Rails.configuration).to receive(:loadbalanced_configuration).and_return(true)
+      allow(Rails.configuration).to receive(:allow_user_signup).and_return(true)
+
+      expect(controller.allow_greenlight_accounts?).to eql(false)
+    end
+
+    it "allows if user provider is set to greenlight" do
+      allow(Rails.configuration).to receive(:loadbalanced_configuration).and_return(true)
+      allow(Rails.configuration).to receive(:allow_user_signup).and_return(true)
+      allow(controller).to receive(:retrieve_provider_info).and_return("provider" => "greenlight")
+      controller.instance_variable_set(:@user_domain, "provider1")
+
+      expect(controller.allow_greenlight_accounts?).to eql(true)
+    end
+
+    it "doesnt allow if user provider is not set to greenlight" do
+      allow(Rails.configuration).to receive(:loadbalanced_configuration).and_return(true)
+      allow(Rails.configuration).to receive(:allow_user_signup).and_return(true)
+      allow(controller).to receive(:retrieve_provider_info).and_return("provider" => "google")
+      controller.instance_variable_set(:@user_domain, "provider1")
+
+      expect(controller.allow_greenlight_accounts?).to eql(false)
+    end
+  end
+
   context "errors" do
     it "renders a BigBlueButton error if a BigBlueButtonException occurrs" do
       routes.draw { get "error" => "anonymous#error" }
@@ -74,7 +132,7 @@ describe ApplicationController do
     it "renders a 404 error if user is not found" do
       allow(Rails.configuration).to receive(:loadbalanced_configuration).and_return(true)
       allow(Rails.env).to receive(:test?).and_return(false)
-      allow_any_instance_of(SessionsHelper).to receive(:parse_user_domain).and_return("fake_provider")
+      allow_any_instance_of(ApplicationController).to receive(:parse_user_domain).and_return("fake_provider")
       allow_any_instance_of(BbbApi).to receive(:retrieve_provider_info).and_raise("No user with that id exists")
 
       routes.draw { get "user_not_found" => "anonymous#user_not_found" }
@@ -86,7 +144,7 @@ describe ApplicationController do
     it "renders a 404 error if user is not given" do
       allow(Rails.configuration).to receive(:loadbalanced_configuration).and_return(true)
       allow(Rails.env).to receive(:test?).and_return(false)
-      allow_any_instance_of(SessionsHelper).to receive(:parse_user_domain).and_return("")
+      allow_any_instance_of(ApplicationController).to receive(:parse_user_domain).and_return("")
       allow_any_instance_of(BbbApi).to receive(:retrieve_provider_info).and_raise("Provider not included.")
 
       routes.draw { get "user_not_found" => "anonymous#user_not_found" }
@@ -98,7 +156,7 @@ describe ApplicationController do
     it "renders a 500 error if any other error related to bbb api" do
       allow(Rails.configuration).to receive(:loadbalanced_configuration).and_return(true)
       allow(Rails.env).to receive(:test?).and_return(false)
-      allow_any_instance_of(SessionsHelper).to receive(:parse_user_domain).and_return("")
+      allow_any_instance_of(ApplicationController).to receive(:parse_user_domain).and_return("")
       allow_any_instance_of(BbbApi).to receive(:retrieve_provider_info).and_raise("Other error")
 
       routes.draw { get "user_not_found" => "anonymous#user_not_found" }

spec/controllers/password_resets_controller_spec.rb

@@ -73,7 +73,9 @@ describe PasswordResetsController, type: :controller do
   end
 
   describe "PATCH #update" do
-    before { allow(Rails.configuration).to receive(:enable_email_verification).and_return(true) }
+    before do
+      allow(Rails.configuration).to receive(:enable_email_verification).and_return(true)
+    end
 
     context "valid user" do
       it "reloads page with notice if password is empty" do
@@ -120,7 +122,7 @@ describe PasswordResetsController, type: :controller do
 
         allow(controller).to receive(:valid_user).and_return(nil)
         allow(controller).to receive(:check_expiration).and_return(nil)
-        allow(controller).to receive(:current_user).and_return(user)
+        controller.instance_variable_set(:@user, user)
 
         params = {
           id: token,

spec/controllers/recordings_controller_spec.rb

@@ -27,10 +27,10 @@ describe RecordingsController, type: :controller do
 
   context "POST #update_recording" do
     it "updates the recordings details" do
-      allow_any_instance_of(Room).to receive(:update_recording).and_return(updated: true)
+      allow_any_instance_of(BbbServer).to receive(:update_recording).and_return(updated: true)
       @request.session[:user_id] = @user.uid
 
-      post :update_recording, params: { meetingID: @room.bbb_id, record_id: Faker::IDNumber.valid, state: "public" }
+      post :update, params: { meetingID: @room.bbb_id, record_id: Faker::IDNumber.valid, state: "public" }
 
       expect(response).to have_http_status(302)
     end
@@ -38,7 +38,7 @@ describe RecordingsController, type: :controller do
     it "redirects to root if not the room owner" do
       @request.session[:user_id] = @secondary_user.uid
 
-      post :update_recording, params: { meetingID: @room.bbb_id, record_id: Faker::IDNumber.valid, state: "public" }
+      post :update, params: { meetingID: @room.bbb_id, record_id: Faker::IDNumber.valid, state: "public" }
 
       expect(response).to redirect_to(root_path)
     end
@@ -46,10 +46,10 @@ describe RecordingsController, type: :controller do
 
   context "DELETE #delete_recording" do
     it "deletes the recording" do
-      allow_any_instance_of(Room).to receive(:delete_recording).and_return(true)
+      allow_any_instance_of(BbbServer).to receive(:delete_recording).and_return(true)
       @request.session[:user_id] = @user.uid
 
-      post :delete_recording, params: { meetingID: @room.bbb_id, record_id: Faker::IDNumber.valid, state: "public" }
+      post :delete, params: { meetingID: @room.bbb_id, record_id: Faker::IDNumber.valid, state: "public" }
 
       expect(response).to have_http_status(302)
     end
@@ -57,7 +57,7 @@ describe RecordingsController, type: :controller do
     it "redirects to root if not the room owner" do
       @request.session[:user_id] = @secondary_user.uid
 
-      post :delete_recording, params: { meetingID: @room.bbb_id, record_id: Faker::IDNumber.valid, state: "public" }
+      post :delete, params: { meetingID: @room.bbb_id, record_id: Faker::IDNumber.valid, state: "public" }
 
       expect(response).to redirect_to(root_path)
     end

spec/controllers/rooms_controller_spec.rb

@@ -28,8 +28,11 @@ def random_valid_room_params
 end
 
 describe RoomsController, type: :controller do
-  it_behaves_like "recorder"
   include Recorder
+  include BbbServer
+
+  let(:bbb_server) { BigBlueButton::BigBlueButtonApi.new("http://bbb.example.com/bigbluebutton/api", "secret", "0.8") }
+
   describe "GET #show" do
     before do
       @user = create(:user)
@@ -41,8 +44,7 @@ describe RoomsController, type: :controller do
 
       get :show, params: { room_uid: @owner.main_room }
 
-      expect(assigns(:recordings)).to eql(recordings(@owner.main_room.bbb_id, @owner.provider))
-      expect(assigns(:is_running)).to eql(@owner.main_room.running?)
+      expect(assigns(:recordings)).to eql(recordings(@owner.main_room.bbb_id))
     end
 
     it "should be able to search recordings if user is owner" do
@@ -199,11 +201,6 @@ describe RoomsController, type: :controller do
       @user = create(:user)
       @owner = create(:user)
       @room = @owner.main_room
-
-      allow_any_instance_of(BigBlueButton::BigBlueButtonApi).to receive(:get_meeting_info).and_return(
-        moderatorPW: "modpass",
-        attendeePW: "attpass",
-      )
     end
 
     it "should use account name if user is logged in and meeting running" do
@@ -212,7 +209,7 @@ describe RoomsController, type: :controller do
       @request.session[:user_id] = @user.id
       post :join, params: { room_uid: @room, join_name: @user.name }
 
-      expect(response).to redirect_to(@owner.main_room.join_path(@user.name, {}, @user.uid))
+      expect(response).to redirect_to(join_path(@owner.main_room, @user.name, {}, @user.uid))
     end
 
     it "should use join name if user is not logged in and meeting running" do
@@ -220,7 +217,7 @@ describe RoomsController, type: :controller do
 
       post :join, params: { room_uid: @room, join_name: "Join Name" }
 
-      expect(response).to redirect_to(@owner.main_room.join_path("Join Name", {}))
+      expect(response).to redirect_to(join_path(@owner.main_room, "Join Name", {}))
     end
 
     it "should render wait if meeting isn't running" do
@@ -243,7 +240,7 @@ describe RoomsController, type: :controller do
       @request.session[:user_id] = @user.id
       post :join, params: { room_uid: room, join_name: @user.name }
 
-      expect(response).to redirect_to(room.join_path(@user.name, { user_is_moderator: false }, @user.uid))
+      expect(response).to redirect_to(join_path(room, @user.name, { user_is_moderator: false }, @user.uid))
     end
 
     it "should join the room as moderator if room has the all_join_moderator setting" do
@@ -257,7 +254,7 @@ describe RoomsController, type: :controller do
       @request.session[:user_id] = @user.id
       post :join, params: { room_uid: room, join_name: @user.name }
 
-      expect(response).to redirect_to(room.join_path(@user.name, { user_is_moderator: true }, @user.uid))
+      expect(response).to redirect_to(join_path(room, @user.name, { user_is_moderator: true }, @user.uid))
     end
 
     it "should render wait if the correct access code is supplied" do
@@ -292,7 +289,7 @@ describe RoomsController, type: :controller do
       @request.session[:user_id] = @owner.id
       post :join, params: { room_uid: @room, join_name: @owner.name }
 
-      expect(response).to redirect_to(@owner.main_room.join_path(@owner.name, { user_is_moderator: true }, @owner.uid))
+      expect(response).to redirect_to(join_path(@owner.main_room, @owner.name, { user_is_moderator: true }, @owner.uid))
     end
 
     it "redirects to root if owner of room is not verified" do
@@ -362,7 +359,7 @@ describe RoomsController, type: :controller do
       @request.session[:user_id] = @user.id
       post :start, params: { room_uid: @user.main_room }
 
-      expect(response).to redirect_to(@user.main_room.join_path(@user.name, { user_is_moderator: true }, @user.uid))
+      expect(response).to redirect_to(join_path(@user.main_room, @user.name, { user_is_moderator: true }, @user.uid))
     end
 
     it "should bring to room if not owner" do

spec/controllers/users_controller_spec.rb

@@ -373,7 +373,7 @@ describe UsersController, type: :controller do
 
         user.reload
 
-        expect(flash[:alert]).to eq(I18n.t("administrator.roles.invalid_removal"))
+        expect(flash[:alert]).to eq(I18n.t("administrator.roles.invalid_assignment"))
         expect(response).to render_template(:edit)
       end
 
@@ -439,7 +439,6 @@ describe UsersController, type: :controller do
     it "allows admins to delete users" do
       allow(Rails.configuration).to receive(:loadbalanced_configuration).and_return(true)
       allow_any_instance_of(User).to receive(:greenlight_account?).and_return(true)
-      allow_any_instance_of(Room).to receive(:delete_all_recordings).and_return('')
       allow_any_instance_of(ApplicationController).to receive(:set_user_domain).and_return("provider1")
       controller.instance_variable_set(:@user_domain, "provider1")