Skip to content

Commit

Permalink
Do not pass options params around anymore
Browse files Browse the repository at this point in the history
This was only done because we supported to have local options in
the element editor and essence editor partials. We do not support
this anymore and expect to use static content.settings instead.
  • Loading branch information
tvdeyen committed Oct 16, 2019
1 parent 161ce9b commit 014c7e2
Show file tree
Hide file tree
Showing 13 changed files with 17 additions and 69 deletions.
14 changes: 2 additions & 12 deletions app/controllers/alchemy/admin/base_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@ class BaseController < Alchemy::BaseController
before_action { enforce_ssl if ssl_required? && !request.ssl? }
before_action :load_locked_pages

helper_method :clipboard_empty?, :trash_empty?, :get_clipboard, :is_admin?,
:options_from_params
helper_method :clipboard_empty?, :trash_empty?, :get_clipboard, :is_admin?

check_authorization

Expand Down Expand Up @@ -128,16 +127,7 @@ def do_redirect_to(url_or_path)
end
end

# Extracts options from params and permits all keys
#
# If no options are present it returns an empty parameters hash.
#
# @returns [ActionController::Parameters]
def options_from_params
@_options_from_params ||= begin
(params[:options] || ActionController::Parameters.new).permit!
end
end
deprecate :options_from_params, deprecator: Alchemy::Deprecation

# This method decides if we want to raise an exception or not.
#
Expand Down
2 changes: 0 additions & 2 deletions app/controllers/alchemy/admin/resources_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,6 @@ def search_filter_params

