Add proposal translations

Adapt retire form to include needed translations and move validations
from controller to model.

Also change sanitizable concern to sanitize not marked for destruction
translations.
This commit is contained in:
Senén Rodero Rodríguez
2018-12-22 23:35:32 +01:00
committed by voodoorai2000
parent 5343448c5a
commit 02be0c61f9
10 changed files with 137 additions and 31 deletions

View File

@@ -62,7 +62,7 @@ class ProposalsController < ApplicationController
end
def retire
if valid_retired_params? && @proposal.update(retired_params.merge(retired_at: Time.current))
if @proposal.update(retired_params.merge(retired_at: Time.current))
redirect_to proposal_path(@proposal), notice: t("proposals.notice.retired")
else
render action: :retire_form
@@ -107,13 +107,8 @@ class ProposalsController < ApplicationController
end
def retired_params
params.require(:proposal).permit(:retired_reason, :retired_explanation)
end
def valid_retired_params?
@proposal.errors.add(:retired_reason, I18n.t("errors.messages.blank")) if params[:proposal][:retired_reason].blank?
@proposal.errors.add(:retired_explanation, I18n.t("errors.messages.blank")) if params[:proposal][:retired_explanation].blank?
@proposal.errors.empty?
attributes = [:retired_reason]
params.require(:proposal).permit(attributes, translation_params(Proposal))
end
def resource_model

View File

@@ -28,6 +28,13 @@ class Proposal < ApplicationRecord
RETIRE_OPTIONS = %w[duplicated started unfeasible done other]
translates :title, touch: true
translates :description, touch: true
translates :summary, touch: true
translates :retired_explanation, touch: true
include Globalizable
translation_class_delegate :retired_at
belongs_to :author, -> { with_hidden }, class_name: "User", foreign_key: "author_id"
belongs_to :geozone
has_many :comments, as: :commentable, dependent: :destroy
@@ -39,18 +46,16 @@ class Proposal < ApplicationRecord
extend DownloadSettings::ProposalCsv
delegate :name, :email, to: :author, prefix: true
extend DownloadSettings::ProposalCsv
delegate :name, :email, to: :author, prefix: true
validates_translation :title, presence: true, length: { in: 4..Proposal.title_max_length }
validates_translation :description, length: { maximum: Proposal.description_max_length }
validates_translation :summary, presence: true
validates_translation :retired_explanation, presence: true, unless: -> { retired_at.blank? }
validates :title, presence: true
validates :summary, presence: true
validates :author, presence: true
validates :responsible_name, presence: true, unless: :skip_user_verification?
validates :title, length: { in: 4..Proposal.title_max_length }
validates :description, length: { maximum: Proposal.description_max_length }
validates :responsible_name, length: { in: 6..Proposal.responsible_name_max_length }, unless: :skip_user_verification?
validates :retired_reason, inclusion: { in: RETIRE_OPTIONS, allow_nil: true }
validates :retired_reason, presence: true, inclusion: { in: RETIRE_OPTIONS }, unless: -> { retired_at.blank? }
validates :terms_of_service, acceptance: { allow_nil: false }, on: :create
@@ -264,5 +269,4 @@ class Proposal < ApplicationRecord
self.responsible_name = author.document_number
end
end
end

View File

@@ -10,7 +10,9 @@
<%= t("proposals.retire_form.warning") %>
</div>
<%= form_for(@proposal, url: retire_proposal_path(@proposal)) do |f| %>
<%= render "admin/shared/globalize_locales", resource: @proposal %>
<%= translatable_form_for(@proposal, url: retire_proposal_path(@proposal)) do |f| %>
<%= render "shared/errors", resource: @proposal %>
<div class="row">
<div class="small-12 medium-6 large-4 column">
@@ -20,11 +22,14 @@
</div>
<div class="row">
<div class="small-12 medium-9 column">
<%= f.label :retired_explanation, t("proposals.retire_form.retired_explanation_label") %>
<%= f.text_area :retired_explanation, rows: 4, maxlength: 500, label: false,
placeholder: t("proposals.retire_form.retired_explanation_placeholder") %>
</div>
<%= f.translatable_fields do |translations_form| %>
<div class="small-12 medium-9 column">
<%= translations_form.text_area :retired_explanation,
rows: 4, maxlength: 500,
label: t("proposals.retire_form.retired_explanation_label"),
placeholder: t("proposals.retire_form.retired_explanation_placeholder") %>
</div>
<% end %>
</div>
<div class="row">

View File

@@ -0,0 +1,17 @@
class AddProposalsTranslations < ActiveRecord::Migration[4.2]
def self.up
Proposal.create_translation_table!(
{
title: :string,
description: :text,
summary: :text,
retired_explanation: :text
},
{ migrate_data: true }
)
end
def self.down
Proposal.drop_translation_table!
end
end

View File

@@ -1285,6 +1285,19 @@ ActiveRecord::Schema.define(version: 20190607160900) do
t.datetime "confirmed_hide_at"
end
create_table "proposal_translations", force: :cascade do |t|
t.integer "proposal_id", null: false
t.string "locale", null: false
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
t.string "title"
t.text "description"
t.text "summary"
t.text "retired_explanation"
t.index ["locale"], name: "index_proposal_translations_on_locale", using: :btree
t.index ["proposal_id"], name: "index_proposal_translations_on_proposal_id", using: :btree
end
create_table "proposals", force: :cascade do |t|
t.string "title", limit: 80
t.text "description"

View File

