Move logic from key definition to views

Before this change, two important things depend on the format of each key,
where to render it in the administration panel and which kind of interface
to use for each setting. Following this strategy led us to a very complex
code, very difficult to maintain or modify. So, we do not want to depend
on the setting key structure anymore to decide how or where to render each
setting.

With this commit, we get rid of the key format-based rules. Now we render
each setting explicitly passing to it the type and the tab where it belongs.
This commit is contained in:
Senén Rodero Rodríguez
2023-12-07 17:06:42 +01:00
committed by Javi Martín
parent 91c3bde36b
commit f8835debae
19 changed files with 157 additions and 150 deletions

View File

@@ -0,0 +1,18 @@
<tr>
<td>
<strong id="<%= dom_id(setting, :title) %>"><%= t("settings.#{setting.key}") %></strong>
<br>
<span id="<%= dom_id(setting, :description) %>" class="small">
<%= t("settings.#{setting.key}_description", default: t("admin.settings.no_description")) %>
</span>
</td>
<td>
<% if content_type_setting? %>
<%= render Admin::Settings::ContentTypesFormComponent.new(setting, tab: tab) %>
<% elsif featured_setting? %>
<%= render Admin::Settings::FeaturedSettingsFormComponent.new(setting, tab: tab) %>
<% else %>
<%= render Admin::Settings::TextFormComponent.new(setting, tab: tab) %>
<% end %>
</td>
</tr>

View File

@@ -0,0 +1,22 @@
class Admin::Settings::RowComponent < ApplicationComponent
attr_reader :key, :tab, :type
delegate :dom_id, to: :helpers
def initialize(key, type: :text, tab: nil)
@key = key
@type = type
@tab = tab
end
def setting
@setting ||= Setting.find_by!(key: key)
end
def content_type_setting?
type == :content_type
end
def featured_setting?
type == :feature
end
end

View File

@@ -6,25 +6,6 @@
</tr> </tr>
</thead> </thead>
<tbody> <tbody>
<% settings.each do |setting| %> <%= content %>
<tr>
<td>
<strong id="<%= dom_id(setting, :title) %>"><%= t("settings.#{setting.key}") %></strong>
<br>
<span id="<%= dom_id(setting, :description) %>" class="small">
<%= t("settings.#{setting.key}_description", default: t("admin.settings.no_description")) %>
</span>
</td>
<td>
<% if setting.content_type? %>
<%= render Admin::Settings::ContentTypesFormComponent.new(setting, tab: tab) %>
<% elsif setting.feature? %>
<%= render Admin::Settings::FeaturedSettingsFormComponent.new(setting, tab: tab) %>
<% else %>
<%= render Admin::Settings::TextFormComponent.new(setting, tab: tab) %>
<% end %>
</td>
</tr>
<% end %>
</tbody> </tbody>
</table> </table>

View File

@@ -1,11 +1,9 @@
class Admin::Settings::TableComponent < ApplicationComponent class Admin::Settings::TableComponent < ApplicationComponent
attr_reader :settings, :setting_name, :tab attr_reader :setting_name, :table_class
delegate :dom_id, to: :helpers
def initialize(settings:, setting_name:, tab: nil) def initialize(setting_name:, table_class: "mixed-settings-table")
@settings = settings
@setting_name = setting_name @setting_name = setting_name
@tab = tab @table_class = table_class
end end
def key_header def key_header
@@ -25,12 +23,4 @@ class Admin::Settings::TableComponent < ApplicationComponent
t("admin.settings.setting_value") t("admin.settings.setting_value")
end end
end end
def table_class
if settings.all?(&:feature?)
"featured-settings-table"
else
"mixed-settings-table"
end
end
end end

View File

@@ -1,16 +1,5 @@
class Admin::SettingsController < Admin::BaseController class Admin::SettingsController < Admin::BaseController
def index def index
all_settings = Setting.all.group_by(&:type)
@configuration_settings = all_settings["configuration"]
@feature_settings = all_settings["feature"]
@participation_processes_settings = all_settings["process"]
@map_configuration_settings = all_settings["map"]
@proposals_settings = all_settings["proposals"]
@remote_census_general_settings = all_settings["remote_census.general"]
@remote_census_request_settings = all_settings["remote_census.request"]
@remote_census_response_settings = all_settings["remote_census.response"]
@uploads_settings = all_settings["uploads"]
@sdg_settings = all_settings["sdg"]
end end
def update def update