def common_search_filter_includes
[
# contrary to Rails' documentation passing an empty hash to permit all keys does not work
{options: options_from_params.keys},
{q: [
resource_handler.search_field_name,
:s
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@
in_dialog: true,
redirect_url: admin_attachments_path(
element_id: @element.try(:id),
content_id: @content.try(:id),
options: options_from_params
content_id: @content.try(:id)
) %>
<% end %>
<%= render 'alchemy/admin/partials/remote_search_form' %>
Expand Down
3 changes: 1 addition & 2 deletions app/views/alchemy/admin/attachments/_file_to_assign.html.erb
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
<li class="assign_file_file <%= cycle('even', 'odd') %>">
<%= link_to alchemy.assign_admin_essence_files_path(
attachment_id: file_to_assign.id,
content_id: @content.id,
options: options_from_params
content_id: @content.id
),
remote: true,
method: 'put' do %>
Expand Down
3 changes: 1 addition & 2 deletions app/views/alchemy/admin/essence_files/assign.js.erb
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
$('#<%= @content.dom_id %>').replaceWith('<%= j(
render "alchemy/essences/essence_file_editor",
formats: [:html],
content: @content,
options: options_from_params
content: @content
) %>');
Alchemy.closeCurrentDialog();
Alchemy.setElementDirty($('#element_<%= @content.element.id %>'));
3 changes: 1 addition & 2 deletions app/views/alchemy/admin/essence_pictures/assign.js.erb
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@ $('#picture_to_assign_<%= @picture.id %> a').attr('href', '#').off('click');
$('#<%= @content.dom_id -%>').replaceWith('<%= j(
render(
"alchemy/essences/essence_picture_editor",
content: @content,
options: options_from_params
content: @content
)
) %>');
Alchemy.closeCurrentDialog();
Expand Down
14 changes: 7 additions & 7 deletions app/views/alchemy/admin/essence_pictures/edit.html.erb
Original file line number Diff line number Diff line change
@@ -1,20 +1,20 @@
<%= alchemy_form_for @essence_picture,
url: alchemy.admin_essence_picture_path(@essence_picture, options: options_from_params) do |f| %>
url: alchemy.admin_essence_picture_path(@essence_picture) do |f| %>
<%= hidden_field_tag 'content_id', @content.id %>
<%= f.input :caption, as: options_from_params[:caption_as_textarea] ? 'text' : 'string' %>
<%= f.input :caption, as: @content.settings[:caption_as_textarea] ? 'text' : 'string' %>
<%= f.input :title %>
<%= f.input :alt_tag %>
<%- if options_from_params[:sizes].present? -%>
<%- if @content.settings[:sizes].present? -%>
<%= f.input :render_size,
collection: [
[Alchemy.t('Layout default'), options_from_params[:size]]
] + options_from_params[:sizes].to_a,
[Alchemy.t('Layout default'), @content.settings[:size]]
] + @content.settings[:sizes].to_a,
include_blank: false,
input_html: {class: 'alchemy_selectbox'} %>
<%- end -%>
<%- if options_from_params[:css_classes].present? -%>
<%- if @content.settings[:css_classes].present? -%>
<%= f.input :css_class,
collection: options_from_params[:css_classes],
collection: @content.settings[:css_classes],
include_blank: Alchemy.t('None'),
input_html: {class: 'alchemy_selectbox'} %>
<%- else -%>
Expand Down
2 changes: 0 additions & 2 deletions app/views/alchemy/admin/partials/_remote_search_form.html.erb
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
<%= search_form_for @query, url: url_for(
action: 'index',
options: options_from_params,
size: @size
), remote: true, html: {class: 'search_form', id: nil} do |f| %>
<%= hidden_field_tag("element_id", @element.blank? ? "" : @element.id) %>
Expand All @@ -17,7 +16,6 @@
action: 'index',
element_id: @element.blank? ? '' : @element.id,
content_id: @content.blank? ? '' : @content.id,
options: options_from_params,
size: @size,
overlay: true
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@
size: search_filter_params[:size],
filter: 'last_upload',
content_id: @content.try(:id),
element_id: @element.try(:id),
options: options_from_params
element_id: @element.try(:id)
) %>
<div class="toolbar_spacer"></div>
<% end %>
Expand All @@ -23,7 +22,6 @@
content_id: @content,
element_id: @element,
q: search_filter_params[:q],
options: options_from_params,
filter: search_filter_params[:filter],
tagged_with: search_filter_params[:tagged_with]
}),
Expand All @@ -40,7 +38,6 @@
content_id: @content,
element_id: @element,
q: search_filter_params[:q],
options: options_from_params,
filter: search_filter_params[:filter],
tagged_with: search_filter_params[:tagged_with]
}),
Expand All @@ -57,7 +54,6 @@
content_id: @content,
element_id: @element,
q: search_filter_params[:q],
options: options_from_params,
filter: search_filter_params[:filter],
tagged_with: search_filter_params[:tagged_with]
}),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,6 @@
<% else %>
<%= render partial: 'picture_to_assign',
collection: @pictures,
locals: {options: options_from_params, size: @size} %>
locals: {size: @size} %>
<%= paginate @pictures, theme: 'alchemy', remote: true, hide_per_page_select: true %>
<% end %>
3 changes: 1 addition & 2 deletions app/views/alchemy/admin/pictures/_picture_to_assign.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@
),
alchemy.assign_admin_essence_pictures_path(
picture_id: picture_to_assign.id,
content_id: @content,
options: options
content_id: @content
),
remote: true,
onclick: '$(self).attr("href", "#").off("click"); return false',
Expand Down
2 changes: 1 addition & 1 deletion config/brakeman.ignore
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
"file": "app/views/alchemy/admin/contents/create.js.erb",
"line": 1,
"link": "http://brakemanscanner.org/docs/warning_types/dynamic_render_path/",
"code": "render(action => \"alchemy/essences/#{Content.create(Element.find(params[:content][:element_id]), content_params).essence_partial_name}_editor\", { :content => Content.create(Element.find(params[:content][:element_id]), content_params), :options => options_from_params, :html_options => ((params[:html_options] or {})) })",
"code": "render(action => \"alchemy/essences/#{Content.create(Element.find(params[:content][:element_id]), content_params).essence_partial_name}_editor\", { :content => Content.create(Element.find(params[:content][:element_id]), content_params) })",
"render_path": [{"type":"controller","class":"Alchemy::Admin::ContentsController","method":"create","line":21,"file":"app/controllers/alchemy/admin/contents_controller.rb"}],
"location": {
"type": "template",
Expand Down
29 changes: 0 additions & 29 deletions spec/controllers/alchemy/admin/base_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,35 +3,6 @@
require 'rails_helper'

describe Alchemy::Admin::BaseController do
describe '#options_from_params' do
subject { controller.send(:options_from_params) }

before do
expect(controller).to receive(:params).at_least(:once) do
ActionController::Parameters.new(options: options)
end
end

context "params[:options] are Rails parameters" do
let(:options) do
ActionController::Parameters.new('hello' => 'world')
end

it "returns the options as permitted parameters with indifferent access" do
expect(subject).to be_permitted
expect(subject[:hello]).to eq('world')
end
end

context "params[:options] is nil" do
let(:options) { nil }

it "returns an empty permitted parameters hash" do
is_expected.to eq(ActionController::Parameters.new.permit!)
end
end
end

describe '#raise_exception?' do
subject { controller.send(:raise_exception?) }

Expand Down

0 comments on commit 014c7e2

Please sign in to comment.