Refactor rows

Author: hunchr

app/controllers/hotsheet/sheets_controller.rb

@@ -2,18 +2,20 @@
 
 class Hotsheet::SheetsController < Hotsheet::ApplicationController
   before_action :set_sheet
+  before_action :set_row, only: :update
 
   def index
     @rows = @sheet.rows
-    @cells = @sheet.model.pluck(*@rows.pluck(:name))
+    @cells = @sheet.model.pluck(*@rows.map(&:name))
     @cells = @rows.length == 1 ? [@cells] : @cells.transpose
   end
 
-  def update # rubocop:disable Metrics/AbcSize
-    @sheet.update! id: params[:id], attr: params[:attr]&.to_sym, value: params[:value]
-    respond_to { |format| format.any { render json: { error: false } } }
+  def update
+    @row&.update! params[:id], params[:to]
+    json = {}
   rescue Hotsheet::Error => e
-    json = { error: e.message, value: params[:prev], x: params[:x], y: params[:y] }
+    json = { error: e.message, value: params[:from], x: params[:x], y: params[:y] }
+  ensure
     respond_to { |format| format.any { render json: } }
   end
 
@@ -26,4 +28,8 @@ def error
   def set_sheet
     @sheet = Hotsheet.sheets[params[:sheet_name]]
   end
+
+  def set_row
+    @row = @sheet.rows.find { |row| row.name == params[:row_name] }
+  end
 end

app/javascripts/controllers/sheets_controller.ts

