diff --git a/app/controllers/admin/profile_fields_controller.rb b/app/controllers/admin/profile_fields_controller.rb index 1db2df7c5..05705c043 100644 --- a/app/controllers/admin/profile_fields_controller.rb +++ b/app/controllers/admin/profile_fields_controller.rb @@ -20,21 +20,23 @@ module Admin end def create - profile_field = ProfileField.new(profile_field_params) - if profile_field.save + add_result = ProfileFields::Add.call(profile_field_params) + if add_result.success? + profile_field = add_result.profile_field flash[:success] = "Profile field #{profile_field.label} created" else - flash[:error] = "Error: #{profile_field.errors_as_sentence}" + flash[:error] = "Error: #{add_result.error_message}" end redirect_to admin_profile_fields_path end def destroy - profile_field = ProfileField.find(params[:id]) - if profile_field.destroy + remove_result = ProfileFields::Remove.call(params[:id]) + if remove_result.success? + profile_field = remove_result.profile_field flash[:success] = "Profile field #{profile_field.label} deleted" else - flash[:error] = "Error: #{profile_field.errors_as_sentence}" + flash[:error] = "Error: #{remove_result.error_message}" end redirect_to admin_profile_fields_path end diff --git a/app/models/profile.rb b/app/models/profile.rb index 505c87c2f..c34e8a5b7 100644 --- a/app/models/profile.rb +++ b/app/models/profile.rb @@ -2,13 +2,13 @@ class Profile < ApplicationRecord belongs_to :user # This method generates typed accessors for all active profile fields - def self.define_store_accessors! + def self.refresh_store_accessors! ProfileField.active.find_each do |field| store_attribute :data, field.attribute_name, field.type end end - define_store_accessors! + refresh_store_accessors! validates :data, presence: true end diff --git a/app/services/profile_fields/add.rb b/app/services/profile_fields/add.rb new file mode 100644 index 000000000..eaa3dd3fa --- /dev/null +++ b/app/services/profile_fields/add.rb @@ -0,0 +1,31 @@ +# This service creates a new profile field and ensures that the correct store +# accessor gets added to profiles immediately. +module ProfileFields + class Add + def self.call(attributes) + new(attributes).call + end + + attr_reader :profile_field, :error_message + + def initialize(attributes) + @attributes = attributes + @success = false + end + + def call + @profile_field = ProfileField.create(@attributes) + if @profile_field.persisted? + @success = true + Profile.refresh_store_accessors! + else + @error_message = @profile_field.errors_as_sentence + end + self + end + + def success? + @success + end + end +end diff --git a/app/services/profile_fields/remove.rb b/app/services/profile_fields/remove.rb new file mode 100644 index 000000000..a4172b75c --- /dev/null +++ b/app/services/profile_fields/remove.rb @@ -0,0 +1,32 @@ +# This service removes a profile field and ensures that the corresponding store +# accessor also gets removed immediately. +module ProfileFields + class Remove + def self.call(id) + new(id).call + end + + attr_reader :profile_field, :error_message + + def initialize(id) + @id = id + @success = false + end + + def call + @profile_field = ProfileField.find(@id) + if @profile_field.destroy + accessor = profile_field.attribute_name.to_s + Profile.undef_method(accessor) if accessor.in?(Profile.stored_attributes[:data]) + @success = true + else + @error_message = @profile_field.errors_as_sentence + end + self + end + + def success? + @success + end + end +end diff --git a/spec/models/profile_spec.rb b/spec/models/profile_spec.rb index 3353efc7a..0cce155ca 100644 --- a/spec/models/profile_spec.rb +++ b/spec/models/profile_spec.rb @@ -8,7 +8,7 @@ RSpec.describe Profile, type: :model do create(:profile_field, label: "Test 1") create(:profile_field, label: "Test 2", input_type: :check_box) create(:profile_field, label: "Test 3", active: false) - described_class.define_store_accessors! + described_class.refresh_store_accessors! end let(:profile) { described_class.new } diff --git a/spec/requests/admin/profile_fields_spec.rb b/spec/requests/admin/profile_fields_spec.rb index 995b25a51..5a35394f8 100644 --- a/spec/requests/admin/profile_fields_spec.rb +++ b/spec/requests/admin/profile_fields_spec.rb @@ -21,7 +21,7 @@ RSpec.describe "/admin/profile_fields", type: :request do profile_field.input_type, profile_field.description, profile_field.placeholder_text, - profile_field.group + profile_field.group, ) end end @@ -77,7 +77,9 @@ RSpec.describe "/admin/profile_fields", type: :request do end describe "DELETE /admin/profile_fields/:id" do - let(:profile_field) { create(:profile_field) } + let(:profile_field) do + create(:profile_field).tap { Profile.refresh_store_accessors! } + end it "redirects successfully" do delete "#{admin_profile_fields_path}/#{profile_field.id}" diff --git a/spec/services/profile_fields/add_spec.rb b/spec/services/profile_fields/add_spec.rb new file mode 100644 index 000000000..2551eacc7 --- /dev/null +++ b/spec/services/profile_fields/add_spec.rb @@ -0,0 +1,35 @@ +require "rails_helper" + +RSpec.describe ProfileFields::Add, type: :service do + let(:profile) { create(:user).profile } + + context "when successfully adding a new profile field" do + it "creates a new profile field and adds a store accessor", :aggregate_failures do + expect(profile.respond_to?(:new_field)).to be false + expect do + described_class.call(label: "New Field") + end.to change(ProfileField, :count).by(1) + expect(profile.respond_to?(:new_field)).to be true + end + + it "returns the correct response object", :aggregate_failures do + add_response = described_class.call(label: "Another New Field") + expect(add_response.success?).to be true + expect(add_response.profile_field).to be_an_instance_of(ProfileField) + expect(add_response.error_message).to be_blank + end + end + + context "when profile field creation fails" do + it "does not create a new profile field or store accessor" do + expect { described_class.call({}) }.not_to change(ProfileField, :count) + end + + it "returns the correct response object", :aggregate_failures do + add_response = described_class.call({}) + expect(add_response.success?).to be false + expect(add_response.profile_field).to be_an_instance_of(ProfileField) + expect(add_response.error_message).to eq "Label can't be blank" + end + end +end diff --git a/spec/services/profile_fields/remove_spec.rb b/spec/services/profile_fields/remove_spec.rb new file mode 100644 index 000000000..4e4d75726 --- /dev/null +++ b/spec/services/profile_fields/remove_spec.rb @@ -0,0 +1,47 @@ +require "rails_helper" + +RSpec.describe ProfileFields::Remove, type: :service do + context "when successfully removing a profile field" do + it "removes the profile field and store accessor", :aggregate_failures do + profile_field = create(:profile_field, label: "Removed field") + Profile.refresh_store_accessors! + profile = create(:user).profile + + expect(profile.respond_to?(:removed_field)).to be true + expect do + described_class.call(profile_field.id) + end.to change(ProfileField, :count).by(-1) + expect(profile.respond_to?(:removed_field)).to be false + end + + it "returns the correct response object", :aggregate_failures do + profile_field = create(:profile_field, label: "Another Removed field") + Profile.refresh_store_accessors! + + add_response = described_class.call(profile_field.id) + expect(add_response.success?).to be true + expect(add_response.profile_field).to be_an_instance_of(ProfileField) + expect(add_response.error_message).to be_blank + end + end + + context "when profile field removal fails" do + let(:id) { 428 } + + before do + profile_field = instance_double("ProfileField", destroy: false, errors_as_sentence: "Something went wrong") + allow(ProfileField).to receive(:find).with(id).and_return(profile_field) + end + + it "does not remove a profile field" do + expect { described_class.call(id) }.not_to change(ProfileField, :count) + end + + it "returns the correct response object", :aggregate_failures do + add_response = described_class.call(id) + expect(add_response.success?).to be false + expect(add_response.profile_field).to be_present + expect(add_response.error_message).to eq "Something went wrong" + end + end +end