From 1ebec4b67f2516e622f541a1e15bbe6ef0f6f3fd Mon Sep 17 00:00:00 2001 From: Michael Kohl Date: Wed, 18 Aug 2021 11:44:53 +0700 Subject: [PATCH] Remove hardcoded user profile fields (#14079) * Remove duplicated work display from header * Update work profile field handling * Update DUS + spec * Delegate more carefully * Update delegation guard * Adapt for removed delegation * Undo accidental schema changes * Fix seeds * Remove accidentaly change * Fix User#processed_website_url * Update guard clause * Update profile card content * Add Organization#profile * Be more conservative with profile fields * Spec fixes round 1 * Fix typo * Update spec * Limit number of header fields and update card content * Decorate correct model * Update factory * Update schema.rb * Fix validation * How bad could this possibly be? * Pretty bad, nevermind * Remove obsolete code * Reset profile fields during test runs * Move profile fields back to before(:suite) * Spec fixes * Remove accidentally re-added files * More spec fixes * Specs * Change User#tag_keywords_for_search * More spec fixes * Add comment * Undo accidental schema changes * Attempt spec fix * Remove fix attempt * Fix e2e test * Update spec * Remove guard clause * Remove hardcoded instances of education field * WIP Access display_email_on_profile via User Settings * Remove unused profile column * WIP fix issues revealed by systems specs * WIP fix issues revealed by services specs * WIP Fix failing tests * WIP Fix spec failures * wip * Move two attributes from controller to decorator * Update comment * More consistently use user.tag_line Even before the profile changes we sometimes used the user.summary attribute directly but used the user.tag_line method in other places. * Remove education * Add comment * WIP * Clean up mostly_work_with * WIP * Update work profile field handling * More work-related changes * Remove settings_only from display_area enum * Remove quickfix from _metadata partial * Remove special attributes * Remove leftover spec * Retrieve location from profile, not user * Profile.special_attributes no longer exists * Update specs * More spec fixes * Update UsersController * Update UsersController and spec * Fix e2e seeds * Minor cleanup * More e2e seed fixes * Fix profile field CSV * Fix e2e seeds * Move one more attribute in e2e seeds * Remove duplicate line * Clear inputs before typing in them Co-authored-by: Jacob Herrington --- app/controllers/stories_controller.rb | 6 +-- app/controllers/users_controller.rb | 9 ++-- app/decorators/user_decorator.rb | 15 ++++++ app/models/profile.rb | 15 +----- app/models/profile_field.rb | 3 +- app/models/user.rb | 14 ++---- app/policies/user_policy.rb | 7 +-- app/services/profiles/update.rb | 2 + app/services/users/suggest_for_sidebar.rb | 2 +- .../api/v0/shared/_user_show.json.jbuilder | 7 +-- app/views/articles/feed.rss.builder | 2 +- app/views/organizations/_header.html.erb | 4 +- .../shared/_profile_card_content.html.erb | 5 +- app/views/users/_liquid.html.erb | 2 +- app/views/users/_meta.html.erb | 7 ++- app/views/users/_metadata.html.erb | 1 - app/views/users/index.json.jbuilder | 2 +- app/views/users/show.html.erb | 10 ++-- config/environments/test.rb | 8 +-- .../userUpdateSettingsProfile.spec.js | 10 ++-- ...72907_backfill_profile_skills_languages.rb | 9 ---- ..._proper_display_area_for_profile_fields.rb | 28 ----------- spec/helpers/application_helper_spec.rb | 2 +- ...er_display_area_for_profile_fields_spec.rb | 13 ----- spec/models/profile_field_spec.rb | 12 +++++ spec/models/user_spec.rb | 11 +--- spec/requests/api/v0/organizations_spec.rb | 17 +++++-- spec/requests/api/v0/users_spec.rb | 31 ++++++++---- spec/requests/user/user_profile_spec.rb | 10 +--- spec/requests/user/user_settings_spec.rb | 7 +-- spec/requests/user/user_show_spec.rb | 3 +- spec/requests/user/user_suggestions_spec.rb | 4 +- spec/requests/users_onboarding_spec.rb | 7 ++- spec/services/profiles/update_spec.rb | 7 ++- spec/support/seeds/seeds_e2e.rb | 50 ++++++++++++------- 35 files changed, 157 insertions(+), 185 deletions(-) delete mode 100644 lib/data_update_scripts/20200922072907_backfill_profile_skills_languages.rb delete mode 100644 lib/data_update_scripts/20201223013903_set_proper_display_area_for_profile_fields.rb delete mode 100644 spec/lib/data_update_scripts/set_proper_display_area_for_profile_fields_spec.rb diff --git a/app/controllers/stories_controller.rb b/app/controllers/stories_controller.rb index b6fce9cbe..decfc44eb 100644 --- a/app/controllers/stories_controller.rb +++ b/app/controllers/stories_controller.rb @@ -333,6 +333,7 @@ class StoriesController < ApplicationController def set_user_json_ld # For more info on structuring data with JSON-LD, # please refer to this link: https://moz.com/blog/json-ld-for-beginners + decorated_user = @user.decorate @user_json_ld = { "@context": "http://schema.org", "@type": "Person", @@ -344,9 +345,8 @@ class StoriesController < ApplicationController sameAs: user_same_as, image: Images::Profile.call(@user.profile_image_url, length: 320), name: @user.name, - email: @user.setting.display_email_on_profile ? @user.email : nil, - description: @user.profile.summary.presence || "404 bio not found", - alumniOf: @user.education.presence + email: decorated_user.profile_email, + description: decorated_user.profile_summary }.reject { |_, v| v.blank? } end diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 01acd4297..fcb5d4923 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -46,9 +46,7 @@ class UsersController < ApplicationController set_current_tab(params["user"]["tab"]) set_users_setting_and_notification_setting - @user.assign_attributes(permitted_attributes(@user)) - - if @user.save + if @user.update(permitted_attributes(@user)) # NOTE: [@rhymes] this queues a job to fetch the feed each time the profile is updated, regardless if the user # explicitly requested "Feed fetch now" or simply updated any other field import_articles_from_feed(@user) @@ -170,6 +168,7 @@ class UsersController < ApplicationController def onboarding_update authorize User + user_params = { saw_onboarding: true } if params[:user] @@ -181,7 +180,7 @@ class UsersController < ApplicationController user_params.merge!(params[:user].permit(ALLOWED_USER_PARAMS)) end - update_result = Profiles::Update.call(current_user, { user: user_params, profile: profile_params }) + update_result = Profiles::Update.call(current_user, user: user_params, profile: profile_params) render_update_response(update_result.success?, update_result.errors_as_sentence) end @@ -381,7 +380,7 @@ class UsersController < ApplicationController end def profile_params - params[:profile] ? params[:profile].permit(Profile.attributes) : nil + params[:profile] ? params[:profile].permit(Profile.static_fields + Profile.attributes) : nil end def password_params diff --git a/app/decorators/user_decorator.rb b/app/decorators/user_decorator.rb index b68bd35c8..b364c1759 100644 --- a/app/decorators/user_decorator.rb +++ b/app/decorators/user_decorator.rb @@ -18,6 +18,8 @@ class UserDecorator < ApplicationDecorator }, ].freeze + DEFAULT_PROFILE_SUMMARY = "404 bio not found".freeze + def cached_followed_tags follows_map = Rails.cache.fetch("user-#{id}-#{following_tags_count}-#{last_followed_at&.rfc3339}/followed_tags", expires_in: 20.hours) do @@ -110,6 +112,19 @@ class UserDecorator < ApplicationDecorator created_at.after?(min_days.days.ago) end + # Returns the user's public email if it is set and the display_email_on_profile + # settings is set to true. + def profile_email + return unless setting.display_email_on_profile? + + email + end + + # Returns the users profile summary or a placeholder text + def profile_summary + profile.summary.presence || DEFAULT_PROFILE_SUMMARY + end + delegate :display_sponsors, to: :setting delegate :display_announcements, to: :setting diff --git a/app/models/profile.rb b/app/models/profile.rb index 6eede755b..cc55cec91 100644 --- a/app/models/profile.rb +++ b/app/models/profile.rb @@ -11,15 +11,6 @@ class Profile < ApplicationRecord # any profile on a given Forem. STATIC_FIELDS = %w[summary location website_url].freeze - SPECIAL_DISPLAY_ATTRIBUTES = %w[summary location].freeze - - # NOTE: @citizen428 This is a temporary mapping so we don't break DEV during - # profile migration/generalization work. - MAPPED_ATTRIBUTES = { - education: :education, - skills_languages: :mostly_work_with - }.with_indifferent_access.freeze - # Generates typed accessors for all currently defined profile fields. def self.refresh_attributes! return if ENV["ENV_AVAILABLE"] == "false" @@ -30,7 +21,7 @@ class Profile < ApplicationRecord # TODO: [@jacobherrington] Remove this when ProfileFields for the static # fields are dropped from production and the associated data is removed. # https://github.com/forem/forem/pull/13641#discussion_r637641185 - next if STATIC_FIELDS.any?(field.attribute_name) + next if field.attribute_name.in?(STATIC_FIELDS) store_attribute :data, field.attribute_name.to_sym, field.type end @@ -45,10 +36,6 @@ class Profile < ApplicationRecord (stored_attributes[:data] || []).map(&:to_s) end - def self.special_attributes - SPECIAL_DISPLAY_ATTRIBUTES - end - def self.static_fields STATIC_FIELDS end diff --git a/app/models/profile_field.rb b/app/models/profile_field.rb index 26390f289..33f1629cd 100644 --- a/app/models/profile_field.rb +++ b/app/models/profile_field.rb @@ -13,8 +13,7 @@ class ProfileField < ApplicationRecord enum display_area: { header: 0, - left_sidebar: 1, - settings_only: 2 + left_sidebar: 1 } belongs_to :profile_field_group, optional: true diff --git a/app/models/user.rb b/app/models/user.rb index 238c935e5..4c2fb821c 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -12,7 +12,6 @@ class User < ApplicationRecord bg_color_hex currently_hacking_on currently_learning - currently_streaming_on dribbble_url education email_public @@ -58,19 +57,12 @@ class User < ApplicationRecord after_create_commit -> { Profile.create(user: self) }, unless: :_skip_creating_profile # Getters and setters for unmapped profile attributes - (PROFILE_COLUMNS - Profile::MAPPED_ATTRIBUTES.values).each do |column| + PROFILE_COLUMNS.each do |column| next if INACTIVE_PROFILE_COLUMNS.include?(column) + next unless column.in?(Profile.attributes) delegate column, "#{column}=", to: :profile, allow_nil: true end - - # Getters and setters for mapped profile attributes - Profile::MAPPED_ATTRIBUTES.each do |profile_attribute, user_attribute| - define_method(user_attribute) { profile&.public_send(profile_attribute) } - define_method("#{user_attribute}=") do |value| - profile&.public_send("#{profile_attribute}=", value) - end - end end end @@ -289,7 +281,7 @@ class User < ApplicationRecord end def tag_line - summary + profile.summary end def twitter_url diff --git a/app/policies/user_policy.rb b/app/policies/user_policy.rb index 280771ede..5758f6afe 100644 --- a/app/policies/user_policy.rb +++ b/app/policies/user_policy.rb @@ -20,7 +20,8 @@ class UserPolicy < ApplicationPolicy email_follower_notifications email_membership_newsletter email_mention_notifications - email_newsletter email_public + email_newsletter + email_public email_tag_mod_newsletter email_unread_notifications employer_name @@ -33,21 +34,17 @@ class UserPolicy < ApplicationPolicy feed_url inbox_guidelines inbox_type - location mobile_comment_notifications mod_roundrobin_notifications welcome_notifications - mostly_work_with name password password_confirmation payment_pointer permit_adjacent_sponsors profile_image - summary text_color_hex username - website_url ].freeze def edit? diff --git a/app/services/profiles/update.rb b/app/services/profiles/update.rb index 0dd66e899..f511b8e89 100644 --- a/app/services/profiles/update.rb +++ b/app/services/profiles/update.rb @@ -1,3 +1,5 @@ +# TODO: This should probably become `Users::Update` as we're dealing with core +# user attributes, profile attributes and user settings. module Profiles # Deals with updates that affect fields on +Profile+ and/or +User+ in a transparent way. class Update diff --git a/app/services/users/suggest_for_sidebar.rb b/app/services/users/suggest_for_sidebar.rb index f5390230b..1803a524e 100644 --- a/app/services/users/suggest_for_sidebar.rb +++ b/app/services/users/suggest_for_sidebar.rb @@ -16,7 +16,7 @@ module Users suggested_user_ids = Rails.cache.fetch(generate_cache_name, expires_in: 120.hours) do (reputable_user_ids + random_user_ids).uniq end - User.select(:id, :name, :username, :profile_image, :summary).where(id: suggested_user_ids) + User.select(:id, :name, :username, :profile_image).where(id: suggested_user_ids) end private diff --git a/app/views/api/v0/shared/_user_show.json.jbuilder b/app/views/api/v0/shared/_user_show.json.jbuilder index 7c01a27bd..7f2a57d5c 100644 --- a/app/views/api/v0/shared/_user_show.json.jbuilder +++ b/app/views/api/v0/shared/_user_show.json.jbuilder @@ -5,12 +5,13 @@ json.extract!( :id, :username, :name, - :summary, :twitter_username, :github_username, - :website_url, - :location, ) +Profile.static_fields.each do |attr| + json.set! attr, user.profile.public_send(attr) +end + json.joined_at user.created_at.strftime("%b %e, %Y") json.profile_image Images::Profile.call(user.profile_image_url, length: 320) diff --git a/app/views/articles/feed.rss.builder b/app/views/articles/feed.rss.builder index c3bd52e11..270dc1cc1 100644 --- a/app/views/articles/feed.rss.builder +++ b/app/views/articles/feed.rss.builder @@ -7,7 +7,7 @@ xml.rss version: "2.0" do xml.channel do xml.title user ? user.name : community_name xml.author user ? user.name : community_name - xml.description user ? user.summary : Settings::Community.community_description + xml.description user ? user.tag_line : Settings::Community.community_description xml.link user ? app_url(user.path) : app_url xml.language "en" if user diff --git a/app/views/organizations/_header.html.erb b/app/views/organizations/_header.html.erb index 20b8005d8..a7629b58f 100644 --- a/app/views/organizations/_header.html.erb +++ b/app/views/organizations/_header.html.erb @@ -53,8 +53,8 @@ <%= inline_svg_tag("github.svg", class: "crayons-icon", aria: true, title: "GitHub logo") %> <% end %> - <% if @user.website_url.present? %> - + <% if @user.profile.website_url.present? %> + <%= inline_svg_tag("external.svg", class: "crayons-icon", aria: true, title: "External link icon") %> <% end %> diff --git a/app/views/shared/_profile_card_content.html.erb b/app/views/shared/_profile_card_content.html.erb index 90cb15529..6a0b3d1cd 100644 --- a/app/views/shared/_profile_card_content.html.erb +++ b/app/views/shared/_profile_card_content.html.erb @@ -29,19 +29,18 @@ <% end %> - <% if actor.location.present? %> + <% if actor.profile.location.present? %>
  • Location
    - <%= actor.location %> + <%= actor.profile.location %>
    • <% end %> <% if (header_fields = actor.profile.decorate.ui_attributes_for(area: :header)).present? %> <% header_fields.sort.each do |title, value| %> - <% next if Profile.special_attributes.include?(title) %>
  • <%= title %> diff --git a/app/views/users/_liquid.html.erb b/app/views/users/_liquid.html.erb index b61b48fef..e1ce311c6 100644 --- a/app/views/users/_liquid.html.erb +++ b/app/views/users/_liquid.html.erb @@ -20,7 +20,7 @@

    <%= link_to_if user_path.present?, user.name, user_path, class: "ltag__user__link" %><%= follow_button %>

    - <%= link_to_if user_path.present?, user.summary, user_path, class: "ltag__user__link" %> + <%= link_to_if user_path.present?, user.tag_line, user_path, class: "ltag__user__link" %>
    diff --git a/app/views/users/_meta.html.erb b/app/views/users/_meta.html.erb index d99529d31..ce4b698fc 100644 --- a/app/views/users/_meta.html.erb +++ b/app/views/users/_meta.html.erb @@ -1,19 +1,18 @@ - + <%= meta_keywords_default %> - + - "> - + <% if @stories.any? %> <%= auto_discovery_link_tag(:rss, app_url("/feed/#{@user.username}"), title: "#{community_name} RSS Feed") %> diff --git a/app/views/users/_metadata.html.erb b/app/views/users/_metadata.html.erb index fd615a18d..5083fd7d2 100644 --- a/app/views/users/_metadata.html.erb +++ b/app/views/users/_metadata.html.erb @@ -1,7 +1,6 @@ <% if (header_fields = @profile.ui_attributes_for(area: :header)).present? %>
    <% header_fields.sort.each do |title, value| %> - <% next if Profile.special_attributes.include?(title) %>
    <%= sanitized_sidebar title.titleize %> diff --git a/app/views/users/index.json.jbuilder b/app/views/users/index.json.jbuilder index 67db4f3c0..25a8cf3aa 100644 --- a/app/views/users/index.json.jbuilder +++ b/app/views/users/index.json.jbuilder @@ -1,7 +1,7 @@ json.array! @users.each do |user| json.extract!(user, :id, :name, :username) - json.summary truncate(user.summary.presence || "Active #{community_name} author", length: 100) + json.summary truncate(user.tag_line || "Active #{community_name} author", length: 100) json.profile_image_url Images::Profile.call(user.profile_image_url, length: 90) json.following false end diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb index e310982d1..d6dcf449f 100644 --- a/app/views/users/show.html.erb +++ b/app/views/users/show.html.erb @@ -61,13 +61,13 @@

    <%= @user.name %>

    -

    <%= @user.profile.summary.presence || ["404 bio not found"].sample %>

    +

    <%= @user.tag_line.presence || ["404 bio not found"].sample %>

    <% if @user.profile.location.present? %> <%= inline_svg_tag("location.svg", aria: true, class: "crayons-icon mr-2 shrink-0", title: "Location") %> - <%= @user.location %> + <%= @user.profile.location %> <% end %> @@ -80,14 +80,14 @@ <% if @user.setting.display_email_on_profile %> <%= inline_svg_tag("email.svg", class: "crayons-icon mr-2 shrink-0", aria: true, title: "Email address") %> - <%= @user.email %> + <%= @user.setting.display_email_on_profile %> <% end %> <% if @user.profile.website_url.present? %> - + <%= inline_svg_tag("link-external.svg", class: "crayons-icon mr-2 shrink-0", aria: true, title: "Personal website") %> - <%= @user.website_url %> + <%= @user.profile.website_url %> <% end %> diff --git a/config/environments/test.rb b/config/environments/test.rb index a69598f96..cba190902 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -81,9 +81,11 @@ Rails.application.configure do Bullet.add_safelist(type: :unused_eager_loading, class_name: "Article", association: :top_comments) Bullet.add_safelist(type: :unused_eager_loading, class_name: "Comment", association: :user) # NOTE: @citizen428 Temporarily ignoring this while working out user - profile relationship - Bullet.add_safelist(type: :n_plus_one_query, class_name: "User", association: :profile) - Bullet.add_safelist(type: :n_plus_one_query, class_name: "User", association: :setting) - Bullet.add_safelist(type: :n_plus_one_query, class_name: "User", association: :notification_setting) + Bullet.add_whitelist(type: :n_plus_one_query, class_name: "User", association: :profile) + Bullet.add_whitelist(type: :n_plus_one_query, class_name: "User", association: :setting) + Bullet.add_whitelist(type: :n_plus_one_query, class_name: "User", association: :notification_setting) + # NOTE: @citizen428 Let's ignore this for now, we have to revisit the user - profile relationship anyway + Bullet.add_whitelist(type: :n_plus_one_query, class_name: "Profile", association: :user) # @mstruve: These occur during setting updates, not sure how since we are only dealing with single setting records Bullet.add_safelist(type: :n_plus_one_query, class_name: "Users::Setting", association: :user) Bullet.add_safelist(type: :n_plus_one_query, class_name: "Users::NotificationSetting", association: :user) diff --git a/cypress/integration/seededFlows/profileFlows/userUpdateSettingsProfile.spec.js b/cypress/integration/seededFlows/profileFlows/userUpdateSettingsProfile.spec.js index 81a2d54cb..9dc6fac13 100644 --- a/cypress/integration/seededFlows/profileFlows/userUpdateSettingsProfile.spec.js +++ b/cypress/integration/seededFlows/profileFlows/userUpdateSettingsProfile.spec.js @@ -13,9 +13,13 @@ describe('User Update Settings Profile', () => { const summary = 'This is my story...'; const location = 'New York City'; - cy.findByLabelText(/^Website URL$/i).type(websiteURL); - cy.findByLabelText(/^Bio$/i).type(summary); - cy.findByLabelText(/^Location$/i).type(location); + cy.findByLabelText(/^Website URL$/i) + .clear() + .type(websiteURL); + cy.findByLabelText(/^Bio$/i).clear().type(summary); + cy.findByLabelText(/^Location$/i) + .clear() + .type(location); cy.findByText(/^Save Profile Information$/i).click(); diff --git a/lib/data_update_scripts/20200922072907_backfill_profile_skills_languages.rb b/lib/data_update_scripts/20200922072907_backfill_profile_skills_languages.rb deleted file mode 100644 index ceb5aa634..000000000 --- a/lib/data_update_scripts/20200922072907_backfill_profile_skills_languages.rb +++ /dev/null @@ -1,9 +0,0 @@ -module DataUpdateScripts - class BackfillProfileSkillsLanguages - def run - User.where.not(mostly_work_with: [nil, ""]).find_each do |user| - user.profile.update(skills_languages: user.mostly_work_with) - end - end - end -end diff --git a/lib/data_update_scripts/20201223013903_set_proper_display_area_for_profile_fields.rb b/lib/data_update_scripts/20201223013903_set_proper_display_area_for_profile_fields.rb deleted file mode 100644 index 5ab2d2050..000000000 --- a/lib/data_update_scripts/20201223013903_set_proper_display_area_for_profile_fields.rb +++ /dev/null @@ -1,28 +0,0 @@ -module DataUpdateScripts - class SetProperDisplayAreaForProfileFields - def run - ProfileField.where(attribute_name: %w[brand_color1 - brand_color2 - recruiters_can_contact_me_about_job_opportunities - display_email_on_profile - twitter_url - github_url - facebook_url - linkedin_url - youtube_url - instagram_url - behance_url - medium_url - stackoverflow_url - gitlab_url - twitch_url - mastodon_url - website_url]) - .update_all(display_area: "settings_only") - ProfileField.where.not(display_area: "settings_only").update_all(display_area: "header") - ProfileField.where(attribute_name: - %w[currently_hacking_on currently_learning mostly_work_with available_for]) - .update_all(display_area: "left_sidebar") - end - end -end diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb index fe7592ddd..32e28d69a 100644 --- a/spec/helpers/application_helper_spec.rb +++ b/spec/helpers/application_helper_spec.rb @@ -14,7 +14,7 @@ RSpec.describe ApplicationHelper, type: :helper do expect(user.darker_color).to eq Color::CompareHex.new(described_class::USER_COLORS).brightness expect(user.username).to eq "[deleted user]" expect(user.name).to eq "[Deleted User]" - expect(user.summary).to be_nil + expect(user.tag_line).to be_nil expect(user.twitter_username).to be_nil expect(user.github_username).to be_nil end diff --git a/spec/lib/data_update_scripts/set_proper_display_area_for_profile_fields_spec.rb b/spec/lib/data_update_scripts/set_proper_display_area_for_profile_fields_spec.rb deleted file mode 100644 index 15110f57a..000000000 --- a/spec/lib/data_update_scripts/set_proper_display_area_for_profile_fields_spec.rb +++ /dev/null @@ -1,13 +0,0 @@ -require "rails_helper" -require Rails.root.join( - "lib/data_update_scripts/20201223013903_set_proper_display_area_for_profile_fields.rb", -) - -describe DataUpdateScripts::SetProperDisplayAreaForProfileFields do - it "migrates profile fields to proper areas" do - # Run the script - described_class.new.run - expect(ProfileField.find_by(attribute_name: "education").display_area).to eq("header") - expect(ProfileField.find_by(attribute_name: "currently_learning").display_area).to eq("left_sidebar") - end -end diff --git a/spec/models/profile_field_spec.rb b/spec/models/profile_field_spec.rb index c8cc784a0..c55f898bd 100644 --- a/spec/models/profile_field_spec.rb +++ b/spec/models/profile_field_spec.rb @@ -48,5 +48,17 @@ RSpec.describe ProfileField, type: :model do field = create(:profile_field, label: "Test? Test! 1") expect(field.attribute_name).to eq "test_test1" end + + describe "#maximum_header_field_count" do + it "limits the number of header fields" do + count = [0, described_class::HEADER_FIELD_LIMIT - described_class.header.count].max + create_list(:profile_field, count, :header) + + expected_message = /#{Regexp.quote(ProfileField::HEADER_LIMIT_MESSAGE)}/ + + expect { create(:profile_field, :header) } + .to raise_error(ActiveRecord::RecordInvalid, expected_message) + end + end end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 095637fe6..9a7129b21 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -413,7 +413,7 @@ RSpec.describe User, type: :model do user = described_class.find(create(:user, :ignore_mailchimp_subscribe_callback).id) sidekiq_assert_no_enqueued_jobs(only: Users::SubscribeToMailchimpNewsletterWorker) do - user.update(website_url: "http://example.com") + user.update(credits_count: 100) end end end @@ -827,14 +827,5 @@ RSpec.describe User, type: :model do expect(user.profile).to be_present expect(user.profile).to respond_to(:available_for) end - - it "propagates changes of unmapped attributes to the profile model", :aggregate_failures do - expect do - user.update(available_for: "profile migrations") - end.to change { user.profile.reload.available_for }.from(nil).to("profile migrations") - - # Changes were also persisted in the users table - expect(user.reload.available_for).to eq "profile migrations" - end end end diff --git a/spec/requests/api/v0/organizations_spec.rb b/spec/requests/api/v0/organizations_spec.rb index 42ac31dd5..05dc5d4b3 100644 --- a/spec/requests/api/v0/organizations_spec.rb +++ b/spec/requests/api/v0/organizations_spec.rb @@ -58,12 +58,15 @@ RSpec.describe "Api::V0::Organizations", type: :request do expect(response_org_users["type_of"]).to eq("user") - %w[ - id username name summary twitter_username github_username website_url location - ].each do |attr| + %w[id username name twitter_username github_username].each do |attr| expect(response_org_users[attr]).to eq(org_user.public_send(attr)) end + org_user_profile = org_user.profile + %w[summary location website_url].each do |attr| + expect(response_org_users[attr]).to eq(org_user_profile.public_send(attr)) + end + expect(response_org_users["joined_at"]).to eq(org_user.created_at.strftime("%b %e, %Y")) expect(response_org_users["profile_image"]).to eq(Images::Profile.call(org_user.profile_image_url, length: 320)) end @@ -104,10 +107,12 @@ RSpec.describe "Api::V0::Organizations", type: :request do expect(response_listing["tag_list"]).to eq(listing.cached_tag_list) expect(response_listing["tags"]).to match_array(listing.tag_list) - %w[name username twitter_username github_username website_url].each do |attr| + %w[name username twitter_username github_username].each do |attr| expect(response_listing["user"][attr]).to eq(org_user.public_send(attr)) end + expect(response_listing["organization"]["website_url"]).to eq(org_user.profile.website_url) + %w[name username slug].each do |attr| expect(response_listing["organization"][attr]).to eq(organization.public_send(attr)) end @@ -149,10 +154,12 @@ RSpec.describe "Api::V0::Organizations", type: :request do expect(response_article["tag_list"]).to match_array(article.tag_list) - %w[name username twitter_username github_username website_url].each do |attr| + %w[name username twitter_username github_username].each do |attr| expect(response_article["user"][attr]).to eq(org_user.public_send(attr)) end + expect(response_article["user"]["website_url"]).to eq(org_user.profile.website_url) + %w[name username slug].each do |attr| expect(response_article["organization"][attr]).to eq(organization.public_send(attr)) end diff --git a/spec/requests/api/v0/users_spec.rb b/spec/requests/api/v0/users_spec.rb index 1ff4e435a..cc27fa633 100644 --- a/spec/requests/api/v0/users_spec.rb +++ b/spec/requests/api/v0/users_spec.rb @@ -2,7 +2,12 @@ require "rails_helper" RSpec.describe "Api::V0::Users", type: :request do describe "GET /api/users/:id" do - let(:user) { create(:user, summary: "Something something", profile_image: "") } + let!(:user) do + create(:user, + profile_image: "", + _skip_creating_profile: true, + profile: create(:profile, summary: "Something something")) + end it "returns 404 if the user id is not found" do get api_user_path("invalid-id") @@ -39,12 +44,14 @@ RSpec.describe "Api::V0::Users", type: :request do expect(response_user["type_of"]).to eq("user") - %w[ - id username name summary twitter_username github_username website_url location - ].each do |attr| + %w[id username name twitter_username github_username].each do |attr| expect(response_user[attr]).to eq(user.public_send(attr)) end + %w[summary website_url location].each do |attr| + expect(response_user[attr]).to eq(user.profile.public_send(attr)) + end + expect(response_user["joined_at"]).to eq(user.created_at.strftime("%b %e, %Y")) expect(response_user["profile_image"]).to eq(Images::Profile.call(user.profile_image_url, length: 320)) end @@ -67,12 +74,14 @@ RSpec.describe "Api::V0::Users", type: :request do expect(response_user["type_of"]).to eq("user") - %w[ - id username name summary twitter_username github_username website_url location - ].each do |attr| + %w[id username name twitter_username github_username].each do |attr| expect(response_user[attr]).to eq(user.public_send(attr)) end + %w[summary website_url location].each do |attr| + expect(response_user[attr]).to eq(user.profile.public_send(attr)) + end + expect(response_user["joined_at"]).to eq(user.created_at.strftime("%b %e, %Y")) expect(response_user["profile_image"]).to eq(Images::Profile.call(user.profile_image_url, length: 320)) end @@ -85,12 +94,14 @@ RSpec.describe "Api::V0::Users", type: :request do expect(response_user["type_of"]).to eq("user") - %w[ - id username name summary twitter_username github_username website_url location - ].each do |attr| + %w[id username name twitter_username github_username].each do |attr| expect(response_user[attr]).to eq(user.public_send(attr)) end + %w[summary website_url location].each do |attr| + expect(response_user[attr]).to eq(user.profile.public_send(attr)) + end + expect(response_user["joined_at"]).to eq(user.created_at.strftime("%b %e, %Y")) expect(response_user["profile_image"]).to eq(Images::Profile.call(user.profile_image_url, length: 320)) end diff --git a/spec/requests/user/user_profile_spec.rb b/spec/requests/user/user_profile_spec.rb index feb76c4a1..4540b0d1d 100644 --- a/spec/requests/user/user_profile_spec.rb +++ b/spec/requests/user/user_profile_spec.rb @@ -98,19 +98,13 @@ RSpec.describe "UserProfiles", type: :request do expect(response.body.split("Whoaaaa").first).to include "crayons-layout__sidebar-left" end - it "does not render settings_only on page" do - create(:profile_field, label: "whoaaaa", display_area: "settings_only") - get "/#{user.username}" - expect(response.body).not_to include "Whoaaaa" - end - it "does not render special display header elements naively" do - user.location = "hawaii" + user.profile.location = "hawaii" user.save get "/#{user.username}" # Does not include the word, but does include the SVG expect(response.body).not_to include "

    Location

    " - expect(response.body).to include user.location + expect(response.body).to include user.profile.location expect(response.body).to include "M18.364 17.364L12 23.728l-6.364-6.364a9 9 0 1112.728 0zM12 13a2 2 0 100-4 2 2 0" end diff --git a/spec/requests/user/user_settings_spec.rb b/spec/requests/user/user_settings_spec.rb index f3c608764..99054f809 100644 --- a/spec/requests/user/user_settings_spec.rb +++ b/spec/requests/user/user_settings_spec.rb @@ -238,14 +238,9 @@ RSpec.describe "UserSettings", type: :request do describe "PUT /update/:id" do before { sign_in user } - it "updates summary" do - put "/users/#{user.id}", params: { user: { tab: "profile", summary: "Hello new summary" } } - expect(user.summary).to eq("Hello new summary") - end - it "updates profile_updated_at" do user.update_column(:profile_updated_at, 2.weeks.ago) - put "/users/#{user.id}", params: { user: { tab: "profile", summary: "Hello new summary" } } + put "/users/#{user.id}", params: { user: { tab: "profile", username: "new_username" } } expect(user.reload.profile_updated_at).to be > 2.minutes.ago end diff --git a/spec/requests/user/user_show_spec.rb b/spec/requests/user/user_show_spec.rb index cea6f353a..4df22ca6a 100644 --- a/spec/requests/user/user_show_spec.rb +++ b/spec/requests/user/user_show_spec.rb @@ -38,8 +38,7 @@ RSpec.describe "UserShow", type: :request do "image" => Images::Profile.call(user.profile_image_url, length: 320), "name" => user.name, "email" => user.email, - "description" => user.summary, - "alumniOf" => user.education, + "description" => user.tag_line, ) end # rubocop:enable RSpec/ExampleLength diff --git a/spec/requests/user/user_suggestions_spec.rb b/spec/requests/user/user_suggestions_spec.rb index 32eb182f3..ed2d538e8 100644 --- a/spec/requests/user/user_suggestions_spec.rb +++ b/spec/requests/user/user_suggestions_spec.rb @@ -38,7 +38,7 @@ RSpec.describe "Users", type: :request do "id" => suggested_user.id, "name" => suggested_user.name, "username" => suggested_user.username, - "summary" => suggested_user.summary, + "summary" => suggested_user.profile.summary, "profile_image_url" => Images::Profile.call(suggested_user.profile_image_url, length: 90), "following" => false, ) @@ -86,7 +86,7 @@ RSpec.describe "Users", type: :request do "id" => suggested_user.id, "name" => suggested_user.name, "username" => suggested_user.username, - "summary" => suggested_user.summary, + "summary" => suggested_user.profile.summary, "profile_image_url" => Images::Profile.call(suggested_user.profile_image_url, length: 90), "following" => false, ) diff --git a/spec/requests/users_onboarding_spec.rb b/spec/requests/users_onboarding_spec.rb index 76e9fa8e1..8cd5f089d 100644 --- a/spec/requests/users_onboarding_spec.rb +++ b/spec/requests/users_onboarding_spec.rb @@ -1,7 +1,12 @@ require "rails_helper" RSpec.describe "UsersOnboarding", type: :request do - let(:user) { create(:user, saw_onboarding: false, location: "Llama Town") } + let!(:user) do + create(:user, + saw_onboarding: false, + _skip_creating_profile: true, + profile: create(:profile, location: "Llama Town")) + end describe "PATCH /onboarding_update" do context "when signed in" do diff --git a/spec/services/profiles/update_spec.rb b/spec/services/profiles/update_spec.rb index 6cee30743..95f86d351 100644 --- a/spec/services/profiles/update_spec.rb +++ b/spec/services/profiles/update_spec.rb @@ -16,9 +16,8 @@ RSpec.describe Profiles::Update, type: :service do let(:user) { profile.user } it "correctly typecasts new attributes", :aggregate_failures do - described_class.call(user, profile: { location: 123, education: "false" }) - expect(user.location).to eq "123" - expect(profile.education).to eq "false" + described_class.call(user, profile: { location: 123 }) + expect(user.profile.location).to eq "123" end it "removes old attributes from the profile" do @@ -87,7 +86,7 @@ RSpec.describe Profiles::Update, type: :service do it "enqueues resave articles job when changing summary" do sidekiq_assert_resave_article_worker(user) do - described_class.call(user, profile: { summary: "#{user.summary} changed" }) + described_class.call(user, profile: { summary: "#{user.profile.summary} changed" }) end end diff --git a/spec/support/seeds/seeds_e2e.rb b/spec/support/seeds/seeds_e2e.rb index 4793e7807..91283b01e 100644 --- a/spec/support/seeds/seeds_e2e.rb +++ b/spec/support/seeds/seeds_e2e.rb @@ -32,7 +32,6 @@ seeder.create_if_doesnt_exist(User, "email", "admin@forem.local") do email: "admin@forem.local", username: "Admin_McAdmin", profile_image: File.open(Rails.root.join("app/assets/images/#{rand(1..40)}.png")), - website_url: Faker::Internet.url, confirmed_at: Time.current, password: "password", password_confirmation: "password", @@ -51,6 +50,7 @@ seeder.create_if_doesnt_exist(User, "email", "admin@forem.local") do work: "Software developer at Company", location: "Edinburgh", education: "University of Life", + website_url: Faker::Internet.url, ) user.add_role(:super_admin) @@ -71,7 +71,6 @@ seeder.create_if_doesnt_exist(User, "email", "trusted-user-1@forem.local") do email: "trusted-user-1@forem.local", username: "trusted_user_1", profile_image: File.open(Rails.root.join("app/assets/images/#{rand(1..40)}.png")), - website_url: Faker::Internet.url, confirmed_at: Time.current, password: "password", password_confirmation: "password", @@ -84,6 +83,8 @@ seeder.create_if_doesnt_exist(User, "email", "trusted-user-1@forem.local") do email_follower_notifications: false, ) + user.profile.update(website_url: Faker::Internet.url) + user.add_role(:trusted) end @@ -113,9 +114,7 @@ seeder.create_if_doesnt_exist(User, "email", "change-password-user@forem.com") d name: "Change Password User", email: "change-password-user@forem.com", username: "changepassworduser", - summary: Faker::Lorem.paragraph_by_chars(number: 199, supplemental: false), profile_image: File.open(Rails.root.join("app/assets/images/#{rand(1..40)}.png")), - website_url: Faker::Internet.url, confirmed_at: Time.current, password: "password", password_confirmation: "password", @@ -127,6 +126,10 @@ seeder.create_if_doesnt_exist(User, "email", "change-password-user@forem.com") d email_comment_notifications: false, email_follower_notifications: false, ) + user.profile.update( + summary: Faker::Lorem.paragraph_by_chars(number: 199, supplemental: false), + website_url: Faker::Internet.url, + ) user end @@ -137,9 +140,7 @@ seeder.create_if_doesnt_exist(User, "email", "article-editor-v1-user@forem.com") name: "Article Editor v1 User", email: "article-editor-v1-user@forem.local", username: "article_editor_v1_user", - summary: Faker::Lorem.paragraph_by_chars(number: 199, supplemental: false), profile_image: File.open(Rails.root.join("app/assets/images/#{rand(1..40)}.png")), - website_url: Faker::Internet.url, confirmed_at: Time.current, password: "password", password_confirmation: "password", @@ -152,6 +153,10 @@ seeder.create_if_doesnt_exist(User, "email", "article-editor-v1-user@forem.com") email_comment_notifications: false, email_follower_notifications: false, ) + user.profile.update( + summary: Faker::Lorem.paragraph_by_chars(number: 199, supplemental: false), + website_url: Faker::Internet.url, + ) user end @@ -162,9 +167,7 @@ seeder.create_if_doesnt_exist(User, "email", "article-editor-v2-user@forem.com") name: "Article Editor v2 User", email: "article-editor-v2-user@forem.local", username: "article_editor_v2_user", - summary: Faker::Lorem.paragraph_by_chars(number: 199, supplemental: false), profile_image: File.open(Rails.root.join("app/assets/images/#{rand(1..40)}.png")), - website_url: Faker::Internet.url, confirmed_at: Time.current, password: "password", password_confirmation: "password", @@ -176,6 +179,10 @@ seeder.create_if_doesnt_exist(User, "email", "article-editor-v2-user@forem.com") email_comment_notifications: false, email_follower_notifications: false, ) + user.profile.update( + summary: Faker::Lorem.paragraph_by_chars(number: 199, supplemental: false), + website_url: Faker::Internet.url, + ) user end @@ -186,9 +193,7 @@ chat_user_1 = seeder.create_if_doesnt_exist(User, "email", "chat-user-1@forem.lo name: "Chat user 1", email: "chat-user-1@forem.local", username: "chat_user_1", - summary: Faker::Lorem.paragraph_by_chars(number: 199, supplemental: false), profile_image: File.open(Rails.root.join("app/assets/images/#{rand(1..40)}.png")), - website_url: Faker::Internet.url, confirmed_at: Time.current, password: "password", password_confirmation: "password", @@ -200,6 +205,10 @@ chat_user_1 = seeder.create_if_doesnt_exist(User, "email", "chat-user-1@forem.lo email_comment_notifications: false, email_follower_notifications: false, ) + user.profile.update( + summary: Faker::Lorem.paragraph_by_chars(number: 199, supplemental: false), + website_url: Faker::Internet.url, + ) user end @@ -210,9 +219,7 @@ chat_user_2 = seeder.create_if_doesnt_exist(User, "email", "chat-user-2@forem.lo name: "Chat user 2", email: "chat-user-2@forem.local", username: "chat_user_2", - summary: Faker::Lorem.paragraph_by_chars(number: 199, supplemental: false), profile_image: File.open(Rails.root.join("app/assets/images/#{rand(1..40)}.png")), - website_url: Faker::Internet.url, confirmed_at: Time.current, password: "password", password_confirmation: "password", @@ -224,6 +231,10 @@ chat_user_2 = seeder.create_if_doesnt_exist(User, "email", "chat-user-2@forem.lo email_comment_notifications: false, email_follower_notifications: false, ) + user.profile.update( + summary: Faker::Lorem.paragraph_by_chars(number: 199, supplemental: false), + website_url: Faker::Internet.url, + ) user end @@ -233,9 +244,7 @@ seeder.create_if_doesnt_exist(User, "email", "notifications-user@forem.com") do name: "Notifications User", email: "notifications-user@forem.local", username: "notifications_user", - summary: Faker::Lorem.paragraph_by_chars(number: 199, supplemental: false), profile_image: File.open(Rails.root.join("app/assets/images/#{rand(1..40)}.png")), - website_url: Faker::Internet.url, confirmed_at: Time.current, password: "password", password_confirmation: "password", @@ -247,6 +256,10 @@ seeder.create_if_doesnt_exist(User, "email", "notifications-user@forem.com") do email_comment_notifications: false, email_follower_notifications: false, ) + user.profile.update( + summary: Faker::Lorem.paragraph_by_chars(number: 199, supplemental: false), + website_url: Faker::Internet.url, + ) follow = admin_user.follows.create!(followable: user) Notification.send_new_follower_notification_without_delay(follow) @@ -259,9 +272,7 @@ seeder.create_if_doesnt_exist(User, "email", "liquid-tags-user@forem.com") do name: "Liquid tags User", email: "liquid-tags-user@forem.local", username: "liquid_tags_user", - summary: Faker::Lorem.paragraph_by_chars(number: 199, supplemental: false), profile_image: File.open(Rails.root.join("app/assets/images/#{rand(1..40)}.png")), - website_url: Faker::Internet.url, confirmed_at: Time.current, password: "password", password_confirmation: "password", @@ -273,6 +284,10 @@ seeder.create_if_doesnt_exist(User, "email", "liquid-tags-user@forem.com") do email_comment_notifications: false, email_follower_notifications: false, ) + liquid_tags_user.profile.update( + summary: Faker::Lorem.paragraph_by_chars(number: 199, supplemental: false), + website_url: Faker::Internet.url, + ) admin_user.follows.create!(followable: liquid_tags_user) end @@ -389,9 +404,7 @@ seeder.create_if_doesnt_exist(User, "email", "series-user@forem.com") do name: "Series User", email: "series-user@forem.local", username: "series_user", - summary: Faker::Lorem.paragraph_by_chars(number: 199, supplemental: false), profile_image: File.open(Rails.root.join("app/assets/images/#{rand(1..40)}.png")), - website_url: Faker::Internet.url, confirmed_at: Time.current, password: "password", password_confirmation: "password", @@ -404,6 +417,7 @@ seeder.create_if_doesnt_exist(User, "email", "series-user@forem.com") do work: "Software developer at Company", location: "Edinburgh", education: "University of Life", + website_url: Faker::Internet.url, ) series_user.notification_setting.update( email_comment_notifications: false,