refactor the paginate liquid tag (+ add specs)

did committed Feb 05, 2015

commit 836cbdf4afd917b57ad2c4621dd275d28426a5d0

Showing 13 changed files with 366 additions and 90 deletions

locomotive/steam.rb b/lib/locomotive/steam.rb +1 -0

@@	@@ -3,6 +3,7 @@ require 'locomotive/common'
	require_relative 'steam/core_ext'
	require_relative 'steam/exceptions'
	require_relative 'steam/configuration'
+	require_relative 'steam/monkey_patches'
	require_relative 'steam/liquid'

	require_relative 'steam/repositories'

locomotive/steam/core_ext/hash.rb b/lib/locomotive/steam/core_ext/hash.rb +10 -6

@@	@@ -3,8 +3,12 @@
	module HashConverter
	class << self

-	def to_underscore hash
-	convert hash, :underscore
+	def to_underscore(hash)
+	convert(hash, :underscore)
+	end
+
+	def to_string(hash)
+	convert(hash, :to_s)
	end

	# FIXME: not sure it will be ever needed
@@	@@ -12,16 +16,16 @@ module HashConverter
	# convert hash, :camelize, :lower
	# end

-	def convert obj, *method
+	def convert(obj, *method)
	case obj
	when Hash
-	obj.inject({}) do \|h,(k,v)\|
-	v = convert v, *method
+	obj.inject({}) do \|h, (k,v)\|
+	v = convert(v, *method)
	h[k.send(*method)] = v
	h
	end
	when Array
-	obj.map {\|m\| convert m, *method }
+	obj.map { \|m\| convert(m, *method) }
	else
	obj
	end

locomotive/steam/liquid/tags/consume.rb b/lib/locomotive/steam/liquid/tags/consume.rb +1 -1

@@	@@ -15,7 +15,7 @@ module Locomotive
	#
	class Consume < ::Liquid::Block