View File

@@ -9,8 +9,6 @@ class Admin::SiteCustomization::ContentBlocksController < Admin::SiteCustomizati
def index def index
@content_blocks = SiteCustomization::ContentBlock.order(:name, :locale) @content_blocks = SiteCustomization::ContentBlock.order(:name, :locale)
@headings_content_blocks = Budget::ContentBlock.all @headings_content_blocks = Budget::ContentBlock.all
all_settings = Setting.all.group_by(&:type)
@html_settings = all_settings["html"]
end end
def create def create

View File

@@ -11,14 +11,6 @@ class Setting < ApplicationRecord
value.present? value.present?
end end
def content_type?
key.split(".").last == "content_types"
end
def feature?
%w[feature process sdg].include?(prefix)
end
def content_type_group def content_type_group
key.split(".").second key.split(".").second
end end

View File

@@ -1,3 +1,14 @@
<h2><%= t("admin.settings.index.general") %></h2> <h2><%= t("admin.settings.index.general") %></h2>
<%= render Admin::Settings::TableComponent.new(settings: @configuration_settings, setting_name: "setting", tab: "#tab-configuration") %> <%= render Admin::Settings::TableComponent.new(setting_name: "setting") do %>
<% %w[official_level_1_name official_level_2_name official_level_3_name official_level_4_name
official_level_5_name max_ratio_anon_votes_on_debates max_votes_for_debate_edit
max_votes_for_proposal_edit comments_body_max_length proposal_code_prefix votes_for_proposal_success
months_to_archive_proposals email_domain_for_officials facebook_handle instagram_handle
telegram_handle twitter_handle twitter_hashtag youtube_handle org_name meta_title meta_description
meta_keywords proposal_notification_minimum_interval_in_days direct_message_max_per_day mailer_from_name
mailer_from_address min_age_to_participate hot_score_period_in_days related_content_score_threshold
featured_proposals_number feature.dashboard.notification_emails postal_codes].each do |key| %>
<%= render Admin::Settings::RowComponent.new(key, tab: "#tab-configuration") %>
<% end %>
<% end %>

View File

@@ -1,3 +1,13 @@
<h2><%= t("admin.settings.index.feature_flags") %></h2> <h2><%= t("admin.settings.index.feature_flags") %></h2>
<%= render Admin::Settings::TableComponent.new(settings: @feature_settings, setting_name: "feature", tab: "#tab-feature-flags") %> <%= render Admin::Settings::TableComponent.new(setting_name: "feature", table_class: "featured-settings-table") do %>
<% %w[feature.featured_proposals feature.facebook_login feature.google_login feature.twitter_login
feature.wordpress_login feature.public_stats feature.signature_sheets feature.user.recommendations
feature.user.recommendations_on_debates feature.user.recommendations_on_proposals
feature.user.skip_verification feature.community feature.map feature.allow_attached_documents
feature.allow_images feature.help_page feature.remote_translations feature.translation_interface
feature.remote_census feature.valuation_comment_notification feature.graphql_api feature.sdg
feature.machine_learning feature.remove_investments_supports].each do |key| %>
<%= render Admin::Settings::RowComponent.new(key, type: :feature, tab: "#tab-feature-flags") %>
<% end %>
<% end %>

View File

@@ -1,3 +1,14 @@
<h2><%= t("admin.settings.index.images_and_documents") %></h2> <h2><%= t("admin.settings.index.images_and_documents") %></h2>
<%= render Admin::Settings::TableComponent.new(settings: @uploads_settings, setting_name: "setting", tab: "#tab-images-and-documents") %> <% tab = "#tab-images-and-documents" %>
<%= render Admin::Settings::TableComponent.new(setting_name: "setting") do %>
<%= render Admin::Settings::RowComponent.new("uploads.images.title.min_length", tab: tab) %>
<%= render Admin::Settings::RowComponent.new("uploads.images.title.max_length", tab: tab) %>
<%= render Admin::Settings::RowComponent.new("uploads.images.min_width", tab: tab) %>
<%= render Admin::Settings::RowComponent.new("uploads.images.min_height", tab: tab) %>
<%= render Admin::Settings::RowComponent.new("uploads.images.max_size", tab: tab) %>
<%= render Admin::Settings::RowComponent.new("uploads.images.content_types", type: :content_type, tab: tab) %>
<%= render Admin::Settings::RowComponent.new("uploads.documents.max_amount", tab: tab) %>
<%= render Admin::Settings::RowComponent.new("uploads.documents.max_size", tab: tab) %>
<%= render Admin::Settings::RowComponent.new("uploads.documents.content_types", type: :content_type, tab: tab) %>
<% end %>

