diff --git a/app/controllers/proposals_controller.rb b/app/controllers/proposals_controller.rb index fc596c5f0..d2b876f06 100644 --- a/app/controllers/proposals_controller.rb +++ b/app/controllers/proposals_controller.rb @@ -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 diff --git a/app/models/proposal.rb b/app/models/proposal.rb index e0f707918..4961b237d 100644 --- a/app/models/proposal.rb +++ b/app/models/proposal.rb @@ -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 diff --git a/app/views/proposals/retire_form.html.erb b/app/views/proposals/retire_form.html.erb index 501a2ce22..704d2db3c 100644 --- a/app/views/proposals/retire_form.html.erb +++ b/app/views/proposals/retire_form.html.erb @@ -10,7 +10,9 @@ <%= t("proposals.retire_form.warning") %> - <%= 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 %>
@@ -20,11 +22,14 @@
-
- <%= 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") %> -
+ <%= f.translatable_fields do |translations_form| %> +
+ <%= 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") %> +
+ <% end %>
diff --git a/db/migrate/20181129115006_add_proposals_translations.rb b/db/migrate/20181129115006_add_proposals_translations.rb new file mode 100644 index 000000000..8f05fbffd --- /dev/null +++ b/db/migrate/20181129115006_add_proposals_translations.rb @@ -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 diff --git a/db/schema.rb b/db/schema.rb index 36bfc4c7c..c7c0e84d1 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -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" diff --git a/spec/factories/proposals.rb b/spec/factories/proposals.rb index 92a925627..08598e977 100644 --- a/spec/factories/proposals.rb +++ b/spec/factories/proposals.rb @@ -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 diff --git a/spec/features/proposals_spec.rb b/spec/features/proposals_spec.rb index 5c095d56a..f477ad7ba 100644 --- a/spec/features/proposals_spec.rb +++ b/spec/features/proposals_spec.rb @@ -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") diff --git a/spec/models/proposal_notification_spec.rb b/spec/models/proposal_notification_spec.rb index 40670b9d9..24c85c6fd 100644 --- a/spec/models/proposal_notification_spec.rb +++ b/spec/models/proposal_notification_spec.rb @@ -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 diff --git a/spec/models/proposal_spec.rb b/spec/models/proposal_spec.rb index 7d5d8067c..c589bfad5 100644 --- a/spec/models/proposal_spec.rb +++ b/spec/models/proposal_spec.rb @@ -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 = "" + proposal.valid? + expect(proposal.description).to eq("alert('danger');") end + it "is sanitized using globalize accessors" do + proposal.description_en = "" + + proposal.valid? + + expect(proposal.description_en).to eq("alert('danger');") + end + + it "is html_safe" do + proposal.description = "" + + proposal.valid? + + expect(proposal.description).to be_html_safe + end + + it "is html_safe using globalize accessors" do + proposal.description_en = "" + + 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 diff --git a/spec/shared/models/notifiable.rb b/spec/shared/models/notifiable.rb index 8d69e1c00..263d89117 100644 --- a/spec/shared/models/notifiable.rb +++ b/spec/shared/models/notifiable.rb @@ -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