-	Syntax = /(#{::Liquid::VariableSignature}+)\sfrom\s(#{::Liquid::QuotedString}\|#{::Liquid::VariableSignature}+)(.*)?/
+	Syntax = /(#{::Liquid::VariableSignature}+)\sfrom\s(#{::Liquid::QuotedString}\|#{::Liquid::VariableSignature}+)(.*)?/o

	def initialize(tag_name, markup, options)
	if markup =~ Syntax

locomotive/steam/liquid/tags/editable/base.rb b/lib/locomotive/steam/liquid/tags/editable/base.rb +1 -1

@@	@@ -5,7 +5,7 @@ module Locomotive
	module Editable
	class Base < ::Liquid::Block

-	Syntax = /(#{::Liquid::QuotedFragment})(\s,\s#{::Liquid::Expression}+)?/
+	Syntax = /(#{::Liquid::QuotedFragment})(\s,\s#{::Liquid::Expression}+)?/o

	attr_accessor :slug

locomotive/steam/liquid/tags/fetch_page.rb b/lib/locomotive/steam/liquid/tags/fetch_page.rb +1 -1

@@	@@ -12,7 +12,7 @@ module Locomotive
	#
	class FetchPage < ::Liquid::Tag

-	Syntax = /(#{::Liquid::VariableSignature}+)\s+as\s+(#{::Liquid::VariableSignature}+)/
+	Syntax = /(#{::Liquid::VariableSignature}+)\s+as\s+(#{::Liquid::VariableSignature}+)/o

	def initialize(tag_name, markup, options)
	if markup =~ Syntax

locomotive/steam/liquid/tags/paginate.rb b/lib/locomotive/steam/liquid/tags/paginate.rb +80 -52

@@	@@ -3,7 +3,7 @@ module Locomotive
	module Liquid
	module Tags

-	# Paginate a collection
+	# Paginate a collection (array or from a DB).
	#
	# Usage:
	#
@@	@@ -13,20 +13,18 @@ module Locomotive
	# {% endfor %}
	# {% endpaginate %}
	#
-
	class Paginate < ::Liquid::Block

-	Syntax = /(#{::Liquid::Expression}+)\s+by\s+([0-9]+)/
+	Syntax = /(#{::Liquid::QuotedFragment}+)\s+by\s+([0-9]+)/o

-	def initialize(tag_name, markup, tokens, context)
+	def initialize(tag_name, markup, options)
	if markup =~ Syntax
-	@collection_name = $1
-	@per_page = $2.to_i
-	@options = { }
-	markup.scan(::Liquid::TagAttributes) { \|key, value\| @options[key.to_sym] = value.gsub(/^'/, '').gsub(/'$/, '') }
-	@window_size = @options[:window_size] ? @options[:window_size].to_i : 3
+	@collection_name = $1
+	@per_page = $2.to_i
+	@paginate_options = {}
+	markup.scan(::Liquid::TagAttributes) { \|key, value\| @paginate_options[key.to_sym] = value.gsub(/^'/, '').gsub(/'$/, '') }
	else
-	raise ::Liquid::SyntaxError.new("Syntax Error in 'paginate' - Valid syntax: paginate <collection> by <number>")
+	raise ::Liquid::SyntaxError.new('Valid syntax: paginate <collection> by <number>')
	end

	super
@@	@@ -34,56 +32,78 @@ module Locomotive

	def render(context)
	context.stack do
-	collection = context[@collection_name]
+	pagination = context['paginate'] = paginate_collection(context)
+
+	path = sanitize_path(context['fullpath'])

-	raise ::Liquid::ArgumentError.new("Cannot paginate array '#{@collection_name}'. Not found.") if collection.nil?
+	build_next_previous_links(pagination, path)

-	if collection.is_a? Array
-	pagination = Kaminari.paginate_array(collection).page(context['current_page']).per(@per_page).to_liquid.stringify_keys
-	else
-	pagination = collection.send(:paginate, {
-	page: context['current_page'],
-	per_page: @per_page
-	}).to_liquid.stringify_keys
+	if pagination['total_pages'] > 1
+	build_parts(pagination, path)
	end
-	page_count, current_page = pagination['total_pages'], pagination['current_page']

-	path = sanitize_path(context['fullpath'])
+	super
+	end
+	end

-	pagination['previous'] = link(I18n.t('pagination.previous'), current_page - 1, path) if pagination['previous_page']
-	pagination['next'] = link(I18n.t('pagination.next'), current_page + 1, path) if pagination['next_page']
-	pagination['parts'] = []
-
-	hellip_break = false
-
-	if page_count > 1
-	1.upto(page_count) do \|page\|
-	if current_page == page
-	pagination['parts'] << no_link(page)
-	elsif page == 1
-	pagination['parts'] << link(page, page, path)
-	elsif page == page_count - 1
-	pagination['parts'] << link(page, page, path)
-	elsif page <= current_page - window_size or page >= current_page + window_size
-	next if hellip_break
-	pagination['parts'] << no_link('…')
-	hellip_break = true
-	next
-	else
-	pagination['parts'] << link(page, page, path)
-	end
-
-	hellip_break = false
-	end
-	end
+	private

-	context['paginate'] = pagination
+	# Paginate the collection and returns a pagination
+	# object storing all the information about the paginated
+	# collection.
+	#
+	def paginate_collection(context)
+	collection = context[@collection_name]
+	current_page = context['current_page']

-	render_all(@nodelist, context)
+	raise ::Liquid::ArgumentError.new("Cannot paginate array '#{@collection_name}'. Not found.") if collection.nil?
+
+	pagination = (if collection.is_a?(Array)
+	Kaminari.paginate_array(collection).page(current_page).per(@per_page)
+	else
+	collection.send(:paginate, page: current_page, per_page: @per_page)
+	end)
+
+	# make sure the pagination object is a hash with strings as keys (and not symbol)
+	HashConverter.to_string(pagination.to_liquid).tap do \|_pagination\|
+	_pagination['parts'] = []
	end
	end

-	private
+	def build_next_previous_links(pagination, path)
+	current_page = pagination['current_page']
+
+	if pagination['previous_page']
+	pagination['previous']= link(I18n.t('pagination.previous'), current_page - 1, path)
+	end
+
+	if pagination['next_page']
+	pagination['next'] = link(I18n.t('pagination.next'), current_page + 1, path)
+	end
+	end
+
+	def build_parts(pagination, path)
+	hellip_break = false
+
+	1.upto(pagination['total_pages']) do \|page\|
+	hellip_break = _build_parts(pagination, path, page, hellip_break)
+	end
+	end
+
+	def _build_parts(pagination, path, page, hellip_break)
+	if pagination['current_page'] == page
+	pagination['parts'] << no_link(page)
+	elsif is_page_a_bound?(pagination, page)
+	pagination['parts'] << link(page, page, path)
+	elsif is_page_inside_window?(pagination, page)
+	pagination['parts'] << no_link('…') unless hellip_break
+	return true
+	else
+	pagination['parts'] << link(page, page, path)
+	end
+
+	false
+	end

	def sanitize_path(path)
	_path = path.gsub(/page=[0-9]+&?/, '').gsub(/_pjax=true&?/, '')
@@	@@ -91,8 +111,16 @@ module Locomotive
	_path
	end

+	def is_page_inside_window?(pagination, page)
+	page <= pagination['current_page'] - window_size or page >= pagination['current_page'] + window_size
+	end
+
+	def is_page_a_bound?(pagination, page)
+	page == 1 \|\| page == pagination['total_pages']
+	end
+
	def window_size
-	@window_size
+	@window_size \|\|= @paginate_options[:window_size] ? @paginate_options[:window_size].to_i : 3
	end

	def no_link(title)
@@	@@ -105,7 +133,7 @@ module Locomotive
	end
	end

-	::Liquid::Template.register_tag('paginate', Paginate)
+	::Liquid::Template.register_tag('paginate'.freeze, Paginate)
	end

	end

locomotive/steam/liquid/tags/session_assign.rb b/lib/locomotive/steam/liquid/tags/session_assign.rb +1 -1

@@	@@ -12,7 +12,7 @@ module Locomotive
	# {{ session.foo }}
	#
	class SessionAssign < ::Liquid::Tag
-	Syntax = /(#{::Liquid::VariableSignature}+)\s=\s(#{::Liquid::QuotedFragment}+)/
+	Syntax = /(#{::Liquid::VariableSignature}+)\s=\s(#{::Liquid::QuotedFragment}+)/o

	def initialize(tag_name, markup, options)
	if markup =~ Syntax

locomotive/steam/liquid/tags/with_scope.rb b/lib/locomotive/steam/liquid/tags/with_scope.rb +1 -1

@@	@@ -18,7 +18,7 @@ module Locomotive

	OPERATORS = %w(all exists gt gte in lt lte ne nin size near within)

-	SYMBOL_OPERATORS_REGEXP = /(\w+\.(#{OPERATORS.join('\|')})){1}\s*\:/
+	SYMBOL_OPERATORS_REGEXP = /(\w+\.(#{OPERATORS.join('\|')})){1}\s*\:/o

	# register the tag
	tag_name :with_scope

locomotive/steam/monkey_patches.rb b/lib/locomotive/steam/monkey_patches.rb +1 -0

@@	@@ -1,3 +1,4 @@
	# require_relative 'monkey_patches/httparty.rb'
	# require_relative 'monkey_patches/dragonfly.rb'
	require_relative 'monkey_patches/haml.rb'
+	require_relative 'monkey_patches/kaminari.rb'

locomotive/steam/monkey_patches/haml.rb b/lib/locomotive/steam/monkey_patches/haml.rb +10 -10

@@	@@ -1,17 +1,17 @@
-	require 'haml'
+	# require 'haml'

-	module Haml::Filters
+	# module Haml::Filters

-	remove_filter("Markdown") #remove the existing Markdown filter
+	# remove_filter("Markdown") #remove the existing Markdown filter

-	module Markdown # the contents of this are as before, but without the lazy_require call
+	# module Markdown # the contents of this are as before, but without the lazy_require call

-	include Haml::Filters::Base
+	# include Haml::Filters::Base

-	def render text
-	Locomotive::Steam::Markdown.new.render text
-	end
+	# def render text
+	# Locomotive::Steam::Markdown.new.render text
+	# end

-	end
+	# end

-	end
+	# end

locomotive/steam/monkey_patches/kaminari.rb b/lib/locomotive/steam/monkey_patches/kaminari.rb +45 -0

@@	@@ -0,0 +1,45 @@
+	# Note: avoid this issue https://github.com/amatsuda/kaminari/issues/518
+	require 'kaminari/config'
+	require 'kaminari/models/configuration_methods'
+	require 'kaminari/models/page_scope_methods'
+	require 'kaminari/models/array_extension'
+
+	module Kaminari
+
+	module PageScopeMethods
+
+	def to_liquid(options = {})
+	{
+	collection: to_a,
+	current_page: current_page,
+	previous_page: first_page? ? nil : current_page - 1,
+	total_entries: total_count,
+	per_page: limit_value
+	}.tap do \|hash\|
+	# note: very important to avoid extra and useless mongodb requests
+	hash[:total_pages] = (hash[:total_entries].to_f / limit_value).ceil
+	hash[:next_page] = current_page >= hash[:total_pages] ? nil : current_page + 1
+	end
+	end
+
+	end
+
+	# FIXME: it does not seem to be called
+	# class PaginatableArray < Array
+
+	# def to_liquid(options = {})
+	# puts "PaginatableArray.instance.to_liquid"
+	# {
+	# collection: to_a,
+	# current_page: current_page,
+	# previous_page: first_page? ? nil : current_page - 1,
+	# next_page: last_page? ? nil : current_page + 1,
+	# total_entries: total_count,
+	# total_pages: num_pages,
+	# per_page: limit_value
+	# }
+	# end
+
+	# end
+
+	end

spec/unit/liquid/tags/paginate_spec.rb +214 -0

@@	@@ -0,0 +1,214 @@
+	require 'spec_helper'
+
+	describe Locomotive::Steam::Liquid::Tags::Paginate do
+
+	let(:source) { <<-EOF
+	{% paginate projects by 5 %}
+	{% for project in paginate.collection %}!{{ project }}{% endfor %}
+	{{ paginate.next.url }}
+	{% endpaginate %}'
+	EOF
+	}
+
+	let(:projects) { ['RoR', 'MongoDB', 'Liquid', 'ReactJS', 'DCI', 'Bootstrap'] }
+	let(:page) { 1 }
+	let(:assigns) { { 'projects' => projects, 'current_page' => page, 'fullpath' => '/' } }
+	let(:context) { ::Liquid::Context.new(assigns, {}, {}) }
+	let(:output) { render_template(source, context) }
+
+	describe 'parsing' do
+
+	subject { parse_template(source) }
+
+	describe 'wrong syntax' do
+
+	let(:source) { '{% paginate projects %}{% endpaginate %}' }
+	it { expect { subject }.to raise_error(::Liquid::SyntaxError, 'Liquid syntax error: Valid syntax: paginate <collection> by <number>') }
+
+	end
+
+	describe 'with options for the pagination' do
+
+	let(:source) { '{% paginate projects by 2, window_size: 4 %}{% endpaginate %}' }
+	let(:block) { subject.root.nodelist.first }
+	it { expect(block.send(:window_size)).to eq 4 }
+
+	end
+
+	end
+
+	describe 'rendering' do
+
+	describe 'nil array' do
+
+	let(:projects) { nil }
+
+	subject { output }
+
+	it { expect { subject }.to raise_error(::Liquid::ArgumentError, "Liquid error: Cannot paginate array 'projects'. Not found.") }
+
+	end
+
+	describe 'simple array' do
+
+	subject { output }
+
+	it { is_expected.to include '!RoR!MongoDB!Liquid!ReactJS!DCI' }
+	it { is_expected.not_to include '!Bootstrap' }
+
+	describe 'second page of results: display the last item' do
+
+	let(:page) { 2 }
+	it { is_expected.to include '!Bootstrap' }
+	it { is_expected.not_to include '!RoR!MongoDB!Liquid!ReactJS!DCI' }
+
+	end
+
+	end
+
+	describe 'array from a db collection' do
+
+	let(:projects) { KindaDBCollection.new(['RoR', 'MongoDB', 'Liquid', 'ReactJS', 'DCI', 'Bootstrap']) }
+
+	subject { output }
+
+	it { is_expected.to include '!RoR!MongoDB!Liquid!ReactJS!DCI' }
+	it { is_expected.not_to include '!Bootstrap' }
+
+	end
+
+	describe 'a very big collection' do
+
+	let(:projects) { (1..100).to_a }
+	let(:page) { 20 }
+	let(:source) { '{% paginate projects by 2, window_size: 10 %}{% assign _pagination = paginate %}{% endpaginate %}' }
+
+	before { output }
+	subject { context['_pagination']['parts'] }
+
+	it { expect(subject.first['title']).to eq 1 }
+	it { expect(subject[1]['title']).to eq '…' }
+	it { expect(subject[2]['title']).to eq 11 }
+	it { expect(subject[21]['title']).to eq '…' }
+	it { expect(subject.last['title']).to eq 50 }
+
+	end
+
+	describe ''
+
+	end
+
+	class KindaDBCollection < Struct.new(:collection)
+
+	def paginate(options = {})
+	total_pages = (collection.size.to_f / options[:per_page].to_f).to_f.ceil + 1
+	offset = (options[:page] - 1) * options[:per_page]
+
+	{
+	collection: collection[offset..(offset + options[:per_page]) - 1],
+	current_page: options[:page],
+	previous_page: options[:page] == 1 ? 1 : options[:page] - 1,
+	next_page: options[:page] == total_pages ? total_pages : options[:page] + 1,
+	total_entries: collection.size,
+	total_pages: total_pages,
+	per_page: options[:per_page]
+	}
+	end
+
+	def each(&block)
+	collection.each(&block)
+	end
+
+	def method_missing(method, *args)
+	collection.send(method, *args)
+	end
+
+	def to_liquid
+	self
+	end
+	end
+
+
+	# it 'keeps the original GET parameters' do
+	# context = liquid_context(fullpath: '/products?foo=1&bar=1&baz=1')
+	# template = Liquid::Template.parse(default_template)
+	# text = template.render!(context)
+	# expect(text).to match /\/products\?foo=1&bar=1&baz=1&page=2/
+	# end
+
+	# it 'does not include twice the page parameter' do
+	# context = liquid_context(fullpath: '/products?page=1')
+	# template = Liquid::Template.parse(default_template)
+	# text = template.render!(context)
+	# expect(text).to match /\/products\?page=2/
+	# end
+
+	# # ___ helpers methods ___ #
+
+	# def liquid_context(options = {})
+	# ::Liquid::Context.new(
+	# {},
+	# {
+	# 'projects' => options.has_key?(:collection) ? options[:collection] : PaginatedCollection.new(['Ruby on Rails', 'jQuery', 'mongodb', 'Liquid', 'sqlite3']),
+	# 'current_page' => options[:page] \|\| 1,
+	# 'path' => '/',
+	# 'fullpath' => options[:fullpath] \|\| '/'
+	# }, {
+	# page: FactoryGirl.build(:page)
+	# }, true)
+	# end
+
+	# def default_template
+	# "{% paginate projects by 2 %}
+	# {% for project in paginate.collection %}
+	# !{{ project }}!
+	# {% endfor %}
+	# {{ paginate.next.url }}
+	# {% endpaginate %}"
+	# end
+
+	# def default_pagination_template(options='')
+	# "{% paginate projects by 2 #{options} %}
+	# {% for project in paginate.collection %}
+	# !{{ project }}!
+	# {% endfor %}
+	# {{ paginate.next.url }}
+	# {{ paginate \| default_pagination }}
+	# {% endpaginate %}"
+	# end
+
+	# class PaginatedCollection
+
+	# def initialize(collection)
+	# @collection = collection \|\| []
+	# end
+
+	# def paginate(options = {})
+	# total_pages = (@collection.size.to_f / options[:per_page].to_f).to_f.ceil + 1
+	# offset = (options[:page] - 1) * options[:per_page]
+
+	# {
+	# collection: @collection[offset..(offset + options[:per_page]) - 1],
+	# current_page: options[:page],
+	# previous_page: options[:page] == 1 ? 1 : options[:page] - 1,
+	# next_page: options[:page] == total_pages ? total_pages : options[:page] + 1,
+	# total_entries: @collection.size,
+	# total_pages: total_pages,
+	# per_page: options[:per_page]
+	# }
+	# end
+
+	# def each(&block)
+	# @collection.each(&block)
+	# end
+
+	# def method_missing(method, *args)
+	# @collection.send(method, *args)
+	# end
+
+	# def to_liquid
+	# self
+	# end
+	# end
+
+	end

spec/unit/liquid/tags/with_scope_spec.rb +0 -17

@@	@@ -63,21 +63,4 @@ describe Locomotive::Steam::Liquid::Tags::WithScope do

	end

-	# describe "advanced queries thanks to h4s" do
-
-	# it 'decodes criteria with gt and lt' do
-	# scope = Locomotive::Liquid::Tags::WithScope.new('with_scope', 'price.gt:42.0, price.lt:50', ["{% endwith_scope %}"], {})
-	# options = decode_options(scope)
-	# expect(options[:price.gt]).to eq(42.0)
-	# expect(options[:price.lt]).to eq(50)
-	# end
-
-	# end
-
-	# def decode_options(tag, assigns = {})
-	# context = ::Liquid::Context.new(assigns)
-	# arguments = tag.instance_variable_get(:@arguments)
-	# tag.send(:decode, *arguments.interpolate(context))
-	# end
-
	end