View File

@@ -1,11 +1,16 @@
<% if feature?(:map) %> <% if feature?(:map) %>
<h2><%= t("admin.settings.index.map.title") %></h2> <h2><%= t("admin.settings.index.map.title") %></h2>
<%= render Admin::Settings::TableComponent.new(settings: @map_configuration_settings, setting_name: "setting", tab: "#tab-map-configuration") %> <% tab = "#tab-map-configuration" %>
<%= render Admin::Settings::TableComponent.new(setting_name: "setting") do %>
<% %w[map.latitude map.longitude map.zoom].each do |key| %>
<%= render Admin::Settings::RowComponent.new(key, tab: tab) %>
<% end %>
<% end %>
<p><%= t("admin.settings.index.map.help") %></p> <p><%= t("admin.settings.index.map.help") %></p>
<%= render Admin::Settings::MapFormComponent.new(tab: "#tab-map-configuration") %> <%= render Admin::Settings::MapFormComponent.new(tab: tab) %>
<% else %> <% else %>
<div class="callout primary"> <div class="callout primary">
<%= t("admin.settings.index.map.how_to_enable") %> <%= t("admin.settings.index.map.how_to_enable") %>

View File

@@ -1,3 +1,7 @@
<h2><%= t("admin.settings.index.participation_processes") %></h2> <h2><%= t("admin.settings.index.participation_processes") %></h2>
<%= render Admin::Settings::TableComponent.new(settings: @participation_processes_settings, setting_name: "feature", tab: "#tab-participation-processes") %> <%= render Admin::Settings::TableComponent.new(setting_name: "feature", table_class: "featured-settings-table") do %>
<% %w[process.debates process.proposals process.polls process.budgets process.legislation].each do |key| %>
<%= render Admin::Settings::RowComponent.new(key, type: :feature, tab: "#tab-participation-processes") %>
<% end %>
<% end %>

View File

@@ -1,3 +1,9 @@
<h2><%= t("admin.settings.index.dashboard.title") %></h2> <h2><%= t("admin.settings.index.dashboard.title") %></h2>
<%= render Admin::Settings::TableComponent.new(settings: @proposals_settings, setting_name: "setting", tab: "#tab-proposals") %> <%= render Admin::Settings::TableComponent.new(setting_name: "setting") do %>
<% %w[proposals.successful_proposal_id proposals.poll_short_title proposals.poll_description
proposals.poll_link proposals.email_short_title proposals.email_description
proposals.poster_short_title proposals.poster_description].each do |key| %>
<%= render Admin::Settings::RowComponent.new(key, tab: "#tab-proposals") %>
<% end %>
<% end %>

View File

@@ -1,9 +1,24 @@
<% if feature?(:remote_census) %> <% if feature?(:remote_census) %>
<h2><%= t("admin.settings.index.remote_census.title") %></h2> <h2><%= t("admin.settings.index.remote_census.title") %></h2>
<%= render Admin::Settings::TableComponent.new(settings: @remote_census_general_settings, setting_name: "remote_census_general_name", tab: "#tab-remote-census-configuration") %> <% tab = "#tab-remote-census-configuration" %>
<%= render Admin::Settings::TableComponent.new(settings: @remote_census_request_settings, setting_name: "remote_census_request_name", tab: "#tab-remote-census-configuration") %> <%= render Admin::Settings::TableComponent.new(setting_name: "remote_census_general_name") do %>
<%= render Admin::Settings::TableComponent.new(settings: @remote_census_response_settings, setting_name: "remote_census_response_name", tab: "#tab-remote-census-configuration") %> <%= render Admin::Settings::RowComponent.new("remote_census.general.endpoint", tab: tab) %>
<% end %>
<%= render Admin::Settings::TableComponent.new(setting_name: "remote_census_request_name") do %>
<% %w[remote_census.request.method_name remote_census.request.structure
remote_census.request.document_type remote_census.request.document_number
remote_census.request.date_of_birth remote_census.request.postal_code].each do |key| %>
<%= render Admin::Settings::RowComponent.new(key, tab: tab) %>
<% end %>
<% end %>
<%= render Admin::Settings::TableComponent.new(setting_name: "remote_census_response_name") do %>
<% %w[remote_census.response.date_of_birth remote_census.response.postal_code
remote_census.response.district remote_census.response.gender remote_census.response.name
remote_census.response.surname remote_census.response.valid].each do |key| %>
<%= render Admin::Settings::RowComponent.new(key, tab: tab) %>
<% end %>
<% end %>
<% else %> <% else %>
<div class="callout primary"> <div class="callout primary">
<%= t("admin.settings.index.remote_census.how_to_enable") %> <%= t("admin.settings.index.remote_census.how_to_enable") %>

