Profile generalization: cleanup (#12947)
* Remove hardcoded instances of education field * WIP Access display_email_on_profile via User Settings * Remove unused profile column * WIP Fix reference to bg_color_hex and text_color_hex * 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 * Match user settings changes * 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 delegation, rename inline concern * Drop profile columns from users table * 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 * Fix formatting issues * Profiles::Update -> Users::Update * Remove RegistrationsController#resolve_profile_field_issues * Fix schema.rb * More cleanup * Fix specs * Fix remaining spec Co-authored-by: Jacob Herrington <jacobherringtondeveloper@gmail.com>
This commit is contained in:
Michael Kohl
GitHub
parent
19db4b0287
commit
08d82e7256
23 changed files with 66 additions and 92 deletions
File diff suppressed because one or more lines are too long
|
Before Width: | Height: | Size: 8.1 KiB After Width: | Height: | Size: 8.1 KiB |
|
|
@ -273,5 +273,5 @@
|
|||
|
||||
.personal-template-button:not(.active),
|
||||
.moderator-template-button:not(.active) {
|
||||
@extend .crayons-btn--outlined;
|
||||
@extend .crayons-btn--outlined;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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])
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
@ -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|
|
||||
|
|
|
|||
|
|
@ -6,7 +6,7 @@
|
|||
<%= sanitized_sidebar title.titleize %>
|
||||
</strong>
|
||||
<p class="crayons-definition__value">
|
||||
<%= sanitized_sidebar value %>
|
||||
<%= sanitized_sidebar value %>
|
||||
</p>
|
||||
</div>
|
||||
<% end %>
|
||||
|
|
|
|||
|
|
@ -78,9 +78,6 @@
|
|||
</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 %>
|
||||
<% next if group.name == "Branding" # TODO: [@msarit @jacobherrington] May be further refactored later as part of Profiles work %>
|
||||
<div class="crayons-card crayons-card--content-rows">
|
||||
<h2><%= group.name %></h2>
|
||||
<% if group.description.present? %>
|
||||
|
|
|
|||
|
|
@ -55,13 +55,13 @@ module.exports = function (api) {
|
|||
'@babel/plugin-proposal-private-property-in-object',
|
||||
{
|
||||
loose: true,
|
||||
}
|
||||
},
|
||||
],
|
||||
[
|
||||
'@babel/plugin-proposal-private-methods',
|
||||
{
|
||||
loose: true,
|
||||
}
|
||||
},
|
||||
],
|
||||
[
|
||||
'@babel/plugin-transform-react-jsx',
|
||||
|
|
|
|||
|
|
@ -0,0 +1,33 @@
|
|||
class RemoveProfileColumnsFromUser < ActiveRecord::Migration[6.0]
|
||||
def change
|
||||
# rubocop:disable Metrics/Blocklength
|
||||
safety_assured do
|
||||
remove_column :users, :available_for
|
||||
remove_column :users, :behance_url
|
||||
remove_column :users, :bg_color_hex
|
||||
remove_column :users, :currently_hacking_on
|
||||
remove_column :users, :currently_learning
|
||||
remove_column :users, :dribbble_url
|
||||
remove_column :users, :education
|
||||
remove_column :users, :email_public
|
||||
remove_column :users, :employer_name
|
||||
remove_column :users, :employer_url
|
||||
remove_column :users, :employment_title
|
||||
remove_column :users, :facebook_url
|
||||
remove_column :users, :gitlab_url
|
||||
remove_column :users, :instagram_url
|
||||
remove_column :users, :linkedin_url
|
||||
remove_column :users, :location
|
||||
remove_column :users, :mastodon_url
|
||||
remove_column :users, :medium_url
|
||||
remove_column :users, :mostly_work_with
|
||||
remove_column :users, :stackoverflow_url
|
||||
remove_column :users, :summary
|
||||
remove_column :users, :text_color_hex
|
||||
remove_column :users, :twitch_url
|
||||
remove_column :users, :website_url
|
||||
remove_column :users, :youtube_url
|
||||
end
|
||||
# rubocop:enable Metrics/Blocklength
|
||||
end
|
||||
end
|
||||
25
db/schema.rb
25
db/schema.rb
|
|
@ -1276,10 +1276,7 @@ ActiveRecord::Schema.define(version: 2021_07_20_042422) do
|
|||
create_table "users", force: :cascade do |t|
|
||||
t.string "apple_username"
|
||||
t.integer "articles_count", default: 0, null: false
|
||||
t.string "available_for"
|
||||
t.integer "badge_achievements_count", default: 0, null: false
|
||||
t.string "behance_url"
|
||||
t.string "bg_color_hex"
|
||||
t.bigint "blocked_by_count", default: 0, null: false
|
||||
t.bigint "blocking_others_count", default: 0, null: false
|
||||
t.boolean "checked_code_of_conduct", default: false
|
||||
|
|
@ -1292,19 +1289,10 @@ ActiveRecord::Schema.define(version: 2021_07_20_042422) do
|
|||
t.integer "credits_count", default: 0, null: false
|
||||
t.datetime "current_sign_in_at"
|
||||
t.inet "current_sign_in_ip"
|
||||
t.string "currently_hacking_on"
|
||||
t.string "currently_learning"
|
||||
t.string "dribbble_url"
|
||||
t.string "education"
|
||||
t.string "email"
|
||||
t.boolean "email_public", default: false
|
||||
t.string "employer_name"
|
||||
t.string "employer_url"
|
||||
t.string "employment_title"
|
||||
t.string "encrypted_password", default: "", null: false
|
||||
t.boolean "export_requested", default: false
|
||||
t.datetime "exported_at"
|
||||
t.string "facebook_url"
|
||||
t.string "facebook_username"
|
||||
t.integer "failed_attempts", default: 0
|
||||
t.datetime "feed_fetched_at", default: "2017-01-01 05:00:00"
|
||||
|
|
@ -1313,8 +1301,6 @@ ActiveRecord::Schema.define(version: 2021_07_20_042422) do
|
|||
t.integer "following_users_count", default: 0, null: false
|
||||
t.datetime "github_repos_updated_at", default: "2017-01-01 05:00:00"
|
||||
t.string "github_username"
|
||||
t.string "gitlab_url"
|
||||
t.string "instagram_url"
|
||||
t.datetime "invitation_accepted_at"
|
||||
t.datetime "invitation_created_at"
|
||||
t.integer "invitation_limit"
|
||||
|
|
@ -1333,13 +1319,8 @@ ActiveRecord::Schema.define(version: 2021_07_20_042422) do
|
|||
t.datetime "last_sign_in_at"
|
||||
t.inet "last_sign_in_ip"
|
||||
t.datetime "latest_article_updated_at"
|
||||
t.string "linkedin_url"
|
||||
t.string "location"
|
||||
t.datetime "locked_at"
|
||||
t.string "mastodon_url"
|
||||
t.string "medium_url"
|
||||
t.integer "monthly_dues", default: 0
|
||||
t.string "mostly_work_with"
|
||||
t.string "name"
|
||||
t.string "old_old_username"
|
||||
t.string "old_username"
|
||||
|
|
@ -1363,21 +1344,15 @@ ActiveRecord::Schema.define(version: 2021_07_20_042422) do
|
|||
t.integer "sign_in_count", default: 0, null: false
|
||||
t.string "signup_cta_variant"
|
||||
t.integer "spent_credits_count", default: 0, null: false
|
||||
t.string "stackoverflow_url"
|
||||
t.string "stripe_id_code"
|
||||
t.integer "subscribed_to_user_subscriptions_count", default: 0, null: false
|
||||
t.text "summary"
|
||||
t.string "text_color_hex"
|
||||
t.string "twitch_url"
|
||||
t.string "twitter_username"
|
||||
t.string "unconfirmed_email"
|
||||
t.string "unlock_token"
|
||||
t.integer "unspent_credits_count", default: 0, null: false
|
||||
t.datetime "updated_at", null: false
|
||||
t.string "username"
|
||||
t.string "website_url"
|
||||
t.datetime "workshop_expiration"
|
||||
t.string "youtube_url"
|
||||
t.index "to_tsvector('simple'::regconfig, COALESCE((name)::text, ''::text))", name: "index_users_on_name_as_tsvector", using: :gin
|
||||
t.index "to_tsvector('simple'::regconfig, COALESCE((username)::text, ''::text))", name: "index_users_on_username_as_tsvector", using: :gin
|
||||
t.index ["apple_username"], name: "index_users_on_apple_username"
|
||||
|
|
|
|||
|
|
@ -56,7 +56,7 @@ import { tagPropTypes } from '../../../components/common-prop-types';
|
|||
|
||||
const SomeComponentUsingTags = ({ tags = [] }) => (
|
||||
<ul>
|
||||
{tags.map(tag => (
|
||||
{tags.map((tag) => (
|
||||
<li key={tag.id}>{tag.name}</li>
|
||||
))}
|
||||
</ul>
|
||||
|
|
|
|||
|
|
@ -290,8 +290,8 @@ There are currently two work-arounds.
|
|||
|
||||
### WSL2 and End to End Tests
|
||||
|
||||
In order to run Cypress Test Runner, a few extra setup steps are required to run GUIs
|
||||
natively inside of WSL2.
|
||||
In order to run Cypress Test Runner, a few extra setup steps are required to run
|
||||
GUIs natively inside of WSL2.
|
||||
|
||||
[This guide](https://nickymeuleman.netlify.app/blog/gui-on-wsl2-cypress) explains
|
||||
how to use graphical user interfaces like Cypress inside WSL2.
|
||||
[This guide](https://nickymeuleman.netlify.app/blog/gui-on-wsl2-cypress)
|
||||
explains how to use graphical user interfaces like Cypress inside WSL2.
|
||||
|
|
|
|||
|
|
@ -4,6 +4,8 @@ title: Self Hosting Forem
|
|||
|
||||
## Hosting your own Forem instance
|
||||
|
||||
Visit [the official Forem Self-Host playbook](https://github.com/forem/selfhost) in order to host on DigitalOcean, AWS, Google Cloud, or bare metal.
|
||||
Visit [the official Forem Self-Host playbook](https://github.com/forem/selfhost)
|
||||
in order to host on DigitalOcean, AWS, Google Cloud, or bare metal.
|
||||
|
||||
If you intend to stand up a Forem, we _strongly_ encourage you to join [forem.dev](https://forem.dev) to take part and keep up with changes. 🌱
|
||||
If you intend to stand up a Forem, we _strongly_ encourage you to join
|
||||
[forem.dev](https://forem.dev) to take part and keep up with changes. 🌱
|
||||
|
|
|
|||
|
|
@ -13,7 +13,7 @@ RSpec.describe NavigationLink, type: :model do
|
|||
it "validates the icon" do
|
||||
navigation_link.icon = "test.png"
|
||||
expect(navigation_link).not_to be_valid
|
||||
|
||||
|
||||
navigation_link.icon = "<svg foo='bar'>"
|
||||
expect(navigation_link).to be_valid
|
||||
|
||||
|
|
|
|||
|
|
@ -99,8 +99,7 @@ RSpec.describe "UserProfiles", type: :request do
|
|||
end
|
||||
|
||||
it "does not render special display header elements naively" do
|
||||
user.profile.location = "hawaii"
|
||||
user.save
|
||||
user.profile.update(location: "hawaii")
|
||||
get "/#{user.username}"
|
||||
# Does not include the word, but does include the SVG
|
||||
expect(response.body).not_to include "<p>Location</p>"
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
require "rails_helper"
|
||||
|
||||
RSpec.describe Profiles::Update, type: :service do
|
||||
RSpec.describe Users::Update, type: :service do
|
||||
def sidekiq_assert_resave_article_worker(user, &block)
|
||||
sidekiq_assert_enqueued_with(
|
||||
job: Users::ResaveArticlesWorker,
|
||||
Loading…
Add table
Reference in a new issue