Skip to content

Commit

Permalink
Translated root menus (AlchemyCMS#1825)
Browse files Browse the repository at this point in the history
* Add menu_type column to nodes

This column holds information about what type this menu is - a footer or
a main menu, for example.

* Make menu_type available in the admin

This adds the following functionality:

- Choose a menu type when creating a new root node
- Change the menu name for a root node if that's necessary or desired
via `edit`
- Restrict the options for menu type depending on which nodes on a
language are already present

* Render child nodes directly in nodes partial

The "options" local is often not given, and the `node_partial_name` key
is never set any longer, as it is always `node.to_partial_path`.`

* Render footer in Dummy App

This can be used to test Alchemy::EssenceNodes from the Dummy app.
  • Loading branch information
mamhoff authored May 19, 2020
1 parent 6a08040 commit 73963dd
Show file tree
Hide file tree
Showing 23 changed files with 108 additions and 78 deletions.
1 change: 1 addition & 0 deletions app/controllers/alchemy/admin/nodes_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ def new

def resource_params
params.require(:node).permit(
:menu_type,
:parent_id,
:language_id,
:page_id,
Expand Down
12 changes: 6 additions & 6 deletions app/helpers/alchemy/pages_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -78,22 +78,22 @@ def render_site_layout
# Menu partials are placed in the `app/views/alchemy/menus` folder
# Use the `rails g alchemy:menus` generator to create the partials
#
# @param [String] - Name of the menu
# @param [String] - Type of the menu
# @param [Hash] - A set of options available in your menu partials
def render_menu(name, options = {})
def render_menu(menu_type, options = {})
root_node = Alchemy::Node.roots.find_by(
name: name,
menu_type: menu_type,
language: Alchemy::Language.current,
)
if root_node.nil?
warning("Menu with name #{name} not found!")
warning("Menu with type #{menu_type} not found!")
return
end

render("alchemy/menus/#{name}/wrapper", menu: root_node, options: options)
render("alchemy/menus/#{menu_type}/wrapper", menu: root_node, options: options)
rescue ActionView::MissingTemplate => e
warning <<~WARN
Menu partial not found for #{name}.
Menu partial not found for #{menu_type}.
#{e}
WARN
end
Expand Down
4 changes: 4 additions & 0 deletions app/models/alchemy/language.rb
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,10 @@ def matching_locales
end
end

def available_menu_names
Alchemy::Node.available_menu_names - nodes.reject(&:parent_id).map(&:menu_type)
end

private

def set_locale
Expand Down
22 changes: 14 additions & 8 deletions app/models/alchemy/node.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ class Node < BaseRecord

has_many :essence_nodes, class_name: "Alchemy::EssenceNode", foreign_key: :node_id, inverse_of: :ingredient_association

before_validation :translate_root_menu_name, if: -> { root? }
before_validation :set_menu_type_from_root, unless: -> { root? }

validates :menu_type, presence: true
validates :name, presence: true, if: -> { page.nil? }
validates :url, format: { with: VALID_URL_REGEX }, unless: -> { url.nil? }

Expand All @@ -24,11 +28,7 @@ class Node < BaseRecord
# Either the value is stored in the database
# or, if attached, the values comes from a page.
def name
if root?
Alchemy.t(read_attribute(:name), scope: :menu_names)
else
read_attribute(:name).presence || page&.name
end
read_attribute(:name).presence || page&.name
end

class << self
Expand Down Expand Up @@ -74,9 +74,7 @@ def to_partial_path
"alchemy/menus/#{menu_type}/node"
end

def menu_type
root.name.parameterize.underscore
end
private

def check_if_related_essence_nodes_present
dependent_essence_nodes = self_and_descendants.flat_map(&:essence_nodes)
Expand All @@ -85,5 +83,13 @@ def check_if_related_essence_nodes_present
throw(:abort)
end
end

def translate_root_menu_name
self.name ||= Alchemy.t(menu_type, scope: :menu_names)
end

def set_menu_type_from_root
self.menu_type = root.menu_type
end
end
end
32 changes: 18 additions & 14 deletions app/views/alchemy/admin/nodes/_form.html.erb
Original file line number Diff line number Diff line change
@@ -1,21 +1,25 @@
<%= alchemy_form_for([:admin, node]) do |f| %>
<% if node.root? %>
<%= f.input :name,
collection: Alchemy::Node.available_menu_names.map { |n| [I18n.t(n, scope: [:alchemy, :menu_names]), n] },
<% if node.new_record? && node.root? %>
<%= f.input :menu_type,
collection: Alchemy::Language.current.available_menu_names.map { |n| [I18n.t(n, scope: [:alchemy, :menu_names]), n] },
include_blank: false,
input_html: { class: 'alchemy_selectbox' } %>
<% else %>
<%= f.input :name, input_html: {
autofocus: true,
value: node.page && node.read_attribute(:name).blank? ? nil : node.name,
placeholder: node.page ? node.page.name : nil
} %>
<%= f.input :page_id, label: Alchemy::Page.model_name.human, input_html: { class: 'alchemy_selectbox' } %>
<%= f.input :url, input_html: { disabled: node.page }, hint: Alchemy.t(:node_url_hint) %>
<%= f.input :title %>
<%= f.input :nofollow %>
<%= f.input :external %>
<%= f.hidden_field :parent_id %>
<% if node.root? %>
<%= f.input :name %>
<% else %>
<%= f.input :name, input_html: {
autofocus: true,
value: node.page && node.read_attribute(:name).blank? ? nil : node.name,
placeholder: node.page ? node.page.name : nil
} %>
<%= f.input :page_id, label: Alchemy::Page.model_name.human, input_html: { class: 'alchemy_selectbox' } %>
<%= f.input :url, input_html: { disabled: node.page }, hint: Alchemy.t(:node_url_hint) %>
<%= f.input :title %>
<%= f.input :nofollow %>
<%= f.input :external %>
<%= f.hidden_field :parent_id %>
<% end %>
<% end %>
<%= f.hidden_field :language_id %>
<%= f.submit button_label %>
Expand Down
1 change: 1 addition & 0 deletions config/locales/alchemy.en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -801,6 +801,7 @@ en:
alchemy/legacy_page_url:
urlname: "URL path"
alchemy/node:
menu_type: Menu Type
name: "Name"
title: "Title"
nofollow: "Search engine must not follow"
Expand Down
23 changes: 23 additions & 0 deletions db/migrate/20200511113603_add_menu_type_to_alchemy_nodes.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# frozen_string_literal: true
class AddMenuTypeToAlchemyNodes < ActiveRecord::Migration[5.2]
class LocalNode < ActiveRecord::Base
self.table_name = :alchemy_nodes
acts_as_nested_set scope: :language_id

def self.root_for(node)
return node if node.parent_id.nil?

root_for(node.parent)
end
end

def change
add_column :alchemy_nodes, :menu_type, :string
LocalNode.all.each do |node|
root = LocalNode.root_for(node)
menu_type = root.name.parameterize.underscore
node.update(menu_type: menu_type)
end
change_column :alchemy_nodes, :menu_type, :string, null: false
end
end
1 change: 1 addition & 0 deletions lib/alchemy/test_support/factories/node_factory.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
factory :alchemy_node, class: "Alchemy::Node" do
language { Alchemy::Language.default || create(:alchemy_language) }
name { "A Node" }
menu_type { Alchemy::Node.available_menu_names.first }

trait :with_page do
association :page, factory: :alchemy_page
Expand Down
5 changes: 1 addition & 4 deletions lib/generators/alchemy/menus/templates/node.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,7 @@
rel: node.nofollow? ? 'nofollow' : nil %>
<%% if node.children.any? %>
<ul class="dropdown-menu">
<%%= render partial: options[:node_partial_name],
collection: node.children.includes(:page, :children),
locals: { options: options },
as: 'node' %>
<%%= render node.children.includes(:page, :children), as: 'node' %>
</ul>
<%% end %>
<%% end %>
Expand Down
5 changes: 1 addition & 4 deletions lib/generators/alchemy/menus/templates/node.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,4 @@
rel: node.nofollow? ? 'nofollow' : nil
- if node.children.any?
%ul.dropdown-menu
= render partial: options[:node_partial_name],
collection: node.children.includes(:page, :children),
locals: { options: options },
as: 'node'
= render node.children.includes(:page, :children), as: 'node'
5 changes: 1 addition & 4 deletions lib/generators/alchemy/menus/templates/node.html.slim
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,4 @@
rel: node.nofollow? ? 'nofollow' : nil
- if node.children.any?
ul.dropdown-menu
= render partial: options[:node_partial_name],
collection: node.children.includes(:page, :children),
locals: { options: options },
as: 'node'
= render node.children.includes(:page, :children), as: 'node'
2 changes: 1 addition & 1 deletion spec/controllers/alchemy/admin/nodes_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ module Alchemy

it "creates node and redirects to index" do
expect {
post :create, params: { node: { name: "Node", language_id: language.id } }
post :create, params: { node: { menu_type: "main_menu", language_id: language.id } }
}.to change { Alchemy::Node.count }.by(1)
expect(response).to redirect_to(admin_nodes_path)
end
Expand Down
5 changes: 5 additions & 0 deletions spec/dummy/app/views/alchemy/elements/_menu.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<%- cache(menu) do -%>
<%= element_view_for(menu) do |el| -%>
<%= el.render :menu %>
<%- end -%>
<%- end -%>
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,7 @@
rel: node.nofollow? ? 'nofollow' : nil %>
<% if node.children.any? %>
<ul class="dropdown-menu">
<%= render partial: options[:node_partial_name],
collection: node.children.includes(:page, :children),
locals: { options: options },
as: 'node' %>
<%= render node.children.includes(:page, :children), as: 'node' %>
</ul>
<% end %>
<% end %>
5 changes: 1 addition & 4 deletions spec/dummy/app/views/alchemy/menus/main_menu/_node.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,7 @@
rel: node.nofollow? ? 'nofollow' : nil %>
<% if node.children.any? %>
<ul class="dropdown-menu">
<%= render partial: options[:node_partial_name],
collection: node.children.includes(:page, :children),
locals: { options: options },
as: 'node' %>
<%= render node.children.includes(:page, :children), as: 'node' %>
</ul>
<% end %>
<% end %>
4 changes: 4 additions & 0 deletions spec/dummy/app/views/layouts/application.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,9 @@
</nav>
<%= yield %>
<%= render "alchemy/edit_mode" %>

<footer>
<%= render_elements from_page: :footer %>
</footer>
</body>
</html>
2 changes: 1 addition & 1 deletion spec/dummy/config/alchemy/elements.yml
Original file line number Diff line number Diff line change
Expand Up @@ -163,5 +163,5 @@

- name: menu
contents:
- name: Menu
- name: menu
type: EssenceNode
1 change: 1 addition & 0 deletions spec/dummy/db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,7 @@
t.integer "updater_id"
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
t.string "menu_type", null: false
t.index ["creator_id"], name: "index_alchemy_nodes_on_creator_id"
t.index ["language_id"], name: "index_alchemy_nodes_on_language_id"
t.index ["lft"], name: "index_alchemy_nodes_on_lft"
Expand Down
4 changes: 2 additions & 2 deletions spec/features/admin/menus_features_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
it "creates menu" do
visit alchemy.admin_nodes_path

select "Main Menu", from: "Name"
select "Main Menu", from: "Menu Type"
click_button "create"

expect(page).to have_selector(".node_name", text: "Main Menu")
Expand All @@ -33,7 +33,7 @@
it "creates menu for current site" do
visit alchemy.new_admin_node_path

select "Main Menu", from: "Name"
select "Main Menu", from: "Menu Type"
click_button "create"

expect(node.site).to eq(default_site)
Expand Down
2 changes: 1 addition & 1 deletion spec/features/page_feature_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@

describe "navigation rendering" do
context "with menu available" do
let(:menu) { create(:alchemy_node, name: "main_menu") }
let(:menu) { create(:alchemy_node, menu_type: "main_menu") }
let(:page1) { create(:alchemy_page, :public, visible: true, name: "Page 1") }
let(:page2) { create(:alchemy_page, :public, visible: true, name: "Page 2") }
let!(:node1) { create(:alchemy_node, page: page1, parent: menu) }
Expand Down
16 changes: 8 additions & 8 deletions spec/helpers/alchemy/pages_helper_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,12 @@ module Alchemy
end

describe "#render_menu" do
subject { helper.render_menu(name) }
subject { helper.render_menu(menu_type) }

let(:name) { "main_menu" }
let(:menu_type) { "main_menu" }

context "if menu exists" do
let(:menu) { create(:alchemy_node, name: name) }
let(:menu) { create(:alchemy_node, menu_type: menu_type) }
let!(:node) { create(:alchemy_node, parent: menu, url: "/") }

context "and the template exists" do
Expand All @@ -55,7 +55,7 @@ module Alchemy
end

context "but the template does not exist" do
let(:name) { "Unkown" }
let(:menu_type) { "unknown" }

it { is_expected.to be_nil }
end
Expand All @@ -67,9 +67,9 @@ module Alchemy

context "with multiple sites" do
let!(:site_2) { create(:alchemy_site, host: "another-site.com") }
let!(:menu) { create(:alchemy_node, name: name, language: Alchemy::Language.current) }
let!(:menu) { create(:alchemy_node, menu_type: menu_type, language: Alchemy::Language.current) }
let!(:node) { create(:alchemy_node, parent: menu, url: "/default-site") }
let!(:menu_2) { create(:alchemy_node, name: name, language: klingon) }
let!(:menu_2) { create(:alchemy_node, menu_type: menu_type, language: klingon) }
let!(:node_2) { create(:alchemy_node, parent: menu_2, language: klingon, url: "/site-2") }

it "renders menu from current site" do
Expand All @@ -78,9 +78,9 @@ module Alchemy
end

context "with multiple languages" do
let!(:menu) { create(:alchemy_node, name: name) }
let!(:menu) { create(:alchemy_node, menu_type: menu_type) }
let!(:node) { create(:alchemy_node, parent: menu, url: "/default") }
let!(:klingon_menu) { create(:alchemy_node, name: name, language: klingon) }
let!(:klingon_menu) { create(:alchemy_node, menu_type: menu_type, language: klingon) }
let!(:klingon_node) { create(:alchemy_node, parent: klingon_menu, language: klingon, url: "/klingon") }

it "should return the menu for the current language" do
Expand Down
28 changes: 11 additions & 17 deletions spec/models/alchemy/node_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
module Alchemy
describe Node do
it { is_expected.to have_many(:essence_nodes) }
it { is_expected.to respond_to(:menu_type) }

it "is only valid with language and name given" do
expect(Node.new).to be_invalid
Expand Down Expand Up @@ -85,27 +86,20 @@ module Alchemy

describe "#name" do
subject { node.name }
context "root node" do
let(:node) { build_stubbed(:alchemy_node, name: "main_menu") }

it { is_expected.to eq("Main Menu") }
end

context "child node" do
let(:parent) { build_stubbed(:alchemy_node) }
context "with page attached" do
let(:node) { build_stubbed(:alchemy_node, :with_page, parent: parent) }
let(:parent) { build_stubbed(:alchemy_node) }
context "with page attached" do
let(:node) { build_stubbed(:alchemy_node, :with_page, parent: parent) }

it "returns the name from page" do
expect(node.name).to eq(node.page.name)
end
it "returns the name from page" do
expect(node.name).to eq(node.page.name)
end

context "but with name set" do
let(:node) { build_stubbed(:alchemy_node, :with_page, name: "Google", parent: parent) }
context "but with name set" do
let(:node) { build_stubbed(:alchemy_node, :with_page, name: "Google", parent: parent) }

it "still returns the name from name attribute" do
expect(node.name).to eq("Google")
end
it "still returns the name from name attribute" do
expect(node.name).to eq("Google")
end
end
end
Expand Down

0 comments on commit 73963dd

Please sign in to comment.