View File

@@ -1,7 +1,11 @@
<% if feature?(:sdg) %> <% if feature?(:sdg) %>
<h2><%= t("admin.settings.index.sdg.title") %></h2> <h2><%= t("admin.settings.index.sdg.title") %></h2>
<%= render Admin::Settings::TableComponent.new(settings: @sdg_settings, setting_name: "feature", tab: "#tab-sdg-configuration") %> <%= render Admin::Settings::TableComponent.new(setting_name: "feature", table_class: "featured-settings-table") do %>
<% %w[sdg.process.debates sdg.process.proposals sdg.process.polls sdg.process.budgets sdg.process.legislation].each do |key| %>
<%= render Admin::Settings::RowComponent.new(key, type: :feature, tab: "#tab-sdg-configuration") %>
<% end %>
<% end %>
<% else %> <% else %>
<div class="callout primary"> <div class="callout primary">
<%= t("admin.settings.index.sdg.how_to_enable") %> <%= t("admin.settings.index.sdg.how_to_enable") %>

View File

@@ -6,7 +6,10 @@
<h2 class="inline-block"><%= t("admin.site_customization.content_blocks.index.title") %></h2> <h2 class="inline-block"><%= t("admin.site_customization.content_blocks.index.title") %></h2>
<%= render Admin::Settings::TableComponent.new(settings: @html_settings, setting_name: "setting") %> <%= render Admin::Settings::TableComponent.new(setting_name: "setting") do %>
<%= render Admin::Settings::RowComponent.new("html.per_page_code_body") %>
<%= render Admin::Settings::RowComponent.new("html.per_page_code_head") %>
<% end %>
<h3><%= t("admin.site_customization.content_blocks.information") %></h3> <h3><%= t("admin.site_customization.content_blocks.information") %></h3>

View File

