Use model scopes and jbuilder (#582)

Adds jbuilder templates and improves logic behind API

app/controllers/api/pages_controller.rb

@@ -1,37 +1,37 @@
 class Api::PagesController < ApplicationController
-  before_action :set_language, only: [:show, :show_featured, :index]
   rescue_from ActiveRecord::RecordNotFound, with: :render_errors
+  before_filter :get_page, except: [:index, :featured]
 
   layout false
 
   def update
-    updater = PageUpdater.new(page, page_url(page))
+    updater = PageUpdater.new(@page, page_url(@page))
 
     if updater.update(all_params)
-      render json: { refresh: updater.refresh?, id: page.id }, status: :ok
+      render json: { refresh: updater.refresh?, id: @page.id }, status: :ok
     else
       render json: { errors: shallow_errors(updater.errors) }, status: 422
     end
   end
 
   def share_rows
-    render json: (page.shares.map do |s|
-      {html: render_to_string(partial: "share/#{s.name}s/summary_row", locals: {share: s, page: page})}
+    render json: (@page.shares.map do |s|
+      {html: render_to_string(partial: "share/#{s.name}s/summary_row", locals: {share: s, page: @page})}
     end)
   end
 
   def index
-    render json: reduce_and_order(page_scope, 100)
+    @pages = PageService.list(language: params[:language], limit: params[:limit])
+    render :index, format: :json
   end
 
   def show
-    render json: page
-  rescue ActiveRecord::RecordNotFound
-    render json: { errors: "No record was found with that slug or ID." }, status: 404
+    render :show, format: :json
   end
 
-  def show_featured
-    render json: page_scope.where(featured: true)
+  def featured
+    @pages = PageService.list_featured(language: params[:language])
+    render :index, format: :json
   end
 
   private
@@ -40,14 +40,6 @@ def render_errors
     render json: { errors: "No record was found with that slug or ID." }, status: 404
   end
 
-  def page_scope
-    @language.present? ? Page.where(language: @language) : Page.all
-  end
-
-  def reduce_and_order(collection, count)
-    collection.last(count).reverse
-  end
-
   def all_params
     # this method flattens a lot of nested data from one object per form element
     # to one object per entity (page, share variant, etc) to modify
@@ -75,14 +67,7 @@ def shallow_errors(errors)
     Rack::Utils.parse_query(errors.to_query)
   end
 
-  def page
+  def get_page
     @page ||= Page.find(params[:id])
   end
-
-  def set_language
-    @language ||= Language.find_by(code: params[:language])
-    if !params[:language].blank? && @language.blank?
-      render json: { errors: "The language you requested is not supported." }, status: 404
-    end
-  end
 end

app/models/page.rb

@@ -17,9 +17,9 @@ class Page < ActiveRecord::Base
   has_many :images,     dependent: :destroy
   has_many :links,      dependent: :destroy
 
-  scope :language,       -> (code) { code ? joins(:language).where(languages: { code: code }) : all }
-  scope :published,      -> { where(active: true) }
-  scope :featured_only,  -> { where(featured: true) }
+  scope :language,  -> (code) { code ? joins(:language).where(languages: { code: code }) : all }
+  scope :published, -> { where(active: true) }
+  scope :featured,  -> { where(featured: true) }
 
   validates :title, presence: true
   validates :liquid_layout, presence: true

app/services/page_service.rb

@@ -0,0 +1,17 @@
+module PageService
+  extend self
+
+  def list(language: nil, limit: 30)
+    Page.language(language).
+      limit(limit).
+      order('created_at desc').
+      published
+  end
+
+  def list_featured(language: nil)
+    Page.language(language).
+      featured.
+      order('created_at desc').
+      published
+  end
+end

app/views/api/pages/index.json.jbuilder

@@ -0,0 +1,4 @@
+json.array! @pages do |page|
+  json.extract! page, :id, :title, :slug, :content, :created_at, :updated_at, :active, :featured, :action_count
+  json.language page.language.code
+end

app/views/api/pages/show.json.jbuilder

@@ -0,0 +1,2 @@
+json.extract! @page, :id, :title, :slug, :content, :created_at, :updated_at, :active, :featured, :action_count
+json.language @page.language.code

config/routes.rb

@@ -137,20 +137,17 @@
       post 'webhook'
     end
 
-    namespace :pages do
-      get 'featured/', action: 'show_featured'
-    end
-
     resources :pages do
+      get 'share-rows', on: :member, action: 'share_rows'
+      get 'featured', on: :collection
+
       resource  :analytics
       resources :actions do
         post 'validate', on: :collection, action: 'validate'
       end
-
-      get 'share-rows', on: :member, action: 'share_rows'
     end
 
-    resources :members 
+    resources :members
   end
   # Example resource route within a namespace:
   #   namespace :admin do

spec/controllers/api/pages_controller_spec.rb

@@ -8,6 +8,36 @@
     allow(Page).to receive(:find){ page }
   end
 
+  describe 'GET index' do
+    before do
+      allow(PageService).to receive(:list)
+      get :index, language: 'en', format: :json
+    end
+
+    it 'gets list of pages' do
+      expect(PageService).to have_received(:list).with(hash_including({language: 'en'}))
+    end
+
+    it 'responds with json' do
+      expect(response.content_type).to eq("application/json")
+    end
+  end
+
+  describe 'GET featured' do
+    before do
+      allow(PageService).to receive(:list_featured)
+      get :featured, language: 'en', format: :json
+    end
+
+    it 'gets list of pages' do
+      expect(PageService).to have_received(:list_featured).with(hash_including({language: 'en'}))
+    end
+
+    it 'responds with json' do
+      expect(response.content_type).to eq("application/json")
+    end
+  end
+
   describe 'PUT update' do
     before do
       allow(PageUpdater).to receive(:new) { page_updater }
@@ -31,7 +61,7 @@
 
   describe 'GET show' do
     context 'for existing page' do
-      before { get :show, id: '2' }
+      before { get :show, id: '2', format: 'json' }
 
       it 'finds page' do
         expect(Page).to have_received(:find).with('2')

spec/models/page_spec.rb

@@ -402,7 +402,7 @@
       end
 
       it 'returns all if no language code is passed' do
-        expect(Page.language(nil)).to match([en_page, fr_page])
+        expect(Page.language(nil)).to match_array([en_page, fr_page])
       end
     end
 
@@ -411,7 +411,7 @@
       let!(:page) { create(:page, featured: false) }
 
       it 'finds featured' do
-        expect(Page.featured_only).to match([featured_page])
+        expect(Page.featured).to match([featured_page])
       end
     end
   end

spec/requests/api/pages_spec.rb

@@ -1,115 +1,73 @@
-require_relative 'shared_language_pages.rb'
 require 'rails_helper'
 
 describe "api/pages" do
-
   def json
     JSON.parse(response.body)
   end
 
-
-  before :each do
-    # I'm rounding the time. Ruby deals with time in nanoseconds whereas the database deals with time in microsecond
-    # precision. If I don't round the time, the expectation comparing the JSON response expects data from the DB
-    # with nanosecond precision.
-    @time_now = Time.at(Time.now.to_i)
-    allow(Time).to receive(:now).and_return(@time_now)
-  end
-
-  describe 'GET pages' do
-    context 'with no specified language' do
-      let!(:featured_pages) { create_list :page, 5, featured: true }
-      let!(:mvp_pages) { create_list :page, 5, featured: false }
-      let!(:last_featured_page) { create :page, title: 'I am the latest featured page', featured: true, slug: 'garden_slug' }
-      let!(:last_mvp_page) { create :page, title: 'I am the latest test page', featured: false}
-
-      it 'gets a hundred of both featured and unfeatured pages in a reversed order if requested without an id' do
-        get api_pages_path
-        expect(response).to be_success
-        # Includes both featured and unfeatured pages.
-        expect(json).to include last_featured_page.as_json
-        expect(json).to include last_mvp_page.as_json
-        # Limits its reach to the latest hundred pages if there are more than a hundred pages to search through.
-        expect(json.size).to eq(Page.count)
-      end
-
-      it 'gets a single page if searched by an id of a page that exists' do
-        get api_page_path(id: last_mvp_page.id.to_s)
-        expect(response).to be_success
-        expect(json['id']).to match last_mvp_page.id
-      end
-
-      it 'gets a single page if searched by a slug of a page that exists' do
-        get api_page_path(id: last_featured_page.slug)
-        expect(response).to be_success
-        expect(json['id']).to match last_featured_page.id
-      end
-
-      it 'returns an error if searching for an ID or slug of a page that does not exist' do
-        get api_page_path(id: last_featured_page.slug + '_epidemic')
-        expect(response.status).to eq(404)
-        expect(json).to match({ "errors" => "No record was found with that slug or ID."})
-      end
+  describe 'GET index' do
+    before do
+      create(:page, active: true, title: 'Foo', content: 'Bar')
     end
 
-    context 'with languages' do
-      describe 'with language that does not exist' do
-        it 'returns json with error' do
-          get api_pages_path, {language: 'klingon'}
-          expect(json).to match({"errors" => "The language you requested is not supported."})
-          expect(response.status).to eq(404)
-        end
-      end
-    end
+    subject { JSON.parse(response.body) }
+
+    before { get('/api/pages.json') }
 
-    describe 'with languages that exist' do
+    it 'returns list of pages' do
+      expect(subject.size).to eq(1)
 
-      include_context "shared language pages" do
-        [:de,:fr,:en,:es].each do |language_code|
-          it "in #{language_code}, it gets pages only in that language" do
-            get api_pages_path, { language: language_code.to_s }
-            expect(json).to include(@page_hash[language_code][:featured].first.as_json)
-            expect(json).to include(@page_hash[language_code][:ordinary].first.as_json)
-          end
-        end
-      end
+      expect(subject.first.keys).to match(
+        %w{id title slug content created_at updated_at active featured action_count language}
+      )
 
+      expect(subject.first.symbolize_keys).to include({
+        title: 'Foo',
+        content: 'Bar'
+      })
     end
   end
 
   describe 'GET featured' do
-    context 'with no specified language' do
-      let!(:featured_page) { create(:page, featured: true) }
-      let!(:mvp_page) { create(:page, featured: false) }
-
-      it 'gets only featured pages' do
-        get api_pages_featured_path
-        expect(json.size).to eq(1)
-        expect(json.first['id']).to eq(featured_page.id)
-      end
+    before do
+      create(:page, featured: true, active: true, title: 'Foo', content: 'Bar')
+      create(:page, featured: false)
     end
 
-    context 'with languages' do
-      describe 'with language that does not exist' do
-        it 'returns json with error' do
-          get api_pages_featured_path, {language: 'klingon'}
-          expect(json).to match({"errors" => "The language you requested is not supported."})
-          expect(response.status).to eq(404)
-        end
-      end
-
-      describe 'with languages that exist' do
-        include_context "shared language pages" do
-          [:de,:fr,:en,:es].each.each do |language_code|
-            it "in #{language_code}, it gets pages only in that language" do
-              get api_pages_featured_path, { language: language_code.to_s }
-              expect(json.first['language_id']).to match(@page_hash[language_code][:featured].first.language_id)
-            end
-          end
-        end
-      end
+    subject { JSON.parse(response.body) }
+
+    before { get( featured_api_pages_path(format: :json)) }
+
+    it 'returns list of pages' do
+      expect(subject.size).to eq(1)
 
+      expect(subject.first.keys).to match(
+        %w{id title slug content created_at updated_at active featured action_count language}
+      )
+
+      expect(subject.first.symbolize_keys).to include({
+        title: 'Foo',
+        content: 'Bar'
+      })
     end
+  end
+
+  describe 'GET show' do
+    let(:page) { create(:page, title: 'Foo', content: 'Bar') }
+
+    subject { JSON.parse(response.body) }
 
+    before { get( api_page_path(page, format: :json) ) }
+
+    it 'returns page' do
+      expect(subject.keys).to match(
+        %w{id title slug content created_at updated_at active featured action_count language}
+      )
+
+      expect(subject.symbolize_keys).to include({
+        title: 'Foo',
+        id: page.id
+      })
+    end
   end
 end