diff --git a/app/models/profile.rb b/app/models/profile.rb index 81eee25a9..5e2b0bc41 100644 --- a/app/models/profile.rb +++ b/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 diff --git a/app/validators/profile_validator.rb b/app/validators/profile_validator.rb index e4b9d362a..23b19bf93 100644 --- a/app/validators/profile_validator.rb +++ b/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 diff --git a/app/views/users/_profile.html.erb b/app/views/users/_profile.html.erb index a12753367..5c8ba976c 100644 --- a/app/views/users/_profile.html.erb +++ b/app/views/users/_profile.html.erb @@ -37,7 +37,52 @@ <% profile = @user.profile %> + +
+

Basic

+ +
+ + <%= f.text_field "profile[website_url]", + value: profile.website_url, + placeholder: "https://yoursite.com", + class: "crayons-textfield js-color-field" %> +
+ +
+ <%= f.check_box "profile[display_email_on_profile]", + checked: profile.display_email_on_profile, + class: "crayons-checkbox" %> + +
+ +
+ + <%= f.text_field "profile[location]", + value: profile.location, + placeholder: "Halifax, Nova Scotia", + class: "crayons-textfield js-color-field" %> +
+ +
+ + <%= f.text_field "profile[summary]", + value: profile.summary, + placeholder: "A short bio...", + class: "crayons-textfield js-color-field" %> +
+
+ <% 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 %>

<%= group.name %>

diff --git a/cypress/integration/profileFlows/userUpdateSettingsProfile.spec.js b/cypress/integration/profileFlows/userUpdateSettingsProfile.spec.js index 82d928603..81a2d54cb 100644 --- a/cypress/integration/profileFlows/userUpdateSettingsProfile.spec.js +++ b/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, diff --git a/lib/data/dev_profile_fields.csv b/lib/data/dev_profile_fields.csv index 4f96e1c74..af6e88200 100644 --- a/lib/data/dev_profile_fields.csv +++ b/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 diff --git a/lib/data_update_scripts/20210630063635_drop_profile_fields_for_static_attributes.rb b/lib/data_update_scripts/20210630063635_drop_profile_fields_for_static_attributes.rb new file mode 100644 index 000000000..00fa941ad --- /dev/null +++ b/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 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 dd68be773..b1a13c5b8 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([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 diff --git a/spec/lib/data_update_scripts/drop_profile_fields_for_static_attributes_spec.rb b/spec/lib/data_update_scripts/drop_profile_fields_for_static_attributes_spec.rb new file mode 100644 index 000000000..0b4caa691 --- /dev/null +++ b/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 diff --git a/spec/models/profile_spec.rb b/spec/models/profile_spec.rb index a75fc85c8..ca0c7857b 100644 --- a/spec/models/profile_spec.rb +++ b/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