@@ -3,28 +3,23 @@ require "rails_helper"
describe Admin::Settings::TableComponent do describe Admin::Settings::TableComponent do
describe "#key_header" do describe "#key_header" do
it "returns correct table header for the setting name colums" do it "returns correct table header for the setting name colums" do
settings = Setting.limit(2) render_inline Admin::Settings::TableComponent.new setting_name: "feature"
render_inline Admin::Settings::TableComponent.new settings: settings, setting_name: "feature"
expect(page).to have_content("Feature") expect(page).to have_content("Feature")
render_inline Admin::Settings::TableComponent.new settings: settings, setting_name: "setting" render_inline Admin::Settings::TableComponent.new setting_name: "setting"
expect(page).to have_content("Setting") expect(page).to have_content("Setting")
setting_name = "remote_census_general_name" render_inline Admin::Settings::TableComponent.new setting_name: "remote_census_general_name"
render_inline Admin::Settings::TableComponent.new settings: settings, setting_name: setting_name
expect(page).to have_content("General Information") expect(page).to have_content("General Information")
setting_name = "remote_census_request_name" render_inline Admin::Settings::TableComponent.new setting_name: "remote_census_request_name"
render_inline Admin::Settings::TableComponent.new settings: settings, setting_name: setting_name
expect(page).to have_content("Request Data") expect(page).to have_content("Request Data")
setting_name = "remote_census_response_name" render_inline Admin::Settings::TableComponent.new setting_name: "remote_census_response_name"
render_inline Admin::Settings::TableComponent.new settings: settings, setting_name: setting_name
expect(page).to have_content("Response Data") expect(page).to have_content("Response Data")
end end
@@ -32,35 +27,28 @@ describe Admin::Settings::TableComponent do
describe "#value_header" do describe "#value_header" do
it "returns correct table header for the setting interface column" do it "returns correct table header for the setting interface column" do
settings = Setting.limit(2) render_inline Admin::Settings::TableComponent.new setting_name: "feature"
render_inline Admin::Settings::TableComponent.new settings: settings, setting_name: "feature"
expect(page).to have_content("Enabled") expect(page).to have_content("Enabled")
render_inline Admin::Settings::TableComponent.new settings: settings, setting_name: "setting" render_inline Admin::Settings::TableComponent.new setting_name: "setting"
expect(page).to have_content("Value") expect(page).to have_content("Value")
end end
end end
describe "#table_class" do describe "#table_class" do
it "returns a CSS class when all given settings are features, otherwise returns a mixed class" do it "returns the `mixed-settings-table` by default" do
settings = [Setting.find_by(key: "feature.map"), Setting.find_by(key: "process.debates")] render_inline Admin::Settings::TableComponent.new setting_name: "feature"
render_inline Admin::Settings::TableComponent.new settings: settings, setting_name: "feature" expect(page).to have_css(".mixed-settings-table")
expect(page).to have_css(".featured-settings-table")
expect(page).not_to have_css(".mixed-settings-table")
end end
it "returns a CSS class when all given settings are features, otherwise returns a mixed class" do it "returns the given table_class" do
settings = [Setting.find_by(key: "feature.map"), Setting.find_by(key: "mailer_from_name")] render_inline Admin::Settings::TableComponent.new setting_name: "feature", table_class: "my-table-class"
render_inline Admin::Settings::TableComponent.new settings: settings, setting_name: "feature" expect(page).not_to have_css(".mixed-settings-table")
expect(page).to have_css(".my-table-class")
expect(page).not_to have_css(".featured-settings-table")
expect(page).to have_css(".mixed-settings-table")
end end
end end
end end

View File

@@ -27,15 +27,6 @@ describe Setting do
end end
end end
describe "#feature?" do
it "returns true if the key prefix is process, feature or sdg" do
expect(Setting.find_by!(key: "process.debates").feature?).to be true
expect(Setting.find_by!(key: "feature.map").feature?).to be true
expect(Setting.find_by!(key: "sdg.process.debates").feature?).to be true
expect(Setting.find_by!(key: "uploads.documents.max_size").feature?).to be false
end
end
describe "#enabled?" do describe "#enabled?" do
it "is true if value is present" do it "is true if value is present" do
setting = Setting.create!(key: "feature.whatever", value: 1) setting = Setting.create!(key: "feature.whatever", value: 1)
@@ -57,16 +48,6 @@ describe Setting do
end end
end end
describe "#content_type?" do
it "returns true if the last part of the key is content_types" do
expect(Setting.create!(key: "key_name.content_types").content_type?).to be true
end
it "returns false if the last part of the key is not content_types" do
expect(Setting.create!(key: "key_name.whatever").content_type?).to be false
end
end
describe "#content_type_group" do describe "#content_type_group" do
it "returns the group for content_types settings" do it "returns the group for content_types settings" do
images = Setting.create!(key: "update.images.content_types") images = Setting.create!(key: "update.images.content_types")

View File

