diff --git a/.travis.yml b/.travis.yml index c97d0669d..c6d8f28c8 100644 --- a/.travis.yml +++ b/.travis.yml @@ -26,7 +26,6 @@ addons: env: global: - RAILS_ENV=test - - RUBY_THREAD_VM_STACK_SIZE=5242880 - DATABASE_URL=postgres://travis@localhost:5433/Forem_prod_test - DATABASE_NAME=Forem_prod_test - DATABASE_URL_TEST=postgres://travis@localhost:5433/Forem_test diff --git a/app/models/profile.rb b/app/models/profile.rb index 846153637..9e0cf9bb4 100644 --- a/app/models/profile.rb +++ b/app/models/profile.rb @@ -6,28 +6,23 @@ class Profile < ApplicationRecord validates :website_url, url: { allow_blank: true, no_local: true, schemes: %w[https http] } validates_with ProfileValidator + ATTRIBUTE_NAME_REGEX = /(?\w+)=?/ + CACHE_KEY = "profile/attributes".freeze # Static fields are columns on the profiles table; they have no relationship # to a ProfileField record. These are columns we can safely assume exist for # any profile on a given Forem. STATIC_FIELDS = %w[summary location website_url].freeze - # Generates typed accessors for all currently defined profile fields. + # Update the Rails cache with the currently available attributes. def self.refresh_attributes! - return if ENV["ENV_AVAILABLE"] == "false" - return unless table_exists? - - ProfileField.find_each do |field| - store_attribute :data, field.attribute_name.to_sym, field.type - end + Rails.cache.delete(CACHE_KEY) + attributes end - # Set up all profile attributes when this class loads so all store_attribute - # accessors get defined immediately. - refresh_attributes! - - # Returns an array of all currently defined `store_attribute`s on `data`. def self.attributes - (stored_attributes[:data] || []).map(&:to_s) + Rails.cache.fetch(CACHE_KEY, expires_in: 24.hours) do + ProfileField.pluck(:attribute_name) + end end def self.static_fields @@ -37,4 +32,26 @@ class Profile < ApplicationRecord def clear! update(data: {}) end + + # Lazily add accessors for profile fields on first use + def method_missing(method_name, *args, **kwargs, &block) + match = method_name.match(ATTRIBUTE_NAME_REGEX) + field = ProfileField.find_by(attribute_name: match[:attribute_name]) + super unless field + + self.class.instance_eval do + store_attribute :data, field.attribute_name.to_sym, field.type + end + public_send(method_name, *args, **kwargs, &block) + end + + # Defining this is not only a good practice in general, it's also necessary + # for `update` to work since the `_assign_attribute` helper it uses performs + # an explicit `responds_to?` check. + def respond_to_missing?(method_name, include_private = false) + match = method_name.match(ATTRIBUTE_NAME_REGEX) + return true if match[:attribute_name].in?(self.class.attributes) + + super + end end diff --git a/app/services/profile_fields/remove.rb b/app/services/profile_fields/remove.rb index 81d1691be..85daf2e93 100644 --- a/app/services/profile_fields/remove.rb +++ b/app/services/profile_fields/remove.rb @@ -16,8 +16,8 @@ module ProfileFields 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.attributes) + remove_store_attributes + Profile.refresh_attributes! @success = true else @error_message = @profile_field.errors_as_sentence @@ -28,5 +28,16 @@ module ProfileFields def success? @success end + + private + + def remove_store_attributes + accessor = @profile_field.attribute_name.to_sym + store_attributes = Profile.stored_attributes.fetch(:data) { [] } + return unless accessor.in?(store_attributes) + + Profile.undef_method(accessor) + Profile.undef_method("#{accessor}=") + end end end diff --git a/spec/models/profile_spec.rb b/spec/models/profile_spec.rb index e99de19a7..788b307c4 100644 --- a/spec/models/profile_spec.rb +++ b/spec/models/profile_spec.rb @@ -85,7 +85,6 @@ RSpec.describe Profile, type: :model do before do create(:profile_field, label: "Test 1") create(:profile_field, label: "Test 2", input_type: :check_box) - described_class.refresh_attributes! 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 4d06a72ac..d4abc8a8f 100644 --- a/spec/requests/admin/profile_fields_spec.rb +++ b/spec/requests/admin/profile_fields_spec.rb @@ -75,7 +75,7 @@ RSpec.describe "/admin/customization/profile_fields", type: :request do describe "DELETE /admin/profile_fields/:id" do let!(:profile_field) do - create(:profile_field).tap { Profile.refresh_attributes! } + create(:profile_field) end it "redirects successfully" do diff --git a/spec/services/profile_fields/remove_spec.rb b/spec/services/profile_fields/remove_spec.rb index 6d621917f..0ce770b12 100644 --- a/spec/services/profile_fields/remove_spec.rb +++ b/spec/services/profile_fields/remove_spec.rb @@ -4,24 +4,20 @@ 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_attributes! 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 + expect { described_class.call(profile_field.id) } + .to change(ProfileField, :count).by(-1) + .and change { profile.respond_to?(:removed_field) }.from(true).to(false) + .and change { profile.respond_to?(:removed_field=) }.from(true).to(false) end it "returns the correct response object", :aggregate_failures do profile_field = create(:profile_field, label: "Another Removed field") - Profile.refresh_attributes! - 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 + result = described_class.call(profile_field.id) + expect(result.success?).to be true + expect(result.error_message).to be_blank end end @@ -38,10 +34,10 @@ RSpec.describe ProfileFields::Remove, type: :service do 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" + result = described_class.call(id) + expect(result.success?).to be false + expect(result.profile_field).to be_present + expect(result.error_message).to eq "Something went wrong" end end end