Drop profile fields for static attributes (#14109)

* Drop profile fields for static attributes

* Update profile page with static field form

* Fix specs

* Update Cypress test

app/models/profile.rb

@@ -3,6 +3,7 @@ class Profile < ApplicationRecord
 
   validates :data, presence: true
   validates :user_id, uniqueness: true
+  validates :location, :website_url, length: { maximum: 100 }
   validates_with ProfileValidator
 
   has_many :custom_profile_fields, dependent: :destroy

app/validators/profile_validator.rb

@@ -9,8 +9,8 @@ class ProfileValidator < ActiveModel::Validator
 
   ERRORS = {
     color_field: "is not a valid hex color",
-    text_area: "is too long (maximum: #{MAX_TEXT_AREA_LENGTH})",
-    text_field: "is too long (maximum: #{MAX_TEXT_FIELD_LENGTH})"
+    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
 
   def validate(record)
@@ -32,19 +32,10 @@ class ProfileValidator < ActiveModel::Validator
   private
 
   def summary_too_long?(record)
-    # Calling the summary attribute method on record during onboarding
-    # throws a NoMethodError
-    return unless record.respond_to?(SUMMARY_ATTRIBUTE)
-
-    # TODO: [@jacobherrington] This will need to be addressed when the summary
-    # ProfileField is dropped from production.
-    return unless ProfileField.exists?(attribute_name: SUMMARY_ATTRIBUTE)
     return if record.summary.blank?
 
     # Grandfather in people who had a too long summary before
-    # TODO: [@jacobherrington] `record.data_was` can be removed when we delete
-    # the old data and drop the fields from `Profile.static_fields`.
-    previous_summary = record.summary_was || record.data_was[SUMMARY_ATTRIBUTE]
+    previous_summary = record.summary_was
     return if previous_summary && previous_summary.size > MAX_SUMMARY_LENGTH
 
     record.summary.size > MAX_SUMMARY_LENGTH

app/views/users/_profile.html.erb

@@ -37,7 +37,52 @@
   </div>
 
   <% profile = @user.profile %>
+
+  <div class="crayons-card crayons-card--content-rows">
+    <h2>Basic</h2>
+
+    <div class="crayons-field">
+      <label class="crayons-field__label" for="profile[website_url]">
+        Website URL
+      </label>
+      <%= f.text_field "profile[website_url]",
+                       value: profile.website_url,
+                       placeholder: "https://yoursite.com",
+                       class: "crayons-textfield js-color-field" %>
+    </div>
+
+    <div class="crayons-field crayons-field--checkbox">
+      <%= f.check_box "profile[display_email_on_profile]",
+                      checked: profile.display_email_on_profile,
+                      class: "crayons-checkbox" %>
+      <label class="crayons-field__label" for="profile[display_email_on_profile]">
+        Display email on profile
+      </label>
+    </div>
+
+    <div class="crayons-field">
+      <label class="crayons-field__label" for="profile[location]">
+        Location
+      </label>
+      <%= f.text_field "profile[location]",
+                       value: profile.location,
+                       placeholder: "Halifax, Nova Scotia",
+                       class: "crayons-textfield js-color-field" %>
+    </div>
+
+    <div class="crayons-field">
+      <label class="crayons-field__label" for="profile[summary]">
+        Bio
+      </label>
+      <%= f.text_field "profile[summary]",
+                       value: profile.summary,
+                       placeholder: "A short bio...",
+                       class: "crayons-textfield js-color-field" %>
+    </div>
+  </div>
+
   <% ProfileFieldGroup.non_empty_groups.each do |group| %>
+    <% next if group.name == "Basic" # TODO: @citizen428 Remove this after user settings work (email field) %>
     <% next if group.name == "Links" # TODO: [@jacobherrington] Remove this when we drop social links %>
     <div class="crayons-card crayons-card--content-rows">
       <h2><%= group.name %></h2>

cypress/integration/profileFlows/userUpdateSettingsProfile.spec.js

@@ -14,7 +14,7 @@ describe('User Update Settings Profile', () => {
     const location = 'New York City';
 
     cy.findByLabelText(/^Website URL$/i).type(websiteURL);
-    cy.findByLabelText(/^Summary$/i).type(summary);
+    cy.findByLabelText(/^Bio$/i).type(summary);
     cy.findByLabelText(/^Location$/i).type(location);
 
     cy.findByText(/^Save Profile Information$/i).click();
@@ -28,7 +28,7 @@ describe('User Update Settings Profile', () => {
       'have.value',
       websiteURL,
     );
-    cy.findByRole('textbox', { name: 'Summary' }).should('have.value', summary);
+    cy.findByRole('textbox', { name: 'Bio' }).should('have.value', summary);
     cy.findByRole('textbox', { name: 'Location' }).should(
       'have.value',
       location,

lib/data/dev_profile_fields.csv

@@ -1,7 +1,4 @@
 Display email on profile,check_box,,,Basic,settings_only,false
-Website URL,text_field,https://yoursite.com,,Basic,settings_only,true
-Summary,text_area,A short bio...,,Basic,settings_only,true
-Location,text_field,"Halifax, Nova Scotia",,Basic,header,true
 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

lib/data_update_scripts/20210630063635_drop_profile_fields_for_static_attributes.rb

@@ -0,0 +1,7 @@
+module DataUpdateScripts
+  class DropProfileFieldsForStaticAttributes
+    def run
+      ProfileField.destroy_by(attribute_name: Profile.static_fields)
+    end
+  end
+end

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([14, 4])
+      end.to change { profile_field_and_group_count }.from([0, 0]).to([11, 4])
     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 [14, 4]
+      expect(profile_field_and_group_count).to eq [11, 4]
     end
   end
 end

spec/lib/data_update_scripts/drop_profile_fields_for_static_attributes_spec.rb

@@ -0,0 +1,14 @@
+require "rails_helper"
+require Rails.root.join(
+  "lib/data_update_scripts/20210630063635_drop_profile_fields_for_static_attributes.rb",
+)
+
+describe DataUpdateScripts::DropProfileFieldsForStaticAttributes do
+  it "removes the 3 static profile fields" do
+    %w[location summary website_url].each do |attribute|
+      ProfileField.find_or_create_by(attribute_name: attribute, label: attribute)
+    end
+
+    expect { described_class.new.run }.to change(ProfileField, :count).by(-3)
+  end
+end

spec/models/profile_spec.rb

@@ -13,12 +13,6 @@ RSpec.describe Profile, type: :model do
     describe "conditionally validating summary" do
       let(:invalid_summary) { "x" * ProfileValidator::MAX_SUMMARY_LENGTH.next }
 
-      it "doesn't validate if the profile field doesn't exist" do
-        allow(ProfileField).to receive(:exists?).with(attribute_name: "summary").and_return(false)
-        profile.summary = invalid_summary
-        expect(profile).to be_valid
-      end
-
       it "is valid if users previously had long summaries and are grandfathered" do
         profile.summary = invalid_summary
         profile.save(validate: false)
@@ -76,7 +70,8 @@ RSpec.describe Profile, type: :model do
       it "is invalid if the text is too long" do
         profile.skills_languages = "x" * ProfileValidator::MAX_TEXT_AREA_LENGTH.next
         expect(profile).not_to be_valid
-        expect(profile.errors_as_sentence).to eq "Skills languages is too long (maximum: 200)"
+        expect(profile.errors_as_sentence)
+          .to eq "Skills languages is too long (maximum is 200 characters)"
       end
     end
 
@@ -89,7 +84,7 @@ RSpec.describe Profile, type: :model do
       it "is invalid if the text is too long" do
         profile.location = "x" * ProfileValidator::MAX_TEXT_FIELD_LENGTH.next
         expect(profile).not_to be_valid
-        expect(profile.errors_as_sentence).to eq "Location is too long (maximum: 100)"
+        expect(profile.errors_as_sentence).to eq "Location is too long (maximum is 100 characters)"
       end
     end
   end