@@ -2,24 +2,18 @@ require "rails_helper"
describe "Admin settings", :admin do describe "Admin settings", :admin do
scenario "Index" do scenario "Index" do
create(:setting, key: "super.users.first")
create(:setting, key: "super.users.second")
create(:setting, key: "super.users.third")
visit admin_settings_path visit admin_settings_path
expect(page).to have_content "First" expect(page).to have_content "Level 1 public official"
expect(page).to have_content "Second" expect(page).to have_content "Maximum ratio of anonymous votes per Debate"
expect(page).to have_content "Third" expect(page).to have_content "Comments body max length"
end end
scenario "Update" do scenario "Update" do
create(:setting, key: "super.users.first")
visit admin_settings_path visit admin_settings_path
within "tr", text: "First" do within "tr", text: "Level 1 public official" do
fill_in "First", with: "Super Users of level 1" fill_in "Level 1 public official", with: "Super Users of level 1"
click_button "Update" click_button "Update"
end end
@@ -173,13 +167,11 @@ describe "Admin settings", :admin do
end end
scenario "On #tab-remote-census-configuration" do scenario "On #tab-remote-census-configuration" do
create(:setting, key: "remote_census.general.whatever")
visit admin_settings_path visit admin_settings_path
find("#remote-census-tab").click find("#remote-census-tab").click
within "tr", text: "Whatever" do within "tr", text: "Endpoint" do
fill_in "Whatever", with: "New value" fill_in "Endpoint", with: "example.org/webservice"
click_button "Update" click_button "Update"
end end
@@ -189,13 +181,10 @@ describe "Admin settings", :admin do
end end
scenario "On #tab-configuration" do scenario "On #tab-configuration" do
Setting.create!(key: "whatever")
visit admin_settings_path visit admin_settings_path
find("#tab-configuration").click
within "tr", text: "Whatever" do within "tr", text: "Level 1 public official" do
fill_in "Whatever", with: "New value" fill_in "Level 1 public official", with: "Super Users of level 1"
click_button "Update" click_button "Update"
end end
@@ -209,13 +198,11 @@ describe "Admin settings", :admin do
end end
scenario "On #tab-map-configuration" do scenario "On #tab-map-configuration" do
Setting.create!(key: "map.whatever")
visit admin_settings_path visit admin_settings_path
click_link "Map configuration" click_link "Map configuration"
within "tr", text: "Whatever" do within "tr", text: "Latitude" do
fill_in "Whatever", with: "New value" fill_in "Latitude", with: "-3.636"
click_button "Update" click_button "Update"
end end
@@ -236,13 +223,11 @@ describe "Admin settings", :admin do
end end
scenario "On #tab-proposals" do scenario "On #tab-proposals" do
Setting.create!(key: "proposals.whatever")
visit admin_settings_path visit admin_settings_path
find("#proposals-tab").click find("#proposals-tab").click
within "tr", text: "Whatever" do within "tr", text: "Polls description" do
fill_in "Whatever", with: "New value" fill_in "Polls description", with: "Polls description"
click_button "Update" click_button "Update"
end end
@@ -251,22 +236,18 @@ describe "Admin settings", :admin do
end end
scenario "On #tab-participation-processes" do scenario "On #tab-participation-processes" do
Setting.create!(key: "process.whatever")
visit admin_settings_path visit admin_settings_path
find("#participation-processes-tab").click find("#participation-processes-tab").click
within("tr", text: "Whatever") { click_button "No" } within("tr", text: "Debates") { click_button "Yes" }
expect(page).to have_current_path(admin_settings_path) expect(page).to have_current_path(admin_settings_path)
expect(page).to have_css("div#tab-participation-processes.is-active") expect(page).to have_css("div#tab-participation-processes.is-active")
end end
scenario "On #tab-feature-flags" do scenario "On #tab-feature-flags" do
Setting.create!(key: "feature.whatever")
visit admin_settings_path visit admin_settings_path
find("#features-tab").click find("#features-tab").click
within("tr", text: "Whatever") { click_button "No" } within("tr", text: "Featured proposals") { click_button "No" }
expect(page).to have_current_path(admin_settings_path) expect(page).to have_current_path(admin_settings_path)
expect(page).to have_css("div#tab-feature-flags.is-active") expect(page).to have_css("div#tab-feature-flags.is-active")
@@ -274,15 +255,13 @@ describe "Admin settings", :admin do
scenario "On #tab-sdg-configuration" do scenario "On #tab-sdg-configuration" do
Setting["feature.sdg"] = true Setting["feature.sdg"] = true
Setting.create!(key: "sdg.whatever")
visit admin_settings_path visit admin_settings_path
click_link "SDG configuration" click_link "SDG configuration"
within("tr", text: "Whatever") do within("tr", text: "Related SDG in debates") do
click_button "No" click_button "Yes"
expect(page).to have_button "Yes" expect(page).to have_button "No"
end end
expect(page).to have_current_path(admin_settings_path) expect(page).to have_current_path(admin_settings_path)