diff --git a/app/assets/images/git_lab.svg b/app/assets/images/git_lab.svg index 9317d8dd8..d83de25ea 100644 --- a/app/assets/images/git_lab.svg +++ b/app/assets/images/git_lab.svg @@ -1,3 +1,3 @@ - + diff --git a/app/assets/images/twemoji/thinking.svg b/app/assets/images/twemoji/thinking.svg index 54b9d0f30..cdcc3962b 100644 --- a/app/assets/images/twemoji/thinking.svg +++ b/app/assets/images/twemoji/thinking.svg @@ -1,6 +1,6 @@ - + diff --git a/app/controllers/api/v0/articles_controller.rb b/app/controllers/api/v0/articles_controller.rb index e787cd3f4..f91c70631 100644 --- a/app/controllers/api/v0/articles_controller.rb +++ b/app/controllers/api/v0/articles_controller.rb @@ -41,7 +41,7 @@ module Api def show @article = Article.published - .includes(:user) + .includes(user: :profile) .select(SHOW_ATTRIBUTES_FOR_SERIALIZATION) .find(params[:id]) .decorate diff --git a/app/controllers/api/v0/comments_controller.rb b/app/controllers/api/v0/comments_controller.rb index 1cf85c150..03085bf6b 100644 --- a/app/controllers/api/v0/comments_controller.rb +++ b/app/controllers/api/v0/comments_controller.rb @@ -12,7 +12,7 @@ module Api commentable = params[:a_id] ? Article.find(params[:a_id]) : PodcastEpisode.find(params[:p_id]) @comments = commentable.comments - .includes(:user) + .includes(user: :profile) .select(ATTRIBUTES_FOR_SERIALIZATION) .arrange @@ -21,7 +21,7 @@ module Api def show tree_with_root_comment = Comment.subtree_of(params[:id].to_i(26)) - .includes(:user) + .includes(user: :profile) .select(ATTRIBUTES_FOR_SERIALIZATION) .arrange diff --git a/app/controllers/api/v0/listings_controller.rb b/app/controllers/api/v0/listings_controller.rb index 181367048..0cf50f64d 100644 --- a/app/controllers/api/v0/listings_controller.rb +++ b/app/controllers/api/v0/listings_controller.rb @@ -25,7 +25,7 @@ module Api def index @listings = Listing.published .select(ATTRIBUTES_FOR_SERIALIZATION) - .includes(:user, :organization, :taggings, :listing_category) + .includes([{ user: :profile }, :organization, :taggings, :listing_category]) if params[:category].present? @listings = @listings.in_category(params[:category]) diff --git a/app/controllers/comments_controller.rb b/app/controllers/comments_controller.rb index fb10f8862..11e803a6f 100644 --- a/app/controllers/comments_controller.rb +++ b/app/controllers/comments_controller.rb @@ -58,7 +58,7 @@ class CommentsController < ApplicationController def create rate_limit!(rate_limit_to_use) - @comment = Comment.new(permitted_attributes(Comment)) + @comment = Comment.includes(user: :profile).new(permitted_attributes(Comment)) @comment.user_id = current_user.id authorize @comment diff --git a/app/controllers/stories_controller.rb b/app/controllers/stories_controller.rb index decfc44eb..0ba2145dd 100644 --- a/app/controllers/stories_controller.rb +++ b/app/controllers/stories_controller.rb @@ -33,7 +33,8 @@ class StoriesController < ApplicationController def show @story_show = true - if (@article = Article.find_by(path: "/#{params[:username].downcase}/#{params[:slug]}")&.decorate) + path = "/#{params[:username].downcase}/#{params[:slug]}" + if (@article = Article.includes(user: :profile).find_by(path: path)&.decorate) handle_article_show elsif (@article = Article.find_by(slug: params[:slug])&.decorate) handle_possible_redirect diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 8d6cdb7d8..b7891c9aa 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -300,7 +300,7 @@ class UsersController < ApplicationController end def default_suggested_users - @default_suggested_users ||= User.where(username: @suggested_users) + @default_suggested_users ||= User.includes(:profile).where(username: @suggested_users) end def determine_follow_suggestions(current_user) diff --git a/app/models/user.rb b/app/models/user.rb index a90b862b6..ecc3c4663 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -5,38 +5,6 @@ class User < ApplicationRecord include CloudinaryHelper include Storext.model - # @citizen428 Preparing to drop profile columns from the users table - PROFILE_COLUMNS = %w[ - available_for - behance_url - bg_color_hex - currently_hacking_on - currently_learning - dribbble_url - education - email_public - employer_name - employer_url - employment_title - facebook_url - gitlab_url - instagram_url - linkedin_url - location - mastodon_url - medium_url - mostly_work_with - stackoverflow_url - summary - text_color_hex - twitch_url - twitch_username - website_url - youtube_url - ].freeze - - self.ignored_columns = PROFILE_COLUMNS - # NOTE: we are using an inline module to keep profile related things together. concerning :Profiles do included do diff --git a/app/services/article_api_index_service.rb b/app/services/article_api_index_service.rb index 879da636d..2efad9cef 100644 --- a/app/services/article_api_index_service.rb +++ b/app/services/article_api_index_service.rb @@ -46,7 +46,7 @@ class ArticleApiIndexService DEFAULT_PER_PAGE end - if (user = User.find_by(username: username)) + if (user = User.includes(:profile).find_by(username: username)) user.articles.published .includes(:organization) .order(published_at: :desc) @@ -54,7 +54,7 @@ class ArticleApiIndexService .per(per_page || num) elsif (organization = Organization.find_by(slug: username)) organization.articles.published - .includes(:user) + .includes(user: :profile) .order(published_at: :desc) .page(page) .per(per_page || num) @@ -64,7 +64,7 @@ class ArticleApiIndexService end def tag_articles - articles = Article.published.cached_tagged_with(tag).includes(:user, :organization) + articles = published_articles_with_users_and_organizations.cached_tagged_with(tag) articles = if Tag.find_by(name: tag)&.requires_approval articles.where(approved: true).order(featured_number: :desc) @@ -79,7 +79,7 @@ class ArticleApiIndexService end def tagged_articles - articles = Article.published.includes(:user, :organization) + articles = published_articles_with_users_and_organizations articles = articles.tagged_with(tags, any: true).unscope(:select) if tags articles = articles.tagged_with(tags_exclude, exclude: true) if tags_exclude @@ -89,14 +89,14 @@ class ArticleApiIndexService end def top_articles - Article.published.includes(:user, :organization) + published_articles_with_users_and_organizations .where("published_at > ?", top.to_i.days.ago) .order(public_reactions_count: :desc) .page(page).per(per_page || DEFAULT_PER_PAGE) end def state_articles(state) - articles = Article.published.includes(:user, :organization) + articles = published_articles_with_users_and_organizations articles = case state when "fresh" @@ -118,9 +118,8 @@ class ArticleApiIndexService end def collection_articles(collection_id) - Article.published + published_articles_with_users_and_organizations .where(collection_id: collection_id) - .includes(:user, :organization) .order(:published_at) .page(page) .per(per_page || DEFAULT_PER_PAGE) @@ -129,8 +128,7 @@ class ArticleApiIndexService def sorted_articles(sort) # This could be expanded to allow additional sorting options if sort == "desc" - Article.published - .includes(:user, :organization) + published_articles_with_users_and_organizations .order(published_at: :desc) .page(page) .per(per_page || DEFAULT_PER_PAGE) @@ -140,11 +138,14 @@ class ArticleApiIndexService end def base_articles - Article.published + published_articles_with_users_and_organizations .where(featured: true) - .includes(:user, :organization) .order(hotness_score: :desc) .page(page) .per(per_page || DEFAULT_PER_PAGE) end + + def published_articles_with_users_and_organizations + Article.published.includes([{ user: :profile }, :organization]) + end end diff --git a/app/services/users/suggest_recent.rb b/app/services/users/suggest_recent.rb index 76f66a9aa..6be2688e5 100644 --- a/app/services/users/suggest_recent.rb +++ b/app/services/users/suggest_recent.rb @@ -34,13 +34,13 @@ module Users def recent_producers(num_weeks = 1) relation_as_array( - User.where(id: tagged_article_user_ids(num_weeks)), + user_relation.where(id: tagged_article_user_ids(num_weeks)), limit: 80, ) end def recent_top_producers - relation = User.where( + relation = user_relation.where( articles_count: established_user_article_count.., comments_count: established_user_comment_count.., ) @@ -48,7 +48,7 @@ module Users end def recent_commenters(num_comments = 2, limit = 8) - relation_as_array(User.where(comments_count: num_comments + 1..), limit: limit) + relation_as_array(user_relation.where(comments_count: num_comments + 1..), limit: limit) end def relation_as_array(relation, limit:) @@ -58,13 +58,13 @@ module Users def established_user_article_count Rails.cache.fetch("established_user_article_count", expires_in: 1.day) do - User.where(articles_count: 1..).average(:articles_count) || User.average(:articles_count) + user_relation.where(articles_count: 1..).average(:articles_count) || User.average(:articles_count) end end def established_user_comment_count Rails.cache.fetch("established_user_comment_count", expires_in: 1.day) do - User.where(comments_count: 1..).average(:comments_count) || User.average(:comments_count) + user_relation.where(comments_count: 1..).average(:comments_count) || User.average(:comments_count) end end @@ -73,5 +73,9 @@ module Users Article.where(score: 0..).average(:score) || Article.average(:score) end end + + def user_relation + User.includes(:profile) + end end end diff --git a/config/environments/test.rb b/config/environments/test.rb index cba190902..cbcd1f520 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -80,12 +80,11 @@ Rails.application.configure do # Supress incorrect warnings from Bullet due to included columns: https://github.com/flyerhzm/bullet/issues/147 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_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) + # @citizen428: We have not yet resolved all user - profile preloads related to profilge generalization + Bullet.add_safelist(type: :n_plus_one_query, class_name: "User", association: :profile) + Bullet.add_safelist(type: :n_plus_one_query, class_name: "Profile", association: :user) + 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) # @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/db/seeds.rb b/db/seeds.rb index 53ceec498..25b85fd42 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -52,15 +52,6 @@ end ############################################################################## -# NOTE: @citizen428 For the time being we want all current DEV profile fields. -# The CSV import is idempotent by itself, since it uses find_or_create_by. -seeder.create("Creating DEV profile fields") do - dev_fields_csv = Rails.root.join("lib/data/dev_profile_fields.csv") - ProfileFields::ImportFromCsv.call(dev_fields_csv) -end - -############################################################################## - num_users = 10 * SEEDS_MULTIPLIER users_in_random_order = seeder.create_if_none(User, num_users) do diff --git a/lib/data/dev_profile_fields.csv b/lib/data/dev_profile_fields.csv deleted file mode 100644 index c6a85679d..000000000 --- a/lib/data/dev_profile_fields.csv +++ /dev/null @@ -1,6 +0,0 @@ -Education,text_field,,,Work,header,false -Skills/Languages,text_area,,What tools and languages are you most experienced with? Are you specialized or more of a generalist?,Coding,left_sidebar,false -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 -Work,text_field,"What do you do? Example: CEO at ACME Inc.",,Work,header,false diff --git a/lib/data_update_scripts/20200901040521_create_profile_fields.rb b/lib/data_update_scripts/20200901040521_create_profile_fields.rb index 6b77e76df..8a8ec51ba 100644 --- a/lib/data_update_scripts/20200901040521_create_profile_fields.rb +++ b/lib/data_update_scripts/20200901040521_create_profile_fields.rb @@ -1,10 +1,7 @@ module DataUpdateScripts class CreateProfileFields def run - # NOTE: the CSV importer uses find_or_create_by for both fields and - # groups, so this operation is idempotent. - csv = Rails.root.join("lib/data/dev_profile_fields.csv") - ProfileFields::ImportFromCsv.call(csv) + # This is no longer needed 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 deleted file mode 100644 index 01a147d56..000000000 --- a/spec/lib/data_update_scripts/create_profile_fields_spec.rb +++ /dev/null @@ -1,35 +0,0 @@ -require "rails_helper" -require Rails.root.join("lib/data_update_scripts/20200901040521_create_profile_fields.rb") - -describe DataUpdateScripts::CreateProfileFields do - def profile_field_and_group_count - [ProfileField.count, ProfileFieldGroup.count] - end - - before do - ProfileFieldGroup.destroy_all - ProfileField.destroy_all - end - - context "when no profile fields or groups exist" 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([6, 2]) - end - end - - context "when profile fields and/or groups already exist" do - before do - csv = Rails.root.join("lib/data/dev_profile_fields.csv") - ProfileFields::ImportFromCsv.call(csv) - end - - it "works when profile fields or groups already exist", :aggregate_failures do - expect do - described_class.new.run - end.not_to change { profile_field_and_group_count } - expect(profile_field_and_group_count).to eq [6, 2] - end - end -end diff --git a/spec/models/profile_field_spec.rb b/spec/models/profile_field_spec.rb index c55f898bd..ecf6493b4 100644 --- a/spec/models/profile_field_spec.rb +++ b/spec/models/profile_field_spec.rb @@ -11,7 +11,13 @@ RSpec.describe ProfileField, type: :model do it { is_expected.to validate_presence_of(:display_area) } it { is_expected.to validate_presence_of(:input_type) } it { is_expected.to validate_presence_of(:label) } - it { is_expected.to validate_uniqueness_of(:label).case_insensitive } + end + + it "ensures the label is case-insensitively unique" do + create(:profile_field, label: "Test") + expect do + described_class.create!(label: "tEsT") + end.to raise_error(ActiveRecord::RecordInvalid, /Label has already been taken/) end describe "#maximum_header_field_count" do @@ -29,7 +35,7 @@ RSpec.describe ProfileField, type: :model do it "limits the number of header fields on update", :aggregate_errors do expect(described_class.header.count).to be >= 3 - profile_field = described_class.left_sidebar.first + profile_field = create(:profile_field, display_area: :left_sidebar) expect { profile_field.header! } .to raise_error(ActiveRecord::RecordInvalid, expected_message) diff --git a/spec/models/profile_spec.rb b/spec/models/profile_spec.rb index 3b481f2f3..e99de19a7 100644 --- a/spec/models/profile_spec.rb +++ b/spec/models/profile_spec.rb @@ -32,16 +32,20 @@ RSpec.describe Profile, type: :model do end describe "validating text areas" do + before do + create(:profile_field, label: "Test Text Area", input_type: :text_area) + end + it "is valid if the text is short enough" do - profile.skills_languages = "Ruby" + profile.test_text_area = "Ruby" expect(profile).to be_valid end it "is invalid if the text is too long" do - profile.skills_languages = "x" * ProfileValidator::MAX_TEXT_AREA_LENGTH.next + profile.test_text_area = "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 is 200 characters)" + .to eq "Test text area is too long (maximum is 200 characters)" end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 9a7129b21..c49c2ae96 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -825,7 +825,7 @@ RSpec.describe User, type: :model do it "automatically creates a profile for new users", :aggregate_failures do user = create(:user) expect(user.profile).to be_present - expect(user.profile).to respond_to(:available_for) + expect(user.profile).to respond_to(:location) end end end diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 699cc3284..6feea0b10 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -103,12 +103,6 @@ RSpec.configure do |config| # Set the TZ ENV variable with the current random timezone from zonebie # which we can then use to properly set the browser time for Capybara specs ENV["TZ"] = Time.zone.tzinfo.name - - # NOTE: @citizen428 needed while we delegate from User to Profile to keep - # spec changes limited for the time being. - csv = Rails.root.join("lib/data/dev_profile_fields.csv") - ProfileFields::ImportFromCsv.call(csv) - Profile.refresh_attributes! end config.before do diff --git a/spec/requests/profile_preview_cards_spec.rb b/spec/requests/profile_preview_cards_spec.rb index 02c5cf5a8..8ff4bad42 100644 --- a/spec/requests/profile_preview_cards_spec.rb +++ b/spec/requests/profile_preview_cards_spec.rb @@ -44,6 +44,11 @@ RSpec.describe "ProfilePreviewCards", type: :request do end describe "GET /:id as JSON" do + before do + create(:profile_field, label: "Education", display_area: :header) + create(:profile_field, label: "Work", display_area: :header) + end + let(:profile) { user.profile } context "when signed out" do diff --git a/spec/requests/stories_show_spec.rb b/spec/requests/stories_show_spec.rb index 59373a45d..d276a7c50 100644 --- a/spec/requests/stories_show_spec.rb +++ b/spec/requests/stories_show_spec.rb @@ -187,10 +187,12 @@ RSpec.describe "StoriesShow", type: :request do end it "handles invalid slug characters" do - allow(Article).to receive(:find_by).and_raise(ArgumentError) + # rubocop:disable RSpec/MessageChain + allow(Article).to receive_message_chain(:includes, :find_by).and_raise(ArgumentError) + # rubocop:enable RSpec/MessageChain get article.path - expect(response.status).to be(400) + expect(response.status).to eq(400) end it "has noindex if article has low score" do diff --git a/spec/requests/users_onboarding_spec.rb b/spec/requests/users_onboarding_spec.rb index 8cd5f089d..6a59eeb22 100644 --- a/spec/requests/users_onboarding_spec.rb +++ b/spec/requests/users_onboarding_spec.rb @@ -38,10 +38,10 @@ RSpec.describe "UsersOnboarding", type: :request do end it "updates the user's profile" do - params = { profile: { work: "Emperor at Galactic Empire" } } + params = { profile: { location: "Galactic Empire" } } expect do patch "/onboarding_update.json", params: params - end.to change(user.profile, :work).to("Emperor at Galactic Empire") + end.to change(user.profile, :location).to("Galactic Empire") end it "does not update the user's last_onboarding_page if it is empty" do diff --git a/spec/services/users/update_spec.rb b/spec/services/users/update_spec.rb index 618846275..6ddf135bd 100644 --- a/spec/services/users/update_spec.rb +++ b/spec/services/users/update_spec.rb @@ -11,7 +11,7 @@ RSpec.describe Users::Update, type: :service do end let(:profile) do - create(:profile, data: { education: "maybe", removed: "Bla" }) + create(:profile, data: { test_field: "maybe", removed: "Bla" }) end let(:user) { profile.user } @@ -33,8 +33,9 @@ RSpec.describe Users::Update, type: :service do end it "updates the profile_updated_at column" do + create(:profile_field, label: "Test field") expect do - described_class.call(user, profile: { education: "false" }) + described_class.call(user, profile: { test_field: "false" }) end.to change { user.reload.profile_updated_at } end diff --git a/spec/support/seeds/seeds_e2e.rb b/spec/support/seeds/seeds_e2e.rb index 4cee93907..881a7a9bb 100644 --- a/spec/support/seeds/seeds_e2e.rb +++ b/spec/support/seeds/seeds_e2e.rb @@ -17,12 +17,10 @@ Settings::SMTP.password = "password" ############################################################################## -# NOTE: @citizen428 For the time being we want all current DEV profile fields. -# The CSV import is idempotent by itself, since it uses find_or_create_by. -seeder.create("Creating DEV profile fields") do - dev_fields_csv = Rails.root.join("lib/data/dev_profile_fields.csv") - ProfileFields::ImportFromCsv.call(dev_fields_csv) -end +# Some of our Cypress tests assume specific DEV profile fields to exist +ProfileField.create!(label: "Work", display_area: :header) +ProfileField.create!(label: "Education", display_area: :header) +Profile.refresh_attributes! ############################################################################## diff --git a/spec/workers/moderator/banish_user_worker_spec.rb b/spec/workers/moderator/banish_user_worker_spec.rb index 6d10e4c71..b32a6e342 100644 --- a/spec/workers/moderator/banish_user_worker_spec.rb +++ b/spec/workers/moderator/banish_user_worker_spec.rb @@ -9,7 +9,8 @@ RSpec.describe Moderator::BanishUserWorker, type: :worker do let(:admin) { create(:user, :super_admin) } before do - user.profile.update!(currently_hacking_on: "text is here") + create(:profile_field, label: "Test field") + user.profile.update!(test_field: "text is here") create(:article, user_id: user.id) create(:article, user_id: user.id) create(:listing, user: user) @@ -34,7 +35,7 @@ RSpec.describe Moderator::BanishUserWorker, type: :worker do end it "reassigns profile info" do - expect(user.profile.currently_hacking_on).to be_blank + expect(user.profile.test_field).to be_blank end it "creates an entry in the BanishedUsers table" do