From fc1cf71a3de95c8c21789ed2680159bf7a883777 Mon Sep 17 00:00:00 2001 From: rhymes Date: Mon, 4 May 2020 20:00:43 +0200 Subject: [PATCH] Fix showing "Remove OAuth" in settings with disabled identities (#7665) --- app/models/identity.rb | 4 +- app/views/users/_account.html.erb | 2 +- spec/requests/user/user_settings_spec.rb | 75 ++++++++++++++++++------ 3 files changed, 60 insertions(+), 21 deletions(-) diff --git a/app/models/identity.rb b/app/models/identity.rb index 792521c97..607651137 100644 --- a/app/models/identity.rb +++ b/app/models/identity.rb @@ -1,6 +1,8 @@ class Identity < ApplicationRecord belongs_to :user - has_many :backup_data, as: :instance, class_name: "BackupData", dependent: :destroy + has_many :backup_data, as: :instance, class_name: "BackupData", dependent: :destroy + + scope :enabled, -> { where(provider: Authentication::Providers.enabled) } validates :uid, :provider, presence: true validates :uid, uniqueness: { scope: :provider }, if: proc { |identity| identity.uid_changed? || identity.provider_changed? } diff --git a/app/views/users/_account.html.erb b/app/views/users/_account.html.erb index f4fd71d30..3a60d1434 100644 --- a/app/views/users/_account.html.erb +++ b/app/views/users/_account.html.erb @@ -45,7 +45,7 @@

Danger Zone

- <% if @user.identities.size > 1 %> + <% if @user.identities.enabled.size > 1 %>

Remove OAuth Associations

You can remove one of your authentication methods. We'll still need one to authenticate you.

diff --git a/spec/requests/user/user_settings_spec.rb b/spec/requests/user/user_settings_spec.rb index c7a46a906..7b2771c76 100644 --- a/spec/requests/user/user_settings_spec.rb +++ b/spec/requests/user/user_settings_spec.rb @@ -26,11 +26,6 @@ RSpec.describe "UserSettings", type: :request do to raise_error(ActiveRecord::RecordNotFound) end - it "allows users to visit the account page" do - get "/settings/account" - expect(response.body).to include("Danger Zone") - end - it "displays content on ux tab properly" do get "/settings/ux" expect(response.body).to include("Style Customization") @@ -48,20 +43,6 @@ RSpec.describe "UserSettings", type: :request do expect(response.body).to include error_message end - it "does not render the ghost account email option if the user has no content" do - ghost_account_message = "If you would like to keep your content under the" - get "/settings/account" - expect(response.body).not_to include ghost_account_message - end - - it "does render the ghost account email option if the user has content" do - ghost_account_message = "If you would like to keep your content under the" - create(:article, user: user) - user.update(articles_count: 1) - get "/settings/account" - expect(response.body).to include ghost_account_message - end - it "renders the proper organization page" do first_org, second_org = create_list(:organization, 2) create(:organization_membership, user: user, organization: first_org) @@ -77,6 +58,62 @@ RSpec.describe "UserSettings", type: :request do end end + describe ":account" do + let(:ghost_account_message) { "If you would like to keep your content under the" } + let(:remove_oauth_section) { "Remove OAuth Associations" } + let(:user) { create(:user, :with_identity) } + + before do + omniauth_mock_providers_payload + sign_in user + end + + it "allows users to visit the account page" do + get user_settings_path(tab: "account") + expect(response).to have_http_status(:ok) + end + + it "does not render the ghost account email option if the user has no content" do + get user_settings_path(tab: "account") + expect(response.body).not_to include(ghost_account_message) + end + + it "does render the ghost account email option if the user has content" do + create(:article, user: user) + user.update(articles_count: 1) + + get user_settings_path(tab: "account") + + expect(response.body).to include(ghost_account_message) + end + + it "shows the 'Remove OAuth' section if a user has multiple enabled identities" do + allow(Authentication::Providers).to receive(:enabled).and_return(Authentication::Providers.available) + providers = Authentication::Providers.available.first(2) + allow(user).to receive(:identities).and_return(user.identities.where(provider: providers)) + + get user_settings_path(tab: "account") + expect(response.body).to include(remove_oauth_section) + end + + it "hides the 'Remove OAuth' section if a user has one enabled identity" do + provider = Authentication::Providers.available.first + allow(Authentication::Providers).to receive(:enabled).and_return([provider]) + allow(user).to receive(:identities).and_return(user.identities.where(provider: provider)) + + get user_settings_path(tab: "account") + expect(response.body).not_to include(remove_oauth_section) + end + + it "hides the 'Remove OAuth' section if a user has one enabled identity and one disabled" do + provider = Authentication::Providers.available.first + allow(Authentication::Providers).to receive(:enabled).and_return([provider]) + + get user_settings_path(tab: "account") + expect(response.body).not_to include(remove_oauth_section) + end + end + describe "connect providers accounts" do before do omniauth_mock_providers_payload