@@ -62,6 +62,8 @@ FactoryBot.define do
trait :retired do
retired_at { Time.current }
retired_reason "unfeasible"
retired_explanation "Retired explanation"
end
trait :published do

View File

@@ -505,7 +505,7 @@ describe "Proposals" do
expect(page).to have_current_path(retire_form_proposal_path(proposal))
select "Duplicated", from: "proposal_retired_reason"
fill_in "proposal_retired_explanation", with: "There are three other better proposals with the same subject"
fill_in "Explanation", with: "There are three other better proposals with the same subject"
click_button "Retire proposal"
expect(page).to have_content "Proposal retired"
@@ -518,7 +518,7 @@ describe "Proposals" do
expect(page).to have_content "There are three other better proposals with the same subject"
end
scenario "Fields are mandatory" do
scenario "Fields are mandatory", :js do
proposal = create(:proposal)
login_as(proposal.author)
@@ -534,7 +534,7 @@ describe "Proposals" do
Setting["feature.featured_proposals"] = true
create_featured_proposals
not_retired = create(:proposal)
retired = create(:proposal, retired_at: Time.current)
retired = create(:proposal, :retired)
visit proposals_path
@@ -548,7 +548,7 @@ describe "Proposals" do
scenario "Index has a link to retired proposals list" do
create_featured_proposals
not_retired = create(:proposal)
retired = create(:proposal, retired_at: Time.current)
retired = create(:proposal, :retired)
visit proposals_path
@@ -568,8 +568,8 @@ describe "Proposals" do
end
scenario "Retired proposals index has links to filter by retired_reason" do
unfeasible = create(:proposal, retired_at: Time.current, retired_reason: "unfeasible")
duplicated = create(:proposal, retired_at: Time.current, retired_reason: "duplicated")
unfeasible = create(:proposal, :retired, retired_reason: "unfeasible")
duplicated = create(:proposal, :retired, retired_reason: "duplicated")
visit proposals_path(retired: "all")

View File

@@ -146,7 +146,9 @@ describe ProposalNotification do
it "returns false if the resource is retired" do
notification = create(:notification, notifiable: notifiable)
notifiable.proposal.update(retired_at: Time.current)
notifiable.proposal.update(retired_at: Time.current,
retired_explanation: "Unfeasible reason explanation",
retired_reason: "unfeasible")
expect(notification.check_availability(proposal)).to be(false)
end

View File

@@ -8,6 +8,7 @@ describe Proposal do
it_behaves_like "has_public_author"
it_behaves_like "notifiable"
it_behaves_like "map validations"
it_behaves_like "globalizable", :proposal
end
it "is valid" do
@@ -44,10 +45,36 @@ describe Proposal do
describe "#description" do
it "is sanitized" do
proposal.description = "<script>alert('danger');</script>"
proposal.valid?
expect(proposal.description).to eq("alert('danger');")
end
it "is sanitized using globalize accessors" do
proposal.description_en = "<script>alert('danger');</script>"
proposal.valid?
expect(proposal.description_en).to eq("alert('danger');")
end
it "is html_safe" do
proposal.description = "<script>alert('danger');</script>"
proposal.valid?
expect(proposal.description).to be_html_safe
end
it "is html_safe using globalize accessors" do
proposal.description_en = "<script>alert('danger');</script>"
proposal.valid?
expect(proposal.description_en).to be_html_safe
end
it "is not valid when very long" do
proposal.description = "a" * 6001
expect(proposal).not_to be_valid
@@ -143,6 +170,46 @@ describe Proposal do
Setting["proposal_code_prefix"] = "MAD"
end
describe "#retired_explanation" do
it "is valid when retired timestamp is present and retired explanation is defined" do
proposal.retired_at = Time.current
proposal.retired_explanation = "Duplicated of ..."
proposal.retired_reason = "duplicated"
expect(proposal).to be_valid
end
it "is not valid when retired_at is present and retired explanation is empty" do
proposal.retired_at = Time.current
proposal.retired_explanation = nil
proposal.retired_reason = "duplicated"
expect(proposal).not_to be_valid
end
end
describe "#retired_reason" do
it "is valid when retired timestamp is present and retired reason is defined" do
proposal.retired_at = Time.current
proposal.retired_explanation = "Duplicated of ..."
proposal.retired_reason = "duplicated"
expect(proposal).to be_valid
end
it "is not valid when retired timestamp is present but defined retired reason
is not included in retired reasons" do
proposal.retired_at = Time.current
proposal.retired_explanation = "Duplicated of ..."
proposal.retired_reason = "duplicate"
expect(proposal).not_to be_valid
end
it "is not valid when retired_at is present and retired reason is empty" do
proposal.retired_at = Time.current
proposal.retired_explanation = "Duplicated of ..."
proposal.retired_reason = nil
expect(proposal).not_to be_valid
end
end
describe "#editable?" do
let(:proposal) { create(:proposal) }
@@ -820,7 +887,7 @@ describe Proposal do
describe "retired" do
let!(:proposal1) { create(:proposal) }
let!(:proposal2) { create(:proposal, retired_at: Time.current) }
let!(:proposal2) { create(:proposal, :retired) }
it "retired? is true" do
expect(proposal1.retired?).to eq false

View File

@@ -78,7 +78,8 @@ shared_examples "notifiable" do
notification = create(:notification, notifiable: notifiable)
if notifiable.respond_to?(:retired_at)
notifiable.update(retired_at: Time.current)
notifiable.update(retired_at: Time.current, retired_reason: "unfeasible",
retired_explanation: "Unfeasibility explanation ...")
expect(notification.check_availability(notifiable)).to be(false)
end
end