@@ -14,7 +14,7 @@ export class SheetsController extends Controller {
 
   private x = 0
   private y = 0
-  private attrs: string[] = []
+  private rowNames: string[] = []
   private input = document.createElement("input")
   private cell = this.input as HTMLElement
   private cells: HTMLElement[][] = []
@@ -26,9 +26,9 @@ export class SheetsController extends Controller {
     const sheet = window.location.pathname
     const id = this.cells[this.y][0].innerHTML
     const params = new URLSearchParams({
-      attr: this.attrs[this.x],
-      value: this.input.value,
-      prev: this.value,
+      row_name: this.rowNames[this.x],
+      from: this.value,
+      to: this.input.value,
     })
 
     fetch(`${sheet}/${id}?${params}&x=${this.x}&y=${this.y}`, {
@@ -132,7 +132,7 @@ export class SheetsController extends Controller {
       cell.addEventListener("keydown", this.moveFocus)
     })
 
-    this.attrs = [...this.element.querySelectorAll<HTMLElement>(".header")].map(
+    this.rowNames = [...this.element.querySelectorAll<HTMLElement>(".header")].map(
       (header) => header.dataset.name!,
     )
   }

app/views/hotsheet/sheets/index.html.erb

@@ -1,12 +1,12 @@
 <div class="table" spellcheck="false" data-controller="sheets">
-  <% @cells.each_with_index do |rows, x_index| %>
+  <% @cells.each_with_index do |values, x_index| %>
     <% row = @rows[x_index] %>
-    <% readonly_class = row[:editable] ? "" : " readonly" %>
+    <% readonly_class = row.editable? ? "" : " readonly" %>
 
     <div class="row">
-      <span class="cell header" data-name="<%= row[:name] %>"><%= row[:label] %></span>
+      <span class="cell header" data-name="<%= row.name %>"><%= row.human_name %></span>
 
-      <% rows.each_with_index do |value, y_index| %>
+      <% values.each_with_index do |value, y_index| %>
         <span class="cell<%= readonly_class %>" tabindex="0"
               data-xy="<%= "#{x_index},#{y_index}" %>"><%= value %></span>
       <% end %>

lib/hotsheet.rb

@@ -3,6 +3,7 @@
 require "hotsheet/config"
 require "hotsheet/engine"
 require "hotsheet/sheet"
+require "hotsheet/sheet/row"
 require "hotsheet/version"
 
 module Hotsheet

lib/hotsheet/config.rb

@@ -7,13 +7,9 @@ def initialize
     @sheets = {}
   end
 
-  def sheet(model_name, &config)
-    sheet = Hotsheet::Sheet.new(model_name).tap do |s|
-      if config
-        s.instance_eval(&config)
-      else
-        s.model.column_names.each { |column_name| s.row column_name }
-      end
+  def sheet(name, config = {}, &rows)
+    sheet = Hotsheet::Sheet.new(name, config).tap do |s|
+      rows ? s.instance_eval(&rows) : s.use_default_configuration
     end
 
     @sheets[sheet.model.table_name] = sheet

lib/hotsheet/sheet.rb

@@ -1,64 +1,39 @@
 # frozen_string_literal: true
 
 class Hotsheet::Sheet
-  DEFAULT_CONFIG = { editable: true, visible: true }.freeze
-
   attr_accessor :model
 
-  def initialize(model_name)
-    error "Unknown model '#{model_name}'" unless Object.const_defined? model_name
+  def initialize(name, config = {})
+    ensure_model_exists! name
 
-    @model = model_name.to_s.constantize
+    @config = config
+    @model = name.to_s.constantize
     @rows = []
   end
 
-  def row(name, **config)
-    @config = config
-
-    validate_column_name! name
-    validate_config! name
-
-    @rows << @config.merge(name: name.to_sym)
+  def human_name
+    @model.model_name.human count: 2
   end
 
-  def rows
-    @rows.filter_map do |row|
-      next unless row[:visible]
-
-      row.merge label: @model.human_attribute_name(row[:name]), editable: row[:editable]
-    end
+  def row(name, config = {})
+    row = Row.new(@model, name, config)
+    row.validate!
+    @rows << row
   end
 
-  def human_name
-    @model.model_name.human count: 2
+  def rows
+    @rows.select(&:visible?)
   end
 
-  def update!(id:, attr:, value:)
-    row = @rows.find { |r| r[:name] == attr }
-    error "Forbidden" unless row && row[:visible] && row[:editable]
-
-    @model.find(id).update! attr => value
-  rescue StandardError => e
-    error e.message
+  def use_default_configuration
+    @model.column_names.each { |name| row name }
   end
 
   private
 
-  def validate_column_name!(name)
-    return if @model.column_names.include? name.to_s
-
-    error "Unknown database column '#{name}' for '#{@model.table_name}'"
-  end
-
-  def validate_config!(name)
-    @config.each do |key, value|
-      error "Unknown config '#{key}' for row '#{name}'" unless DEFAULT_CONFIG.key? key
-      error "Invalid config '#{key}' for row '#{name}'" unless value == false
-    end
-    @config = DEFAULT_CONFIG.merge @config
-  end
+  def ensure_model_exists!(name)
+    return if Object.const_defined? name
 
-  def error(message)
-    raise Hotsheet::Error, message
+    raise Hotsheet::Error, "Unknown model '#{name}'"
   end
 end

lib/hotsheet/sheet/row.rb

@@ -0,0 +1,62 @@
+# frozen_string_literal: true
+
+class Hotsheet::Sheet::Row
+  attr_accessor :name
+
+  DEFAULT_CONFIG = { editable: true, visible: true }.freeze
+
+  def initialize(model, name, config)
+    @model = model
+    @name = name.to_s
+    @config = DEFAULT_CONFIG.merge config
+  end
+
+  def human_name
+    @model.human_attribute_name @name
+  end
+
+  def visible?
+    @config[:visible]
+  end
+
+  def editable?
+    @config[:editable]
+  end
+
+  def update!(id, value)
+    raise Hotsheet::Error, "Forbidden" unless editable?
+
+    model = @model.find_by(id:)
+    raise Hotsheet::Error, "Not found" if model.nil?
+    raise Hotsheet::Error, model.errors.full_messages.first unless model.update @name => value
+  end
+
+  def validate!
+    ensure_database_column_exists!
+
+    @config.each do |key, value|
+      ensure_config_exists! key
+      validate_config_value! key, value
+    end
+  end
+
+  private
+
+  def ensure_database_column_exists!
+    return if @model.column_names.include? @name.to_s
+
+    raise Hotsheet::Error, "Unknown database column '#{@name}' for '#{@model.table_name}'"
+  end
+
+  def ensure_config_exists!(key)
+    return if DEFAULT_CONFIG.key? key
+
+    raise Hotsheet::Error, "Unknown config '#{key}' for row '#{@name}'"
+  end
+
+  def validate_config_value!(key, value)
+    return if [true, false].include? value
+
+    raise Hotsheet::Error, "Invalid config '#{key}' for row '#{@name}'"
+  end
+end

spec/lib/hotsheet/config_spec.rb

@@ -21,8 +21,7 @@
 
       it "uses all database columns" do
         expect(sheet).to be_a Hotsheet::Sheet
-        expect(sheet.model).to eq Post
-        expect(sheet.rows.pluck(:name)).to eq %i[id title body user_id created_at updated_at]
+        expect(sheet.rows.map(&:name)).to eq %w[id title body user_id created_at updated_at]
       end
     end
 
@@ -37,22 +36,7 @@
       end
 
       it "keeps the rows in the correct order" do
-        expect(sheet.rows.pluck(:name)).to eq(%i[title body])
-      end
-    end
-
-    context "with visibility" do
-      let(:config) do
-        Hotsheet.configure do
-          sheet :Post do
-            row :title, visible: false
-            row :body
-          end
-        end
-      end
-
-      it "does not show the hidden rows" do
-        expect(sheet.rows.pluck(:name)).to eq %i[body]
+        expect(sheet.rows.map(&:name)).to eq(%w[title body])
       end
     end
   end

spec/lib/hotsheet/sheet/row_spec.rb

@@ -0,0 +1,75 @@
+# frozen_string_literal: true
+
+require "spec_helper"
+
+RSpec.describe Hotsheet::Sheet::Row do
+  fixtures :users
+
+  let(:config) { {} }
+  let(:name) { :name }
+  let(:row) { described_class.new User, name, config }
+
+  describe "permissions" do
+    it "is visible and editable by default" do
+      expect(row.visible?).to be true
+      expect(row.editable?).to be true
+    end
+  end
+
+  describe "#human_name" do
+    it "reads the human name from the locales" do
+      expect(row.human_name).to eq "Name"
+    end
+  end
+
+  describe "#update!" do
+    let(:user) { users(:admin) }
+
+    context "with readonly config" do
+      let(:config) { { editable: false } }
+
+      it "raises an error" do
+        expect { row.update!(user.id, "Bob") }.to raise_error "Forbidden"
+      end
+    end
+
+    context "with invalid value" do
+      it "raises an error" do
+        expect { row.update!(user.id, "B") }
+          .to raise_error "Name is too short (minimum is 2 characters)"
+      end
+    end
+
+    context "with nonexistent id" do
+      it "raises an error" do
+        expect { row.update!(0, "Bob") }.to raise_error "Not found"
+      end
+    end
+  end
+
+  describe "#validate!" do
+    context "with nonexistent database column" do
+      let(:name) { :age }
+
+      it "raises an error" do
+        expect { row.validate! }.to raise_error "Unknown database column 'age' for 'users'"
+      end
+    end
+
+    context "with nonexistent config" do
+      let(:config) { { readonly: true } }
+
+      it "raises an error" do
+        expect { row.validate! }.to raise_error "Unknown config 'readonly' for row 'name'"
+      end
+    end
+
+    context "with invalid config" do
+      let(:config) { { editable: "no" } }
+
+      it "raises an error" do
+        expect { row.validate! }.to raise_error "Invalid config 'editable' for row 'name'"
+      end
+    end
+  end
+end