diff --git a/app/models/concerns/acts_as_profile_field.rb b/app/models/concerns/acts_as_profile_field.rb new file mode 100644 index 000000000..fc165574a --- /dev/null +++ b/app/models/concerns/acts_as_profile_field.rb @@ -0,0 +1,17 @@ +# Used for sharing behavior between ProfileField and CustomProfileField +concern :ActsAsProfileField do + included do + before_create :generate_attribute_name + + WORD_REGEX = /\w+/.freeze + + validates :label, presence: true, uniqueness: { case_sensitive: false } + validates :attribute_name, presence: true, on: :update + end + + private + + def generate_attribute_name + self.attribute_name = label.titleize.scan(WORD_REGEX).join.underscore + end +end diff --git a/app/models/custom_profile_field.rb b/app/models/custom_profile_field.rb new file mode 100644 index 000000000..7968ad70e --- /dev/null +++ b/app/models/custom_profile_field.rb @@ -0,0 +1,16 @@ +class CustomProfileField < ApplicationRecord + include ActsAsProfileField + + belongs_to :profile + + validate :validate_maximum_count + + private + + # We allow a maximum of 5 custom profile fields per user + def validate_maximum_count + return if profile.custom_profile_fields.count < 5 + + errors.add(:profile_id, "maximum number of custom profile fields exceeded") + end +end diff --git a/app/models/profile.rb b/app/models/profile.rb index f9f7b4ab6..524d1f91c 100644 --- a/app/models/profile.rb +++ b/app/models/profile.rb @@ -3,6 +3,10 @@ class Profile < ApplicationRecord validates :user_id, uniqueness: true + has_many :custom_profile_fields, dependent: :destroy + + store_attribute :data, :custom_attributes, :json, default: {} + # NOTE: @citizen428 This is a temporary mapping so we don't break DEV during # profile migration/generalization work. MAPPED_ATTRIBUTES = { @@ -19,13 +23,13 @@ class Profile < ApplicationRecord # Generates typed accessors for all currently defined profile fields. def self.refresh_attributes! ProfileField.find_each do |field| - store_attribute :data, field.attribute_name, field.type + store_attribute :data, field.attribute_name.to_sym, field.type end end # Returns an array of all currently defined `store_attribute`s on `data`. def self.attributes - stored_attributes[:data] || [] + (stored_attributes[:data] || []).map(&:to_s) end # Forces a reload before returning attributes @@ -39,4 +43,8 @@ class Profile < ApplicationRecord def self.mapped_attributes attributes!.map { |attribute| MAPPED_ATTRIBUTES.fetch(attribute, attribute).to_s } end + + def custom_profile_attributes + custom_profile_fields.pluck(:attribute_name) + end end diff --git a/app/models/profile_field.rb b/app/models/profile_field.rb index b5ef58358..665d7509a 100644 --- a/app/models/profile_field.rb +++ b/app/models/profile_field.rb @@ -1,9 +1,5 @@ class ProfileField < ApplicationRecord - self.ignored_columns = ["group"] - - before_create :generate_attribute_name - - WORD_REGEX = /\w+/.freeze + include ActsAsProfileField # Key names follow the Rails form helpers enum input_type: { @@ -20,8 +16,6 @@ class ProfileField < ApplicationRecord belongs_to :profile_field_group, optional: true - validates :label, presence: true, uniqueness: { case_sensitive: false } - validates :attribute_name, presence: true, on: :update validates :show_in_onboarding, inclusion: { in: [true, false] } def type @@ -29,10 +23,4 @@ class ProfileField < ApplicationRecord :string end - - private - - def generate_attribute_name - self.attribute_name = label.titleize.scan(WORD_REGEX).join.underscore - end end diff --git a/app/services/profiles/update.rb b/app/services/profiles/update.rb index 74c472481..7e1a21757 100644 --- a/app/services/profiles/update.rb +++ b/app/services/profiles/update.rb @@ -16,6 +16,10 @@ module Profiles # Ensure we have up to date attributes Profile.refresh_attributes! + # Handle user specific custom profile fields + custom_attributes = @updated_attributes.extract!(*@profile.custom_profile_attributes) + @updated_attributes[:custom_attributes] = custom_attributes + # We don't update `data` directly. This uses the defined store_attributes # so we can make use of their typecasting. @profile.assign_attributes(@updated_attributes) @@ -26,17 +30,23 @@ module Profiles return unless @profile.save # Propagate changes back to the `users` table - user_attributes = @profile.data.transform_keys do |key| - Profile::MAPPED_ATTRIBUTES.fetch(key, key).to_s - end - @profile.user._skip_profile_sync = true - @success = true if @profile.user.update(user_attributes) - @profile.user._skip_profile_sync = false + sync_to_user self end def success? @success end + + private + + def sync_to_user + user_attributes = @profile.data.transform_keys do |key| + Profile::MAPPED_ATTRIBUTES.fetch(key, key).to_s + end + @profile.user._skip_profile_sync = true + @success = true if @profile.user.update(user_attributes.except("custom_attributes")) + @profile.user._skip_profile_sync = false + end end end diff --git a/db/migrate/20200904040009_create_custom_profile_fields.rb b/db/migrate/20200904040009_create_custom_profile_fields.rb new file mode 100644 index 000000000..1d0f27fad --- /dev/null +++ b/db/migrate/20200904040009_create_custom_profile_fields.rb @@ -0,0 +1,18 @@ +class CreateCustomProfileFields < ActiveRecord::Migration[6.0] + def change + safety_assured do + create_table :custom_profile_fields do |t| + t.references :profile, null: false + + t.string "attribute_name", null: false + t.string "description" + t.citext "label", null: false + + t.timestamps + end + + add_foreign_key :custom_profile_fields, :profiles, on_delete: :cascade + add_index :custom_profile_fields, [:label, :profile_id], unique: true + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 4113212f3..9b739972a 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -444,6 +444,17 @@ ActiveRecord::Schema.define(version: 2020_09_04_151734) do t.index ["spent"], name: "index_credits_on_spent" end + create_table "custom_profile_fields", force: :cascade do |t| + t.string "attribute_name", null: false + t.datetime "created_at", precision: 6, null: false + t.string "description" + t.citext "label", null: false + t.bigint "profile_id", null: false + t.datetime "updated_at", precision: 6, null: false + t.index ["label", "profile_id"], name: "index_custom_profile_fields_on_label_and_profile_id", unique: true + t.index ["profile_id"], name: "index_custom_profile_fields_on_profile_id" + end + create_table "data_update_scripts", force: :cascade do |t| t.datetime "created_at", null: false t.string "file_name" @@ -1388,6 +1399,7 @@ ActiveRecord::Schema.define(version: 2020_09_04_151734) do add_foreign_key "comments", "users", on_delete: :cascade add_foreign_key "credits", "organizations", on_delete: :restrict add_foreign_key "credits", "users", on_delete: :cascade + add_foreign_key "custom_profile_fields", "profiles", on_delete: :cascade add_foreign_key "display_ad_events", "display_ads", on_delete: :cascade add_foreign_key "display_ad_events", "users", on_delete: :cascade add_foreign_key "display_ads", "organizations", on_delete: :cascade diff --git a/spec/factories/custom_profile_fields.rb b/spec/factories/custom_profile_fields.rb new file mode 100644 index 000000000..31ebaf75a --- /dev/null +++ b/spec/factories/custom_profile_fields.rb @@ -0,0 +1,7 @@ +FactoryBot.define do + factory :custom_profile_field do + profile + sequence(:label) { |n| "Email #{n}" } + description { "some description" } + end +end diff --git a/spec/models/custom_profile_field_spec.rb b/spec/models/custom_profile_field_spec.rb new file mode 100644 index 000000000..5adc12e27 --- /dev/null +++ b/spec/models/custom_profile_field_spec.rb @@ -0,0 +1,19 @@ +require "rails_helper" + +RSpec.describe CustomProfileField, type: :model do + it_behaves_like "a profile field" + + describe "#validate_maximum_count" do + it "validates that user's can't have more than 3 custom profile fields" do + profile = create(:profile) + create_list(:custom_profile_field, 5, profile: profile) + + custom_profile_field = build(:custom_profile_field, profile: profile) + custom_profile_field.save + + expect(custom_profile_field).not_to be_valid + expect(custom_profile_field.errors[:profile_id]) + .to include("maximum number of custom profile fields exceeded") + end + end +end diff --git a/spec/models/profile_field_spec.rb b/spec/models/profile_field_spec.rb index a7600945a..97e753445 100644 --- a/spec/models/profile_field_spec.rb +++ b/spec/models/profile_field_spec.rb @@ -1,24 +1,12 @@ require "rails_helper" RSpec.describe ProfileField, type: :model do - let(:profile_field) { create(:profile_field) } + it_behaves_like "a profile field" describe "validations" do describe "builtin validations" do - subject { profile_field } - it { is_expected.to belong_to(:profile_field_group).optional(true) } - it { is_expected.to validate_presence_of(:label) } - it { is_expected.to validate_uniqueness_of(:label).case_insensitive } - it { is_expected.to validate_presence_of(:attribute_name).on(:update) } it { is_expected.to validate_inclusion_of(:show_in_onboarding).in_array([true, false]) } end end - - describe "callbacks" do - it "automatically generates an attribute name" do - pf = create(:profile_field, label: "Is this a test? This is a test! 1") - expect(pf.attribute_name).to eq "is_this_a_test_this_is_a_test1" - end - end end diff --git a/spec/models/shared_examples/a_profile_field.rb b/spec/models/shared_examples/a_profile_field.rb new file mode 100644 index 000000000..2d8f2ae9d --- /dev/null +++ b/spec/models/shared_examples/a_profile_field.rb @@ -0,0 +1,20 @@ +RSpec.shared_examples "a profile field" do + let(:profile_field_class) { described_class.name.underscore } + + describe "validations" do + describe "builtin validations" do + subject { create(profile_field_class) } + + it { is_expected.to validate_presence_of(:label) } + it { is_expected.to validate_uniqueness_of(:label).case_insensitive } + it { is_expected.to validate_presence_of(:attribute_name).on(:update) } + end + end + + describe "callbacks" do + it "automatically generates an attribute name" do + field = create(profile_field_class, label: "Test? Test! 1") + expect(field.attribute_name).to eq "test_test1" + end + end +end diff --git a/spec/services/profiles/update_spec.rb b/spec/services/profiles/update_spec.rb index 747dcb224..e8d00df53 100644 --- a/spec/services/profiles/update_spec.rb +++ b/spec/services/profiles/update_spec.rb @@ -5,19 +5,12 @@ RSpec.describe Profiles::Update, type: :service do create(:profile, data: { name: "Sloan Doe", looking_for_work: true, removed: "Bla" }) end - # rubocop:disable RSpec/BeforeAfterAll - before(:all) do + before do create(:profile_field, label: "Name", input_type: :text_field) create(:profile_field, label: "Looking for work", input_type: :check_box) Profile.refresh_attributes! end - after(:all) do - ProfileField.destroy_all - Profile.refresh_attributes! - end - # rubocop:enable RSpec/BeforeAfterAll - it "correctly typecasts new attributes", :aggregate_failures do described_class.call(profile, name: 123, looking_for_work: "false") expect(profile.name).to eq "123" @@ -36,4 +29,13 @@ RSpec.describe Profiles::Update, type: :service do expect(profile.name).to eq new_name expect(profile.user[:name]).to eq new_name end + + it "sets custom attributes for the user" do + custom_profile_field = create(:custom_profile_field, profile: profile) + custom_attribute = custom_profile_field.attribute_name + + described_class.call(profile, custom_attribute => "Test") + + expect(profile.custom_attributes[custom_attribute]).to eq "Test" + end end