Introduce Cells [closes #47]

CHANGELOG.md

@@ -1,5 +1,7 @@
 # Unreleased changes
 
+- Introduce Cells gem to make views and controllers more manageable. [#47]
+
 # v2.2.0 / 2020-04-10
 
 - Set default time zone to Eastern, since that's what the covidtracking.com API uses for all states. [#38]

Gemfile

@@ -23,6 +23,8 @@ gem 'jbuilder', '~> 2.7'
 # Reduces boot times through caching; required in config/boot.rb
 gem 'bootsnap', '>= 1.4.2', require: false
 
+gem 'cells-rails'
+gem 'cells-haml'
 gem 'city-state'
 gem 'gettext_i18n_rails'
 gem "haml-rails", "~> 2.0"
@@ -34,6 +36,7 @@ gem 'typhoeus'
 group :development, :test do
   # Call 'byebug' anywhere in the code to stop execution and get a debugger console
   gem 'byebug', platforms: [:mri, :mingw, :x64_mingw]
+  gem 'rspec-cells'
   gem 'rspec-rails', '~> 4.0.0'
   gem 'cucumber-rails', require: false
 end

Gemfile.lock

@@ -79,6 +79,17 @@ GEM
       rack-test (>= 0.6.3)
       regexp_parser (~> 1.5)
       xpath (~> 3.2)
+    cells (4.1.7)
+      declarative-builder (< 0.2.0)
+      declarative-option (< 0.2.0)
+      tilt (>= 1.4, < 3)
+      uber (< 0.2.0)
+    cells-haml (0.0.10)
+      cells (>= 4.0.1, <= 6.0.0)
+      haml (>= 4.1.0.beta.1)
+    cells-rails (0.1.0)
+      actionpack (>= 5.0)
+      cells (>= 4.1.6, < 5.0.0)
     city-state (0.1.0)
       rubyzip (>= 1.1)
     coderay (1.1.2)
@@ -108,6 +119,9 @@ GEM
       railties (>= 4.2, < 7)
     cucumber-tag_expressions (1.1.1)
     cucumber-wire (0.0.1)
+    declarative-builder (0.1.0)
+      declarative-option (< 0.2.0)
+    declarative-option (0.1.0)
     diff-lcs (1.3)
     erubi (1.9.0)
     erubis (2.7.0)
@@ -241,6 +255,9 @@ GEM
       rspec-core (~> 3.9.0)
       rspec-expectations (~> 3.9.0)
       rspec-mocks (~> 3.9.0)
+    rspec-cells (0.3.5)
+      cells (>= 4.0.0, < 6.0.0)
+      rspec-rails (< 5.0)
     rspec-core (3.9.1)
       rspec-support (~> 3.9.1)
     rspec-expectations (3.9.1)
@@ -298,6 +315,7 @@ GEM
       ethon (>= 0.9.0)
     tzinfo (1.2.7)
       thread_safe (~> 0.1)
+    uber (0.1.0)
     vcr (5.1.0)
     web-console (4.0.1)
       actionview (>= 6.0.0)
@@ -325,6 +343,8 @@ PLATFORMS
 DEPENDENCIES
   bootsnap (>= 1.4.2)
   byebug
+  cells-haml
+  cells-rails
   city-state
   cucumber-rails
   faker
@@ -339,6 +359,7 @@ DEPENDENCIES
   rails (~> 6.0.2, >= 6.0.2.2)
   rdiscount
   redis
+  rspec-cells
   rspec-rails (~> 4.0.0)
   ruby_parser
   sass-rails (>= 6)

Guardfile

@@ -67,6 +67,9 @@ guard :rspec, cmd: "bundle exec rspec" do
   watch(%r{^spec/acceptance/steps/(.+)_steps\.rb$}) do |m|
     Dir[File.join("**/#{m[1]}.feature")][0] || "spec/acceptance"
   end
+
+  # Cells views
+  watch(%r{^app/(cells/.+)/[^/]+\.(erb|haml)$}) {|m| "spec/#{m[1]}_cell_spec.rb" }
 end
 
 cucumber_options = {

README.md

@@ -14,6 +14,8 @@ BTW, this is a labor of love developed for the purpose of public service. Curren
 
 This is a Rails 6 application, but since all the data comes from the COVID Tracking Project API, we're not using ActiveRecord at the moment (though we could always add it if it became necessary to implement some particular feature). Instead, we're pulling all the data with [Typhoeus](https://github.com/typhoeus/typhoeus), and caching it with Redis (currently for 6 hours), then feeding it to the [svg-graph](https://github.com/lumean/svg-graph2) gem to draw nice charts.
 
+Note that instead of partials and helpers, we're mostly using the [Cells](https://github.com/trailblazer/cells) gem to break up views and controllers. The short summary, for those unfamiliar with this gem, is that a cell is sort of like a partial with its own controller context.
+
 ### Development
 
 The easiest way to run a development instance of this application is probably to use Docker. The project already contains Docker configuration files, so `docker-compose up` should start both a Rails server for the Web application and a Redis server to run the cache. Modify the paths in docker-compose.yml as necessary for your local filesystem bindings.

app/cells/chart/show.haml

@@ -0,0 +1 @@
+= chart

app/cells/chart_cell.rb

@@ -0,0 +1,18 @@
+class ChartCell < Cell::ViewModel
+  private
+
+  def data
+    model
+  end
+
+  def chart_data
+    @chart_data ||=
+    data.transform_values do |requests|
+      requests.map {|(_, response)| [Date.parse(response['date'].to_s), response['positive']] }
+    end
+  end
+
+  def chart
+    @chart ||= Chart.new(chart_data).to_graph.burn_svg_only
+  end
+end

app/cells/state_list/show.haml

@@ -0,0 +1 @@
+= h(_ 'Data for %{states}') % {states: state_names_and_abbrs.join(_ ', ').html_safe}

app/cells/state_list_cell.rb

@@ -0,0 +1,22 @@
+class StateListCell < Cell::ViewModel
+  include ERB::Util
+
+  private
+
+  def states
+    model
+  end
+
+  def state_names_and_abbrs
+    states.map.with_index(1) {|state, index| name_and_abbr state, index }
+  end
+
+  def name_and_abbr(state, index)
+    h(_'%{span}%{state_name}%{_span} (%{state_abbr})') % {
+      span: "<span class='state-#{index}'>".html_safe,
+      _span: '</span>'.html_safe,
+      state_name: _(state.name),
+      state_abbr: _(state.abbr)
+    }
+  end
+end

app/cells/state_requests/show.haml

@@ -0,0 +1,7 @@
+%details
+  %summary= _ '%{state} (%{n_requests}):' % {state: state.name, n_requests: n_('%{n} request', '%{n} requests', requests.length) % {n: requests.length}}
+  %dl.raw-data
+    - requests.each do |(url, response)|
+      %dt= url
+      %dd
+        %code= response

app/cells/state_requests_cell.rb

@@ -0,0 +1,15 @@
+class StateRequestsCell < Cell::ViewModel
+  def initialize(model, _ = {})
+    case model
+    in {state: state, requests: requests}
+      @state = state
+      @requests = requests
+    else
+      raise ArgumentError, ':state and :requests are required'
+    end
+  end
+
+  private
+
+  attr_reader :state, :requests
+end

app/cells/state_selector/show.haml

@@ -0,0 +1,6 @@
+= form_with url: url, local: true, class: 'state-selector' do |form|
+  %p
+    = label_tag :states, _('Choose one or more states:')
+    = select_tag :states, options_for_select(locale_state_options, states&.map(&:abbr)), multiple: true
+  %p
+    = submit_tag _('Go')

app/cells/state_selector_cell.rb

@@ -0,0 +1,23 @@
+class StateSelectorCell < Cell::ViewModel
+  include ActionView::Helpers::FormHelper
+  include ActionView::Helpers::FormOptionsHelper
+
+  private
+
+  def states
+    model
+  end
+
+  def url
+    @url ||= options[:url]
+  end
+
+
+  def locale_state_options
+    @locale_state_options ||= states_for_menu.sort_by {|state| _(state.name)}.map {|state| [_(state.name), state.abbr]}
+  end
+
+  def states_for_menu
+    @states_for_menu ||= State.all
+  end
+end

app/controllers/states_controller.rb

@@ -8,11 +8,6 @@ def show
 
     query = Query.new states: @states, date_range: date_range
     @raw_data = query.raw_data
-    chart_data = @raw_data.transform_values do |requests| # TODO: this should probably move into Query
-      requests.map {|(_, response)| [Date.parse(response['date'].to_s), response['positive']] }
-    end
-
-    @chart = Chart.new(chart_data).to_graph.burn_svg_only.html_safe
   end
 
   def choose

app/helpers/application_helper.rb

@@ -1,5 +1,2 @@
 module ApplicationHelper
-  def states
-    State.all
-  end
 end

app/models/chart.rb

@@ -42,6 +42,6 @@ def max_value
   end
 
   def divisions
-    @divisions ||= [max_value.ceil(-Math.log10(max_value)) / 10, 10].max
+    @divisions ||= max_value.nil? ? 10 : [max_value.ceil(-Math.log10(max_value)) / 10, 10].max
   end
 end

app/views/layouts/application.html.haml

@@ -13,7 +13,7 @@
       %h1
         = link_to '/' do
           = h(_ '%{span}COVID-19%{_span} Tracking Charts by U.S. State') % {span: '<span class="accent">'.html_safe, _span: '</span>'.html_safe}
-    %nav= render 'shared/state_selector', states: states # TODO: consider Cells or something here
+    %nav= cell(:state_selector, @states, url: choose_states_path)
 
     %main
       - if content_for :page_title

app/views/states/show.html.haml

@@ -2,14 +2,14 @@
   = stylesheet_link_tag 'chart'
 
 - content_for :page_title do
-  = render 'state_list', states: @states
+  = cell 'state_list', @states
 
 :markdown
   #{_ 'This chart shows the *daily cumulative total* number of individuals who have tested positive for coronavirus.'}
 
-= @chart
+= cell('chart', @raw_data).call
 
 %details
   %summary= _ 'Raw data'
   - @raw_data.each do |state, requests|
-    = render 'state_requests', state: state, requests: requests
+    = cell('state_requests', {state: state, requests: requests}).call

locale/app.pot

@@ -8,8 +8,8 @@ msgid ""
 msgstr ""
 "Project-Id-Version: app 1.0.0\n"
 "Report-Msgid-Bugs-To: \n"
-"POT-Creation-Date: 2020-04-09 22:42+0000\n"
-"PO-Revision-Date: 2020-04-09 22:42+0000\n"
+"POT-Creation-Date: 2020-04-14 02:20+0000\n"
+"PO-Revision-Date: 2020-04-14 02:20+0000\n"
 "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
 "Language-Team: LANGUAGE <[email protected]>\n"
 "Language: \n"

spec/cells/chart_cell_spec.rb

@@ -0,0 +1,19 @@
+require 'rails_helper'
+
+RSpec.describe ChartCell, type: :cell do
+  let(:data) { {Faker::Lorem.sentence => [[nil, OpenStruct.new(date: rand(1..10).days.ago, positive: rand(1000))]]} }
+
+  subject { described_class.new data }
+
+  describe 'constructor' do
+    it { is_expected.to be_a_kind_of Cell::ViewModel }
+  end
+
+  describe '#show' do
+    subject { Capybara.string cell(described_class, data).call }
+
+    it 'renders an SVG chart' do
+      expect(subject).to have_selector 'svg'
+    end
+  end
+end

spec/cells/state_list_cell_spec.rb

@@ -0,0 +1,25 @@
+require 'rails_helper'
+
+RSpec.describe StateListCell, type: :cell do
+  let(:states) { State.all.sample rand(2..5) }
+  subject { described_class.new states }
+
+  describe 'constructor' do
+    it { is_expected.to be_a_kind_of Cell::ViewModel }
+    it { is_expected.to be_a_kind_of ERB::Util }
+  end
+
+  describe '#show' do
+    subject { Capybara.string cell(described_class, states).call }
+
+    it 'returns a string of the form "Data for state1 (S1), state2 (S2), ..."' do
+      expect(subject.text.chomp).to be == "Data for #{states.map {|state| "#{state.name} (#{state.abbr})" }.join ', ' }"
+    end
+
+    it 'wraps each state name in a span of class "state-n"' do
+      states.each.with_index(1) do |state, index|
+        expect(subject).to have_selector "span.state-#{index}", text: state.name
+      end
+    end
+  end
+end

spec/cells/state_requests_cell_spec.rb

@@ -0,0 +1,49 @@
+require 'rails_helper'
+
+RSpec.describe StateRequestsCell, type: :cell do
+  let(:state) { nil }
+  let(:requests) { [] }
+
+  subject { described_class.new state: state, requests:  requests }
+
+  describe 'constructor' do
+    it { is_expected.to be_a_kind_of Cell::ViewModel }
+  end
+
+  describe '#show' do
+    let(:state) { State.all.sample }
+
+    subject { Capybara.string cell(described_class, state: state, requests: requests).call }
+
+    it 'renders a <details> element' do
+      expect(subject).to have_selector 'details'
+    end
+
+    context 'one request' do
+      let(:requests) { Faker::Lorem.words number: 1 }
+
+      it 'shows the state name in the summary, followed by "1 request"' do
+        expect(subject).to have_selector 'details > summary', text: "#{state.name} (1 request)"
+      end
+    end
+
+    context 'more than one request' do
+      let(:length) { rand 2..10 }
+      let(:urls) { Array.new(length) { File.join Faker::Lorem.words(number: rand(2..5)) } }
+      let(:responses) { Array.new(length) { {Faker::Lorem.word => Faker::Lorem.sentence} } }
+      let(:requests) { urls.zip responses }
+
+      it 'shows the state name in the summary, followed by the number of requests, properly pluralized' do
+        expect(subject).to have_selector 'details > summary', text: "#{state.name} (#{requests.length} requests)"
+      end
+
+      it 'shows each request as a dl entry, with the URL as the term and the response as the description' do
+        subject.find 'dl.raw-data', visible: :all do |raw_data|
+          requests.each do |(url, response)|
+            expect(raw_data).to have_xpath ".//dt[text()='#{url}']/following-sibling::*[1][self::dd]/code[text()='#{response}']", visible: :all
+          end
+        end
+      end
+    end
+  end
+end