From 46ba198c7d0f037219361ca664ada5eb31291fe3 Mon Sep 17 00:00:00 2001 From: Simon Isler Date: Fri, 15 Nov 2024 11:45:42 +0100 Subject: [PATCH] Refactor editable attributes (#33) --- app/controllers/hotsheet/pages_controller.rb | 2 +- app/views/hotsheet/pages/index.html.erb | 4 +- .../hotsheet/editable_attributes.rb | 43 ---------- lib/hotsheet.rb | 1 + lib/hotsheet/configuration.rb | 9 +- lib/hotsheet/editable_attributes.rb | 24 ++++++ spec/dummy/config/initializers/hotsheet.rb | 2 +- spec/lib/editable_attributes_spec.rb | 82 +++++++++++++++++++ spec/spec_helper.rb | 5 +- spec/support/capybara_utils.rb | 9 ++ spec/system/editable_attributes_spec.rb | 8 ++ 11 files changed, 136 insertions(+), 53 deletions(-) delete mode 100644 config/initializers/hotsheet/editable_attributes.rb create mode 100644 lib/hotsheet/editable_attributes.rb create mode 100644 spec/lib/editable_attributes_spec.rb create mode 100644 spec/support/capybara_utils.rb diff --git a/app/controllers/hotsheet/pages_controller.rb b/app/controllers/hotsheet/pages_controller.rb index 42b0813..e849ee3 100644 --- a/app/controllers/hotsheet/pages_controller.rb +++ b/app/controllers/hotsheet/pages_controller.rb @@ -33,7 +33,7 @@ def broadcast_params end def model_params - params.require(model.name.underscore).permit(*model.editable_attributes) + params.require(model.name.underscore).permit(*Hotsheet.editable_attributes_for(model: model)) end def model diff --git a/app/views/hotsheet/pages/index.html.erb b/app/views/hotsheet/pages/index.html.erb index 4c0bffd..7494657 100644 --- a/app/views/hotsheet/pages/index.html.erb +++ b/app/views/hotsheet/pages/index.html.erb @@ -6,7 +6,7 @@ - <% @model.editable_attributes.each do |attribute| %> + <% Hotsheet.editable_attributes_for(model: @model).each do |attribute| %> <% end %> @@ -14,7 +14,7 @@ <% @records.each do |record| %> - <% @model.editable_attributes.each do |attribute| %> + <% Hotsheet.editable_attributes_for(model: @model).each do |attribute| %> diff --git a/config/initializers/hotsheet/editable_attributes.rb b/config/initializers/hotsheet/editable_attributes.rb deleted file mode 100644 index da6afdb..0000000 --- a/config/initializers/hotsheet/editable_attributes.rb +++ /dev/null @@ -1,43 +0,0 @@ -# frozen_string_literal: true - -Rails.application.config.after_initialize do # rubocop:disable Metrics/BlockLength - # Only run this initializer when running the Rails server - next unless Rails.env.test? || defined? Rails::Server - - Hotsheet.configuration.models.each_key do |model| # rubocop:disable Metrics/BlockLength - model.constantize.class_eval do - class << self - def editable_attributes - @editable_attributes ||= fetch_editable_attributes - end - - private - - def fetch_editable_attributes - config = Hotsheet.configuration.models[name] - excluded_attributes = config.excluded_attributes - - if config.included_attributes.present? - raise "Can only specify either included or excluded attributes" if excluded_attributes.present? - - attrs_to_s(config.included_attributes) - elsif excluded_attributes.present? - column_names - attrs_to_s(excluded_attributes) - else - column_names - end - end - - def attrs_to_s(attrs) - attrs.map do |attr| - raise "Attribute '#{attr}' doesn't exist on model '#{name}'" if column_names.exclude? attr.to_s - - attr.to_s - end - end - end - end - - model.constantize.editable_attributes - end -end diff --git a/lib/hotsheet.rb b/lib/hotsheet.rb index 3a58651..4c48e81 100644 --- a/lib/hotsheet.rb +++ b/lib/hotsheet.rb @@ -6,6 +6,7 @@ require "hotsheet/engine" require "hotsheet/version" require "hotsheet/configuration" +require "hotsheet/editable_attributes" module Hotsheet class Error < StandardError; end diff --git a/lib/hotsheet/configuration.rb b/lib/hotsheet/configuration.rb index 6c8c9fa..895e12c 100644 --- a/lib/hotsheet/configuration.rb +++ b/lib/hotsheet/configuration.rb @@ -12,7 +12,7 @@ def initialize def model(name) model_config = ModelConfig.new(name).tap do |config| - yield(config) + yield(config) if block_given? Rails.application.config.to_prepare { config.validate! } end models[name.to_s] = model_config @@ -23,12 +23,11 @@ class ModelConfig def initialize(model_name) @model_name = model_name - @included_attributes = [] - @excluded_attributes = [] end def validate! return unless ActiveRecord::Base.connection.table_exists?(model_class.table_name) + return if included_attributes.nil? && excluded_attributes.nil? ensure_only_one_attribute_set validate_attribute_existence @@ -37,7 +36,7 @@ def validate! private def ensure_only_one_attribute_set - return unless included_attributes.any? && excluded_attributes.any? + return if included_attributes.blank? || excluded_attributes.blank? raise "Can only specify either included or excluded attributes for '#{model_name}'" end @@ -45,7 +44,7 @@ def ensure_only_one_attribute_set def validate_attribute_existence all_attributes = model_class.column_names - (included_attributes + excluded_attributes).each do |attr| + [included_attributes, excluded_attributes].flatten.compact.each do |attr| unless all_attributes.include?(attr.to_s) raise "Attribute '#{attr}' doesn't exist on model '#{model_name}'" end diff --git a/lib/hotsheet/editable_attributes.rb b/lib/hotsheet/editable_attributes.rb new file mode 100644 index 0000000..db60f06 --- /dev/null +++ b/lib/hotsheet/editable_attributes.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +module Hotsheet + class << self + def editable_attributes_for(model:) + @editable_attributes_for ||= {} + @editable_attributes_for[model] ||= fetch_editable_attributes(model) + end + + private + + def fetch_editable_attributes(model) + model_config = Hotsheet.configuration.models[model.to_s] + + if model_config&.included_attributes + model_config.included_attributes.map(&:to_s) + elsif model_config&.excluded_attributes + model.column_names - model_config.excluded_attributes.map(&:to_s) + else + model.column_names + end + end + end +end diff --git a/spec/dummy/config/initializers/hotsheet.rb b/spec/dummy/config/initializers/hotsheet.rb index d6ed110..fd67bb2 100644 --- a/spec/dummy/config/initializers/hotsheet.rb +++ b/spec/dummy/config/initializers/hotsheet.rb @@ -10,7 +10,7 @@ end config.model :TableNameTest do |model| - model.included_attributes = [] + model.included_attributes = %i[] end config.model :VeryLongModelNameForOverflowTest do diff --git a/spec/lib/editable_attributes_spec.rb b/spec/lib/editable_attributes_spec.rb new file mode 100644 index 0000000..4939041 --- /dev/null +++ b/spec/lib/editable_attributes_spec.rb @@ -0,0 +1,82 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe "Hotsheet" do + describe ".editable_attributes_for" do + subject(:editable_attributes) { Hotsheet.editable_attributes_for(model: Author) } + + before do + Hotsheet.instance_variable_set(:@editable_attributes_for, nil) # reset the memoized editable attributes + end + + context "when included attributes are specified" do + before do + Hotsheet.configure do |config| + config.model :Author do |model| + model.included_attributes = %i[name birthdate] + end + end + end + + it "returns only the included attributes" do + expect(editable_attributes).to eq %w[name birthdate] + end + end + + context "when excluded attributes are specified" do + before do + Hotsheet.configure do |config| + config.model :Author do |model| + model.excluded_attributes = %i[gender created_at updated_at] + end + end + end + + it "returns all attributes except the excluded ones" do + expect(editable_attributes).to eq %w[id name birthdate] + end + end + + context "when included attributes are empty" do + before do + Hotsheet.configure do |config| + config.model :Author do |model| + model.included_attributes = %i[] + end + end + end + + it "returns an empty list" do + expect(editable_attributes).to eq %w[] + end + end + + context "when excluded attributes are empty" do + before do + Hotsheet.configure do |config| + config.model :Author do |model| + model.excluded_attributes = %i[] + end + end + end + + it "returns all attributes" do + expect(editable_attributes).to eq Author.column_names + end + end + + context "when no included or excluded attributes are specified" do + before do + Hotsheet.configure do |config| + config.model :Author + end + end + + it "returns all attributes of the model" do + expect(Hotsheet.configuration.models["Author"]).to be_a Hotsheet::Configuration::ModelConfig + expect(editable_attributes).to eq %w[id name birthdate gender created_at updated_at] + end + end + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index ed492d2..677853d 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -7,6 +7,8 @@ require "rspec/rails" require "selenium-webdriver" +require "support/capybara_utils" + %i[firefox firefox_headless].each do |driver| Capybara.register_driver driver do |app| args = ["--headless"] if driver == :firefox_headless @@ -23,7 +25,8 @@ RSpec.configure do |config| Kernel.srand config.seed - config.include Capybara::DSL + config.include Capybara::DSL, type: :system + config.include CapybaraUtils, type: :system config.disable_monkey_patching! config.filter_rails_from_backtrace! diff --git a/spec/support/capybara_utils.rb b/spec/support/capybara_utils.rb new file mode 100644 index 0000000..159eb4d --- /dev/null +++ b/spec/support/capybara_utils.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +module CapybaraUtils + def wait_for_turbo(timeout = nil) + return unless has_css?(".turbo-progress-bar", visible: true, wait: 1.second) + + has_no_css?(".turbo-progress-bar", wait: timeout.presence || 5.seconds) + end +end diff --git a/spec/system/editable_attributes_spec.rb b/spec/system/editable_attributes_spec.rb index 897cc1c..68b1d8e 100644 --- a/spec/system/editable_attributes_spec.rb +++ b/spec/system/editable_attributes_spec.rb @@ -7,8 +7,16 @@ describe "update strings" do before do + Hotsheet.configure do |config| + config.model :Author do |model| + model.included_attributes = %i[name birthdate] + end + end + Hotsheet.instance_variable_set(:@editable_attributes_for, nil) # reset the memoized editable attributes + visit "/hotsheet" click_link "Author" + find(".readonly-attribute", text: "Stephen").click fill_in "author_name", with: "King" end
<%= attribute %>
<%= render "editable_attribute", model: @model, record: record, attribute: attribute %>