diff --git a/app/assets/images/forem-background.svg b/app/assets/images/forem-background.svg index de4181e66..795d995ad 100644 --- a/app/assets/images/forem-background.svg +++ b/app/assets/images/forem-background.svg @@ -1,5 +1,3 @@ diff --git a/app/assets/stylesheets/views/comments.scss b/app/assets/stylesheets/views/comments.scss index 2996cbea4..22e5d833b 100644 --- a/app/assets/stylesheets/views/comments.scss +++ b/app/assets/stylesheets/views/comments.scss @@ -273,5 +273,5 @@ .personal-template-button:not(.active), .moderator-template-button:not(.active) { - @extend .crayons-btn--outlined; + @extend .crayons-btn--outlined; } diff --git a/app/controllers/api/v0/organizations_controller.rb b/app/controllers/api/v0/organizations_controller.rb index a2e6cf8a9..1d14dd317 100644 --- a/app/controllers/api/v0/organizations_controller.rb +++ b/app/controllers/api/v0/organizations_controller.rb @@ -33,7 +33,7 @@ module Api num = [per_page, 1000].min page = params[:page] || 1 - @users = @organization.users.select(USERS_FOR_SERIALIZATION).page(page).per(num) + @users = @organization.users.joins(:profile).select(USERS_FOR_SERIALIZATION).page(page).per(num) end def listings diff --git a/app/controllers/api/v0/readinglist_controller.rb b/app/controllers/api/v0/readinglist_controller.rb index ece02ac28..67d3bb151 100644 --- a/app/controllers/api/v0/readinglist_controller.rb +++ b/app/controllers/api/v0/readinglist_controller.rb @@ -30,6 +30,7 @@ module Api .decorate @users_by_id = User + .joins(:profile) .select(UsersController::SHOW_ATTRIBUTES_FOR_SERIALIZATION) .find(articles.map(&:user_id)) .index_by(&:id) diff --git a/app/controllers/api/v0/users_controller.rb b/app/controllers/api/v0/users_controller.rb index 34f1853c7..4e9a47af3 100644 --- a/app/controllers/api/v0/users_controller.rb +++ b/app/controllers/api/v0/users_controller.rb @@ -10,7 +10,7 @@ module Api ].freeze def show - relation = User.select(SHOW_ATTRIBUTES_FOR_SERIALIZATION) + relation = User.joins(:profile).select(SHOW_ATTRIBUTES_FOR_SERIALIZATION) @user = if params[:id] == "by_username" relation.find_by!(username: params[:url]) diff --git a/app/controllers/profiles_controller.rb b/app/controllers/profiles_controller.rb index 0a789d9ae..59b0f0999 100644 --- a/app/controllers/profiles_controller.rb +++ b/app/controllers/profiles_controller.rb @@ -5,7 +5,7 @@ class ProfilesController < ApplicationController ALLOWED_USERS_SETTING_PARAMS = %i[display_email_on_profile brand_color1 brand_color2].freeze def update - update_result = Profiles::Update.call(current_user, update_params) + update_result = Users::Update.call(current_user, update_params) if update_result.success? flash[:settings_notice] = "Your profile has been updated" redirect_to user_settings_path diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb index c8fe7c1ea..6a2540128 100644 --- a/app/controllers/registrations_controller.rb +++ b/app/controllers/registrations_controller.rb @@ -14,8 +14,6 @@ class RegistrationsController < Devise::RegistrationsController def create authorize(params, policy_class: RegistrationPolicy) - resolve_profile_field_issues - unless recaptcha_verified? flash[:notice] = "You must complete the recaptcha ✅" return redirect_to new_user_registration_path(state: "email_signup") @@ -58,16 +56,6 @@ class RegistrationsController < Devise::RegistrationsController end end - def resolve_profile_field_issues - # Run this data update script when in a state of "first user" in the event - # that we are in a state where this was not already run. - # This is likely only temporarily needed. - return unless Settings::General.waiting_on_first_user - - csv = Rails.root.join("lib/data/dev_profile_fields.csv") - ProfileFields::ImportFromCsv.call(csv) - end - def check_allowed_email(resource) domain = resource.email.split("@").last allow_list = Settings::Authentication.allowed_registration_email_domains diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index fcb5d4923..8d6cdb7d8 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -180,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 = Users::Update.call(current_user, user: user_params, profile: profile_params) render_update_response(update_result.success?, update_result.errors_as_sentence) end diff --git a/app/models/user.rb b/app/models/user.rb index 4c2fb821c..2a514d9a9 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -35,18 +35,13 @@ class User < ApplicationRecord youtube_url ].freeze - INACTIVE_PROFILE_COLUMNS = %w[ - bg_color_hex - text_color_hex - email_public - ].freeze - self.ignored_columns = PROFILE_COLUMNS - # NOTE: @citizen428 This is temporary code during profile migration and will - # be removed. - concerning :ProfileMigration do + # NOTE: we are using an inline module to keep profile related things together. + concerning :Profiles do included do + has_one :profile, dependent: :destroy + # NOTE: There are rare cases were we want to skip this callback, primarily # in tests. `skip_callback` modifies global state, which is not thread-safe # and can cause hard to track down bugs. We use an instance-level attribute @@ -55,14 +50,6 @@ class User < ApplicationRecord # All new users should automatically have a profile after_create_commit -> { Profile.create(user: self) }, unless: :_skip_creating_profile - - # Getters and setters for unmapped profile attributes - 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 end end @@ -88,7 +75,6 @@ class User < ApplicationRecord acts_as_followable acts_as_follower - has_one :profile, dependent: :destroy has_one :notification_setting, class_name: "Users::NotificationSetting", dependent: :destroy has_one :setting, class_name: "Users::Setting", dependent: :destroy @@ -264,9 +250,6 @@ class User < ApplicationRecord before_destroy :unsubscribe_from_newsletters, prepend: true before_destroy :destroy_follows, prepend: true - # NOTE: @citizen428 Temporary while migrating to generalized profiles - after_save { |user| user.profile&.save if user.profile&.changed? } - after_create_commit :send_welcome_notification after_commit :subscribe_to_mailchimp_newsletter diff --git a/app/services/users/suggest_recent.rb b/app/services/users/suggest_recent.rb index 975c53525..887f5dbf2 100644 --- a/app/services/users/suggest_recent.rb +++ b/app/services/users/suggest_recent.rb @@ -52,7 +52,7 @@ module Users end def relation_as_array(relation, limit:) - relation = relation.select(attributes_to_select) if attributes_to_select.any? + relation = relation.joins(:profile).select(attributes_to_select) if attributes_to_select.any? relation.order(updated_at: :desc).limit(limit).to_a end diff --git a/app/services/profiles/update.rb b/app/services/users/update.rb similarity index 92% rename from app/services/profiles/update.rb rename to app/services/users/update.rb index f511b8e89..7adf29f13 100644 --- a/app/services/profiles/update.rb +++ b/app/services/users/update.rb @@ -1,6 +1,4 @@ -# TODO: This should probably become `Users::Update` as we're dealing with core -# user attributes, profile attributes and user settings. -module Profiles +module Users # Deals with updates that affect fields on +Profile+ and/or +User+ in a transparent way. class Update using HashAnyKey @@ -15,11 +13,11 @@ module Profiles # and/or user attributes to update. The corresponding has keys are +:user+ and # +:profile+ respectively. # @example - # Profiles::Update.call( + # Users::Update.call( # current_user, # profile: { website_url: "https://example.com" }, # ) - # @return [Profiles::Update] a class instance that can be used for success checks + # @return [Users::Update] a class instance that can be used for success checks def self.call(user, updated_attributes = {}) new(user, updated_attributes).call end diff --git a/app/validators/profile_validator.rb b/app/validators/profile_validator.rb index 1c9f76e58..064edcb01 100644 --- a/app/validators/profile_validator.rb +++ b/app/validators/profile_validator.rb @@ -11,9 +11,9 @@ class ProfileValidator < ActiveModel::Validator }.with_indifferent_access.freeze def validate(record) - # NOTE: @citizen428 The summary is a base profile field, which we add to all - # new Forem instances, so it should be safe to validate. The method itself - # also guards against the field's absence. + # NOTE: The summary is a base profile field, which we add to all new Forem + # instances, so it should be safe to validate. The method itself also guards + # against the field's absence. record.errors.add(:summary, "is too long") if summary_too_long?(record) ProfileField.all.each do |field| diff --git a/app/views/users/_metadata.html.erb b/app/views/users/_metadata.html.erb index 5083fd7d2..f65fb1d9a 100644 --- a/app/views/users/_metadata.html.erb +++ b/app/views/users/_metadata.html.erb @@ -6,7 +6,7 @@ <%= sanitized_sidebar title.titleize %>
- <%= sanitized_sidebar value %> + <%= sanitized_sidebar value %>
<% end %> diff --git a/app/views/users/_profile.html.erb b/app/views/users/_profile.html.erb index 5127afd6a..4ae8ff13c 100644 --- a/app/views/users/_profile.html.erb +++ b/app/views/users/_profile.html.erb @@ -78,9 +78,6 @@ <% 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 %> - <% next if group.name == "Branding" # TODO: [@msarit @jacobherrington] May be further refactored later as part of Profiles work %>