Skip to content

Commit

Permalink
Add ability to define title and slug of page parts
Browse files Browse the repository at this point in the history
Fix specs with slug support

Use params[:slug] for page_part_field render

Validate presence of title in PagePart model

Add slug locales in FR and EN

Fix page_part_dialog JS

Change new_page_part_save errors text

Fix visual_editor_add_image specs with the adding of :slug param (thanks @parndt)

slug should be able to default to title.to_param (thanks @parndt)

Set default slug before validation

Deprecate old api for title_matches? and part_with_title methods

Add feature and deprecations in changelog

Add speakingurl-rails gem

Rewrite page part title to slug using speakingurl

Credit @simi for page part title feature

Move speakingurl dependency to pages

Precompile speakingurl only if new page part config is enable

Q&A of @parndt

fix speaking url load

fix permitted params for page parts
  • Loading branch information
bricesanchez committed May 2, 2015
1 parent 9c7a7f2 commit 040300c
Show file tree
Hide file tree
Showing 17 changed files with 113 additions and 42 deletions.
2 changes: 2 additions & 0 deletions changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
* Limited the jquery-ui assets loaded in the backend to the ones we use in the core. [#2735](https://github.com/refinery/refinerycms/pull/2735). [Philip Arndt](https://github.com/parndt)
* Moved the tabs to the left hand side of the screen. [#2734](https://github.com/refinery/refinerycms/pull/2734). [Isaac Freeman](https://github.com/isaacfreeman) & [Philip Arndt](https://github.com/parndt) & [Brice Sanchez](https://github.com/bricesanchez)
* Add extra fields partial in Admin Pages form advanced options [#2943](https://github.com/refinery/refinerycms/pull/2943). [Brice Sanchez](https://github.com/bricesanchez)
* Added ability to create page part title different form slug. [#2875](https://github.com/refinery/refinerycms/pull/2875). [Brice Sanchez](https://github.com/bricesanchez) & [Philip Arndt](https://github.com/parndt) & [Josef Šimánek](https://github.com/simi)
* Deprecated `part_with_title` method in `Refinery#Page` and `title_matches?` method in `Refinery#PagePart`. [#2875](https://github.com/refinery/refinerycms/pull/2875). [Brice Sanchez](https://github.com/bricesanchez) & [Philip Arndt](https://github.com/parndt) & [Josef Šimánek](https://github.com/simi)

* [See full list](https://github.com/refinery/refinerycms/compare/2-1-stable...master)

Expand Down
35 changes: 26 additions & 9 deletions core/app/assets/javascripts/refinery/admin.js.erb
Original file line number Diff line number Diff line change
Expand Up @@ -577,33 +577,48 @@ var page_options = {
},

page_part_dialog: function(){
var slugOptions = {
separator: '_'
}

$('#new_page_part_dialog').dialog({
title: 'Create Content Section',
modal: true,
resizable: false,
autoOpen: false,
width: 600,
height: 200
height: 300
});

$('#add_page_part').click(function(e){
e.preventDefault();
$('#new_page_part_dialog').dialog('open');
});

$('#new_page_part_title').on('input propertychange', function() {
var part_title = $(this).val();

if(part_title.length > 0) {
var part_slug = getSlug(part_title, slugOptions);
$('#new_page_part_slug').val(part_slug);
}
});

$('#new_page_part_save').click(function(e){
e.preventDefault();

var part_title = $('#new_page_part_title').val();
var part_slug = $('#new_page_part_slug').val();

if(part_title.length > 0){
var tab_title = part_title.toLowerCase().replace(" ", "_");
if(part_slug.length > 0){
var tab_slug = getSlug(part_slug, slugOptions);

if ($('#page_part_' + tab_title).size() === 0) {
if ($('#page_part_' + tab_slug).size() === 0) {
$.get(page_options.new_part_url, {
title: part_title
, part_index: $('#new_page_part_index').val()
, body: ''
title: part_title,
part_index: $('#new_page_part_index').val(),
slug: tab_slug,
body: ''
}, function(data, status){
$('#submit_continue_button').remove();
// Add a new tab for the new content section.
Expand All @@ -622,22 +637,24 @@ var page_options = {
// Wipe the title and increment the index counter by one.
$('#new_page_part_index').val(parseInt($('#new_page_part_index').val(), 10) + 1);
$('#new_page_part_title').val('');
$('#new_page_part_slug').val('');

$('#new_page_part_dialog').dialog('close');
}, 'html'
);
}else{
alert("A content section with that title already exists, please choose another.");
alert("A content section with that slug already exists, please choose another.");
}
}else{
alert("You have not entered a title for the content section, please enter one.");
alert("You have not entered a slug for the content section, please enter one.");
}
});

$('#new_page_part_cancel').click(function(e){
e.preventDefault();
$('#new_page_part_dialog').dialog('close');
$('#new_page_part_title').val('');
$('#new_page_part_slug').val('');
});

$('#delete_page_part').click(function(e){
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
page = Refinery::Page.create :title => "Add Image to me"
# we need page parts so that there's a visual editor
Refinery::Pages.default_parts.each_with_index do |default_page_part, index|
page.parts.create(:title => default_page_part, :body => nil, :position => index)
page.parts.create(:title => default_page_part, :slug => default_page_part, :body => nil, :position => index)
end
page
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ class PagePartsController < ::Refinery::AdminController

def new
render :partial => '/refinery/admin/pages/page_part_field', :locals => {
:part => ::Refinery::PagePart.new(:title => params[:title], :body => params[:body]),
:part => ::Refinery::PagePart.new(new_page_part_params),
:new_part => true,
:part_index => params[:part_index]
}
Expand All @@ -21,6 +21,11 @@ def destroy
end
end

protected
def new_page_part_params
params.permit(:title, :slug, :body)
end

end
end
end
4 changes: 2 additions & 2 deletions pages/app/controllers/refinery/admin/pages_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ class PagesController < Refinery::AdminController
def new
@page = Page.new(new_page_params)
Pages.default_parts_for(@page).each_with_index do |page_part, index|
@page.parts << PagePart.new(:title => page_part, :position => index)
@page.parts << PagePart.new(:title => page_part, :slug => page_part, :position => index)
end
end

Expand Down Expand Up @@ -103,7 +103,7 @@ def permitted_page_params
[
:browser_title, :draft, :link_url, :menu_title, :meta_description,
:parent_id, :skip_to_first_child, :show_in_menu, :title, :view_template,
:layout_template, :custom_slug, parts_attributes: [:id, :title, :body, :position]
:layout_template, :custom_slug, parts_attributes: [:id, :title, :slug, :body, :position]
]
end

Expand Down
25 changes: 17 additions & 8 deletions pages/app/models/refinery/page.rb
Original file line number Diff line number Diff line change
Expand Up @@ -291,8 +291,12 @@ def to_refinery_menu_item
# ::Refinery::Page.first.content_for(:body)
#
# Will return the body page part of the first page.
def content_for(part_title)
part_with_title(part_title).try(:body)
def content_for(part_slug)
begin
part_with_slug(part_slug).try(:body)
rescue
raise ActiveRecord::RecordNotFound
end
end

# Accessor method to test whether a page part
Expand All @@ -302,24 +306,29 @@ def content_for(part_title)
# ::Refinery::Page.first.content_for?(:body)
#
# Will return true if the page has a body page part and it is not blank.
def content_for?(part_title)
content_for(part_title).present?
def content_for?(part_slug)
content_for(part_slug).present?
end

# Accessor method to get a page part object from a page.
# Example:
#
# ::Refinery::Page.first.part_with_title(:body)
# ::Refinery::Page.first.part_with_slug(:body)
#
# Will return the Refinery::PagePart object with that title using the first page.
def part_with_title(part_title)
# Will return the Refinery::PagePart object with that slug using the first page.
def part_with_slug(part_slug)
# self.parts is usually already eager loaded so we can now just grab
# the first element matching the title we specified.
self.parts.detect do |part|
part.title_matches?(part_title)
part.slug_matches?(part_slug)
end
end

def part_with_title(part_title)
Refinery.deprecate("Refinery::Page#part_with_title", when: "3.1", replacement: "part_with_slug")
part_with_slug(part_title.to_s.parameterize.underscore)
end

private

class FriendlyIdPath
Expand Down
27 changes: 20 additions & 7 deletions pages/app/models/refinery/page_part.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,16 @@ class PagePart < Refinery::Core::BaseModel

belongs_to :page, :foreign_key => :refinery_page_id

validates :title, :presence => true, :uniqueness => {:scope => :refinery_page_id}
before_validation :set_default_slug

validates :title, :presence => true
validates :slug, :presence => true, :uniqueness => {:scope => :refinery_page_id}
alias_attribute :content, :body

translates :body

def to_param
"page_part_#{title.downcase.gsub(/\W/, '_')}"
"page_part_#{slug.downcase.gsub(/\W/, '_')}"
end

def body=(value)
Expand All @@ -18,10 +21,16 @@ def body=(value)
normalise_text_fields
end

def slug_matches?(other_slug)
slug.present? && (# protecting against the problem that occurs when have two nil slugs
slug == other_slug.to_s ||
parameterized_slug == parameterize(other_slug.to_s)
)
end

def title_matches?(other_title)
title.present? and # protecting against the problem that occurs when have nil title
title == other_title.to_s or
parameterized_title == parameterize(other_title.to_s)
Refinery.deprecate("Refinery::PagePart#title_matches?", when: "3.1", replacement: "slug_matches?")
slug_matches?(other_title.to_s.parameterize.underscore)
end

protected
Expand All @@ -37,8 +46,12 @@ def parameterize(string)
string.downcase.gsub(" ", "_")
end

def parameterized_title
parameterize(title)
def parameterized_slug
parameterize(slug)
end

def set_default_slug
self.slug = title.to_s.parameterize.underscore.presence if self.slug.blank?
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@
<div class='field'>
<label for='new_page_part_title'><%= t('.title') %></label>
<input id='new_page_part_title' value='' class='larger widest' />

<label for='new_page_part_slug'><%= t('.slug') %></label>
<input id='new_page_part_slug' value='' class='larger widest' />

<input id='new_page_part_index' type='hidden' value='<%= @page.parts.size %>' />
</div>
<%= render '/refinery/admin/form_actions', :f => f,
Expand All @@ -10,3 +14,7 @@
:cancel_button_id => "new_page_part_cancel",
:hide_delete => true %>
</div>

<% content_for :after_javascript_libraries do %>
<%= javascript_include_tag 'speakingurl' %>
<% end %>
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<div class='page_part' id='<%= new_part ? "page_part_new_#{part_index}" : part.to_param %>'>
<%= hidden_field_tag "page[parts_attributes][#{part_index}][title]", part.title if new_part %>
<%= hidden_field_tag "page[parts_attributes][#{part_index}][slug]", part.slug if new_part %>
<%= text_area_tag "page[parts_attributes][#{part_index}][body]", part.body, :rows => 20, :class => 'visual_editor widest' %>
<%= hidden_field_tag "page[parts_attributes][#{part_index}][position]", part_index %>
</div>
1 change: 1 addition & 0 deletions pages/config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ en:
preview_changes: Preview your changes before making them live
form_new_page_parts:
title: Title
slug: Slug
form_page_parts:
create_content_section: Add content section
delete_content_section: Delete content section
Expand Down
1 change: 1 addition & 0 deletions pages/config/locales/fr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ fr:
preview_changes: Prévisualiser vos changements avant de les appliquer
form_new_page_parts:
title: Titre
slug: Slug
form_page_parts:
create_content_section: Créer une nouvelle section de contenu
delete_content_section: Supprimer une section de contenu
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
class AddSlugToRefineryPageParts < ActiveRecord::Migration
def change
rename_column :refinery_page_parts, :title, :slug
add_column :refinery_page_parts, :title, :string
end
end
1 change: 1 addition & 0 deletions pages/lib/refinery/pages.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,3 +51,4 @@ module Admin
require 'friendly_id'
require 'seo_meta'
require 'babosa'
require 'speakingurl-rails'
7 changes: 6 additions & 1 deletion pages/lib/refinery/pages/engine.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,17 @@ class Engine < ::Rails::Engine
add_route_parts_as_reserved_words if Refinery::Pages.marketable_urls
end

config.to_prepare do
Rails.application.config.assets.precompile += %w(
speakingurl.js
)
end

config.after_initialize do
Refinery.register_extension(Refinery::Pages)
end

protected

def append_marketable_routes
Refinery::Core::Engine.routes.append do
get '*path', :to => 'pages#show', :as => :marketable_page
Expand Down
1 change: 1 addition & 0 deletions pages/refinerycms-pages.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ Gem::Specification.new do |s|
s.add_dependency 'seo_meta', '~> 2.0.0.rc.1'
s.add_dependency 'refinerycms-core', version
s.add_dependency 'babosa', '!= 0.3.6'
s.add_dependency 'speakingurl-rails', '~> 1.1.2'

s.required_ruby_version = Refinery::Version.required_ruby_version
end
7 changes: 4 additions & 3 deletions pages/spec/features/refinery/admin/pages_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -649,9 +649,9 @@ module Admin
let!(:some_page) { Page.create! :title => "Some Page" }

before do
some_page.parts.create! :title => "First Part", :position => 1
some_page.parts.create! :title => "Second Part", :position => 2
some_page.parts.create! :title => "Third Part", :position => 3
some_page.parts.create! :title => "First Part", :slug => "first_part", :position => 1
some_page.parts.create! :title => "Second Part", :slug => "second_part", :position => 2
some_page.parts.create! :title => "Third Part", :slug => "third_part", :position => 3

allow(Refinery::Pages).to receive(:new_page_parts).and_return(true)
end
Expand Down Expand Up @@ -727,6 +727,7 @@ module Admin
page = Refinery::Page.create! :title => "test"
Refinery::Pages.default_parts.each_with_index do |default_page_part, index|
page.parts.create(:title => default_page_part,
:slug => default_page_part,
:body => "<header class='regression'>test</header>",
:position => index)
end
Expand Down
20 changes: 10 additions & 10 deletions pages/spec/models/refinery/page_part_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,28 +7,28 @@ module Refinery
let(:page) { subject.class.new(:title => page_title, :deletable => true)}

before do
page.parts.new(:title => 'body', :content => "I'm the first page part for this page.", :position => 0)
page.parts.new(:title => 'side body', :content => 'Closely followed by the second page part.', :position => 1)
page.parts.new(:title => 'body', :slug => 'body', :content => "I'm the first page part for this page.", :position => 0)
page.parts.new(:title => 'side body', :slug => 'side_body', :content => 'Closely followed by the second page part.', :position => 1)
end

it 'return the content when using content_for' do
expect(page.content_for(:body)).to eq("<p>I'm the first page part for this page.</p>")
expect(page.content_for('BoDY')).to eq("<p>I'm the first page part for this page.</p>")
end

it 'requires a unique title' do
it 'requires a unique slug' do
page.save
page.parts.create(:title => 'body')
duplicate_title_part = page.parts.create(:title => 'body')
page.parts.create(:title => 'body', :slug => 'body')
duplicate_title_part = page.parts.create(:title => 'body', :slug => 'body')

expect(duplicate_title_part.errors[:title]).to be_present
expect(duplicate_title_part.errors[:slug]).to be_present
end

it 'only requires a unique title on the same page' do
part_one = Page.create(:title => 'first page').parts.create(:title => 'body')
part_two = Page.create(:title => 'second page').parts.create(:title => 'body')
it 'only requires a unique slug on the same page' do
part_one = Page.create(:title => 'first page', :slug => 'first_page').parts.create(:title => 'body', :slug => 'body')
part_two = Page.create(:title => 'second page', :slug => 'second_page').parts.create(:title => 'body', :slug => 'body')

expect(part_two.errors[:title]).to be_empty
expect(part_two.errors[:slug]).to be_empty
end

context 'when using content_for?' do
Expand Down

0 comments on commit 040300c

Please sign in to comment.