From 9794305412241f59c266c2331e90c49135f1548c Mon Sep 17 00:00:00 2001
From: Michael Kohl <citizen428@dev.to>
Date: Fri, 16 Jul 2021 20:45:12 +0700
Subject: [PATCH] Remove unused profile fields (#14206)

* Remove unused profile fields

* Clean up CSV

* Account for user settings in Profiles::Update

* Move brand color validation to Users::Setting

* Fix specs
---
 app/models/users/setting.rb                   |  5 ++++
 app/services/profiles/update.rb               | 10 ++++---
 app/validators/profile_validator.rb           |  8 ++---
 app/views/users/_profile.html.erb             |  2 +-
 lib/data/dev_profile_fields.csv               |  3 --
 ...0712054300_remove_unused_profile_fields.rb | 25 ++++++++++++++++
 .../create_profile_fields_spec.rb             |  4 +--
 spec/models/profile_spec.rb                   | 29 -------------------
 spec/models/users/setting_spec.rb             | 29 +++++++++++++++++++
 spec/services/profiles/update_spec.rb         |  4 +--
 10 files changed, 72 insertions(+), 47 deletions(-)
 create mode 100644 lib/data_update_scripts/20210712054300_remove_unused_profile_fields.rb

diff --git a/app/models/users/setting.rb b/app/models/users/setting.rb
index 893c2717a..a998f66d3 100644
--- a/app/models/users/setting.rb
+++ b/app/models/users/setting.rb
@@ -2,6 +2,8 @@ module Users
   class Setting < ApplicationRecord
     self.table_name_prefix = "users_"
 
+    HEX_COLOR_REGEXP = /\A#?(?:\h{6}|\h{3})\z/.freeze
+
     belongs_to :user, touch: true
     scope :with_feed, -> { where.not(feed_url: [nil, ""]) }
 
@@ -13,6 +15,9 @@ module Users
     enum config_theme: { default: 0, minimal_light_theme: 1, night_theme: 2, pink_theme: 3,
                          ten_x_hacker_theme: 4 }
 
+    validates :brand_color1,
+              :brand_color2,
+              format: { with: HEX_COLOR_REGEXP, message: "is not a valid hex color" }
     validates :user_id, presence: true
     validates :experience_level, numericality: { less_than_or_equal_to: 10 }, allow_blank: true
     validates :feed_referential_link, inclusion: { in: [true, false] }
diff --git a/app/services/profiles/update.rb b/app/services/profiles/update.rb
index 4c418d49f..77b2f5b74 100644
--- a/app/services/profiles/update.rb
+++ b/app/services/profiles/update.rb
@@ -4,8 +4,9 @@ module Profiles
     using HashAnyKey
     include ImageUploads
 
-    CORE_PROFILE_FIELDS = %i[summary brand_color1 brand_color2].freeze
+    CORE_PROFILE_FIELDS = %i[summary].freeze
     CORE_USER_FIELDS = %i[name username profile_image].freeze
+    CORE_SETTINGS_FIELDS = %i[brand_color1 brand_color2].freeze
 
     # @param user [User] the user whose profile we are updating
     # @param updated_attributes [Hash<Symbol, Hash<Symbol, Object>>] the profile
@@ -112,15 +113,16 @@ module Profiles
     end
 
     def conditionally_resave_articles
-      return unless core_profile_details_changed? && !@user.suspended?
+      return unless resave_articles? && !@user.suspended?
 
       Users::ResaveArticlesWorker.perform_async(@user.id)
     end
 
-    def core_profile_details_changed?
+    def resave_articles?
       user_fields = CORE_USER_FIELDS + Authentication::Providers.username_fields
       @updated_user_attributes.any_key?(user_fields) ||
-        @updated_profile_attributes.any_key?(CORE_PROFILE_FIELDS)
+        @updated_profile_attributes.any_key?(CORE_PROFILE_FIELDS) ||
+        @updated_users_setting_attributes.any_key?(CORE_SETTINGS_FIELDS)
     end
   end
 end
diff --git a/app/validators/profile_validator.rb b/app/validators/profile_validator.rb
index 23b19bf93..678b8146d 100644
--- a/app/validators/profile_validator.rb
+++ b/app/validators/profile_validator.rb
@@ -5,10 +5,7 @@ class ProfileValidator < ActiveModel::Validator
   MAX_TEXT_AREA_LENGTH = 200
   MAX_TEXT_FIELD_LENGTH = 100
 
-  HEX_COLOR_REGEXP = /^#?(?:\h{6}|\h{3})$/.freeze
-
   ERRORS = {
-    color_field: "is not a valid hex color",
     text_area: "is too long (maximum is #{MAX_TEXT_AREA_LENGTH} characters)",
     text_field: "is too long (maximum is #{MAX_TEXT_FIELD_LENGTH} characters)"
   }.with_indifferent_access.freeze
@@ -45,9 +42,8 @@ class ProfileValidator < ActiveModel::Validator
     true # checkboxes are always valid
   end
 
-  def color_field_valid?(record, attribute)
-    hex_value = record.public_send(attribute)
-    hex_value.nil? || hex_value.match?(HEX_COLOR_REGEXP)
+  def color_field_valid?(_record, _attribute)
+    true # we do not currently validate color fields here
   end
 
   def text_area_valid?(record, attribute)
diff --git a/app/views/users/_profile.html.erb b/app/views/users/_profile.html.erb
index e5a7e6321..fee68612b 100644
--- a/app/views/users/_profile.html.erb
+++ b/app/views/users/_profile.html.erb
@@ -58,7 +58,7 @@
 
     <div class="crayons-field crayons-field--checkbox">
       <%= f.check_box "profile[display_email_on_profile]",
-                      checked: profile.display_email_on_profile,
+                      checked: @user.setting.display_email_on_profile,
                       class: "crayons-checkbox" %>
       <label class="crayons-field__label" for="profile[display_email_on_profile]">
         Display email on profile
diff --git a/lib/data/dev_profile_fields.csv b/lib/data/dev_profile_fields.csv
index af6e88200..5bad9ec51 100644
--- a/lib/data/dev_profile_fields.csv
+++ b/lib/data/dev_profile_fields.csv
@@ -1,4 +1,3 @@
-Display email on profile,check_box,,,Basic,settings_only,false
 Education,text_field,,,Work,header,false
 Employer name,text_field,Acme Inc.,,Work,header,false
 Employer URL,text_field,https://dev.com,,Work,header,false
@@ -7,5 +6,3 @@ Skills/Languages,text_area,,What tools and languages are you most experienced wi
 Currently learning,text_area,,"What are you learning right now? What are the new tools and languages you're picking up right now?",Coding,left_sidebar,false
 Currently hacking on,text_area,,What projects are currently occupying most of your time?,Coding,left_sidebar,false
 Available for,text_area,,"What kinds of collaborations or discussions are you available for? What's a good reason to say Hey! to you these days?",Coding,left_sidebar,false
-Brand color 1,color_field,#000000,"Used for backgrounds, borders etc.",Branding,settings_only,false
-Brand color 2,color_field,#000000,Used for texts (usually put on Brand color 1).,Branding,settings_only,false
diff --git a/lib/data_update_scripts/20210712054300_remove_unused_profile_fields.rb b/lib/data_update_scripts/20210712054300_remove_unused_profile_fields.rb
new file mode 100644
index 000000000..664782ebb
--- /dev/null
+++ b/lib/data_update_scripts/20210712054300_remove_unused_profile_fields.rb
@@ -0,0 +1,25 @@
+module DataUpdateScripts
+  class RemoveUnusedProfileFields
+    OBSOLETE_FIELDS = %w[
+      behance_url
+      brand_color1
+      brand_color2
+      display_email_on_profile
+      dribbble_url
+      facebook_url
+      git_lab_url
+      instagram_url
+      linked_in_url
+      mastodon_url
+      medium_url
+      recruiters_can_contact_me_about_job_opportunities
+      stack_overflow_url
+      twitch_url
+      youtube_url
+    ].freeze
+
+    def run
+      ProfileField.destroy_by(attribute_name: OBSOLETE_FIELDS)
+    end
+  end
+end
diff --git a/spec/lib/data_update_scripts/create_profile_fields_spec.rb b/spec/lib/data_update_scripts/create_profile_fields_spec.rb
index b1a13c5b8..8eeacd5f4 100644
--- a/spec/lib/data_update_scripts/create_profile_fields_spec.rb
+++ b/spec/lib/data_update_scripts/create_profile_fields_spec.rb
@@ -15,7 +15,7 @@ describe DataUpdateScripts::CreateProfileFields do
     it "creates all profile fields and groups" do
       expect do
         described_class.new.run
-      end.to change { profile_field_and_group_count }.from([0, 0]).to([11, 4])
+      end.to change { profile_field_and_group_count }.from([0, 0]).to([8, 2])
     end
   end
 
@@ -29,7 +29,7 @@ describe DataUpdateScripts::CreateProfileFields do
       expect do
         described_class.new.run
       end.not_to change { profile_field_and_group_count }
-      expect(profile_field_and_group_count).to eq [11, 4]
+      expect(profile_field_and_group_count).to eq [8, 2]
     end
   end
 end
diff --git a/spec/models/profile_spec.rb b/spec/models/profile_spec.rb
index ca0c7857b..de7a3fe6f 100644
--- a/spec/models/profile_spec.rb
+++ b/spec/models/profile_spec.rb
@@ -32,35 +32,6 @@ RSpec.describe Profile, type: :model do
       end
     end
 
-    describe "validating color fields" do
-      it "is valid if the field is a correct hex color with leading #" do
-        profile.brand_color1 = "#abcdef"
-        expect(profile).to be_valid
-      end
-
-      it "is valid if the field is a correct hex color without leading #" do
-        profile.brand_color1 = "abcdef"
-        expect(profile).to be_valid
-      end
-
-      it "is valid if the field is a 3-digit hex color" do
-        profile.brand_color1 = "#ccc"
-        expect(profile).to be_valid
-      end
-
-      it "is invalid if the field is too long" do
-        profile.brand_color1 = "#deadbeef"
-        expect(profile).not_to be_valid
-        expect(profile.errors_as_sentence).to eq "Brand color1 is not a valid hex color"
-      end
-
-      it "is invalid if the field contains non hex characters" do
-        profile.brand_color1 = "#abcdeg"
-        expect(profile).not_to be_valid
-        expect(profile.errors_as_sentence).to eq "Brand color1 is not a valid hex color"
-      end
-    end
-
     describe "validating text areas" do
       it "is valid if the text is short enough" do
         profile.skills_languages = "Ruby"
diff --git a/spec/models/users/setting_spec.rb b/spec/models/users/setting_spec.rb
index dde7e6103..d57caf47e 100644
--- a/spec/models/users/setting_spec.rb
+++ b/spec/models/users/setting_spec.rb
@@ -13,6 +13,35 @@ RSpec.describe Users::Setting, type: :model do
     it { is_expected.to define_enum_for(:config_navbar).with_values(default: 0, static: 1).with_suffix(:navbar) }
     it { is_expected.to define_enum_for(:config_theme).with_values(default: 0, minimal_light_theme: 1, night_theme: 2, pink_theme: 3, ten_x_hacker_theme: 4) }
 
+    describe "validating color fields" do
+      it "is valid if the field is a correct hex color with leading #" do
+        setting.brand_color1 = "#abcdef"
+        expect(setting).to be_valid
+      end
+
+      it "is valid if the field is a correct hex color without leading #" do
+        setting.brand_color1 = "abcdef"
+        expect(setting).to be_valid
+      end
+
+      it "is valid if the field is a 3-digit hex color" do
+        setting.brand_color1 = "#ccc"
+        expect(setting).to be_valid
+      end
+
+      it "is invalid if the field is too long" do
+        setting.brand_color1 = "#deadbeef"
+        expect(setting).not_to be_valid
+        expect(setting.errors_as_sentence).to eq "Brand color1 is not a valid hex color"
+      end
+
+      it "is invalid if the field contains non hex characters" do
+        setting.brand_color1 = "#abcdeg"
+        expect(setting).not_to be_valid
+        expect(setting.errors_as_sentence).to eq "Brand color1 is not a valid hex color"
+      end
+    end
+
     describe "#config_theme" do
       it "accepts valid theme" do
         setting.config_theme = 2
diff --git a/spec/services/profiles/update_spec.rb b/spec/services/profiles/update_spec.rb
index 285e31dbd..6d3798a12 100644
--- a/spec/services/profiles/update_spec.rb
+++ b/spec/services/profiles/update_spec.rb
@@ -101,13 +101,13 @@ RSpec.describe Profiles::Update, type: :service do
 
     it "enqueues resave articles job when changing bg_color_hex" do
       sidekiq_assert_resave_article_worker(user) do
-        described_class.call(user, profile: { brand_color1: "#12345F" })
+        described_class.call(user, user_settings: { brand_color1: "#12345F" })
       end
     end
 
     it "enqueues resave articles job when changing text_color_hex" do
       sidekiq_assert_resave_article_worker(user) do
-        described_class.call(user, profile: { brand_color2: "#12345F" })
+        described_class.call(user, user_settings: { brand_color2: "#12345F" })
       end
     end