From 1e4f5391045957bc4e3a20feca7e15921083d7e2 Mon Sep 17 00:00:00 2001 From: Cassiano Sampaio Date: Mon, 24 Jun 2019 08:02:50 -0300 Subject: [PATCH 1/2] Add title to differentiate signature sheets --- .../admin/signature_sheets_controller.rb | 2 +- app/models/signature_sheet.rb | 2 +- app/views/admin/signature_sheets/new.html.erb | 4 +++ ...1440_add_name_field_to_signature_sheets.rb | 5 +++ db/schema.rb | 1 + spec/factories/proposals.rb | 6 ++++ spec/features/admin/signature_sheets_spec.rb | 7 ++-- spec/models/signature_sheet_spec.rb | 36 ++++++++++++++----- 8 files changed, 51 insertions(+), 12 deletions(-) create mode 100644 db/migrate/20190621181440_add_name_field_to_signature_sheets.rb diff --git a/app/controllers/admin/signature_sheets_controller.rb b/app/controllers/admin/signature_sheets_controller.rb index 354d073ee..a4e01b97e 100644 --- a/app/controllers/admin/signature_sheets_controller.rb +++ b/app/controllers/admin/signature_sheets_controller.rb @@ -26,6 +26,6 @@ class Admin::SignatureSheetsController < Admin::BaseController private def signature_sheet_params - params.require(:signature_sheet).permit(:signable_type, :signable_id, :required_fields_to_verify) + params.require(:signature_sheet).permit(:signable_type, :signable_id, :document_numbers, :title, :required_fields_to_verify) end end diff --git a/app/models/signature_sheet.rb b/app/models/signature_sheet.rb index aa6fdbb71..538f27a65 100644 --- a/app/models/signature_sheet.rb +++ b/app/models/signature_sheet.rb @@ -13,7 +13,7 @@ class SignatureSheet < ApplicationRecord validate :signable_found def name - "#{signable_name} #{signable_id}" + title ? "#{signable_name} #{signable_id}: #{title}" : "#{signable_name} #{signable_id}" end def signable_name diff --git a/app/views/admin/signature_sheets/new.html.erb b/app/views/admin/signature_sheets/new.html.erb index f851ebd0f..aa81aa5fd 100644 --- a/app/views/admin/signature_sheets/new.html.erb +++ b/app/views/admin/signature_sheets/new.html.erb @@ -6,6 +6,10 @@ <%= render "shared/errors", resource: @signature_sheet %> +
+ <%= f.text_field :title %> +
+
<%= f.select :signable_type, signable_options %>
diff --git a/db/migrate/20190621181440_add_name_field_to_signature_sheets.rb b/db/migrate/20190621181440_add_name_field_to_signature_sheets.rb new file mode 100644 index 000000000..4f2dfd64a --- /dev/null +++ b/db/migrate/20190621181440_add_name_field_to_signature_sheets.rb @@ -0,0 +1,5 @@ +class AddNameFieldToSignatureSheets < ActiveRecord::Migration[5.0] + def change + add_column :signature_sheets, :title, :string + end +end diff --git a/db/schema.rb b/db/schema.rb index c0c5db67d..1eccac194 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1318,6 +1318,7 @@ ActiveRecord::Schema.define(version: 20191108173350) do t.integer "author_id" t.datetime "created_at" t.datetime "updated_at" + t.string "title" end create_table "signatures", force: :cascade do |t| diff --git a/spec/factories/proposals.rb b/spec/factories/proposals.rb index 1229419d2..a36891e54 100644 --- a/spec/factories/proposals.rb +++ b/spec/factories/proposals.rb @@ -118,6 +118,12 @@ FactoryBot.define do association :signable, factory: :proposal association :author, factory: :user required_fields_to_verify { "123A, 456B, 789C" } + document_numbers "123A, 456B, 789C" + + trait :with_title do + title { Faker::Lorem.sentence } + end + end factory :signature do diff --git a/spec/features/admin/signature_sheets_spec.rb b/spec/features/admin/signature_sheets_spec.rb index a0ba61dda..65b30463f 100644 --- a/spec/features/admin/signature_sheets_spec.rb +++ b/spec/features/admin/signature_sheets_spec.rb @@ -36,6 +36,7 @@ describe "Signature sheets" do proposal = create(:proposal) visit new_admin_signature_sheet_path + fill_in "signature_sheet_title", with: "definitive signature sheet" select "Citizen proposal", from: "signature_sheet_signable_type" fill_in "signature_sheet_signable_id", with: proposal.id fill_in "signature_sheet_required_fields_to_verify", with: "12345678Z; 1234567L; 99999999Z" @@ -58,6 +59,7 @@ describe "Signature sheets" do visit new_admin_signature_sheet_path + fill_in "signature_sheet_title", with: "definitive signature sheet" select "Investment", from: "signature_sheet_signable_type" fill_in "signature_sheet_signable_id", with: investment.id fill_in "signature_sheet_required_fields_to_verify", with: "12345678Z; 1234567L; 99999999Z" @@ -134,6 +136,7 @@ describe "Signature sheets" do proposal = create(:proposal) user = Administrator.first.user signature_sheet = create(:signature_sheet, + :with_title, signable: proposal, required_fields_to_verify: "12345678Z; 123A; 123B", author: user) @@ -141,8 +144,8 @@ describe "Signature sheets" do visit admin_signature_sheet_path(signature_sheet) - expect(page).to have_content "Citizen proposal #{proposal.id}" - expect(page).to have_content "12345678Z; 123A; 123B" + expect(page).to have_content "Citizen proposal #{proposal.id}: #{signature_sheet.title}" + expect(page).to have_content "12345678Z, 123A, 123B" expect(page).to have_content signature_sheet.created_at.strftime("%B %d, %Y %H:%M") expect(page).to have_content user.name diff --git a/spec/models/signature_sheet_spec.rb b/spec/models/signature_sheet_spec.rb index faaf5b9a6..cc2bf9b19 100644 --- a/spec/models/signature_sheet_spec.rb +++ b/spec/models/signature_sheet_spec.rb @@ -38,18 +38,38 @@ describe SignatureSheet do end describe "#name" do - it "returns name for proposal signature sheets" do - proposal = create(:proposal) - signature_sheet.signable = proposal + context "when name is nil" do + it "returns name for proposal signature sheets" do + proposal = create(:proposal) + signature_sheet.signable = proposal - expect(signature_sheet.name).to eq("Citizen proposal #{proposal.id}") + expect(signature_sheet.name).to eq("Citizen proposal #{proposal.id}") + end + + it "returns name for budget investment signature sheets" do + budget_investment = create(:budget_investment) + signature_sheet.signable = budget_investment + + expect(signature_sheet.name).to eq("Investment #{budget_investment.id}") + end end - it "returns name for budget investment signature sheets" do - budget_investment = create(:budget_investment) - signature_sheet.signable = budget_investment + context "when name is not nil" do + let(:signature_sheet) { build(:signature_sheet, :with_title) } - expect(signature_sheet.name).to eq("Investment #{budget_investment.id}") + it "returns name for proposal signature sheets" do + proposal = create(:proposal) + signature_sheet.signable = proposal + + expect(signature_sheet.name).to eq("Citizen proposal #{proposal.id}: #{signature_sheet.title}") + end + + it "returns name for budget investment signature sheets" do + budget_investment = create(:budget_investment) + signature_sheet.signable = budget_investment + + expect(signature_sheet.name).to eq("Investment #{budget_investment.id}: #{signature_sheet.title}") + end end end From 95c82d87772937468751faae24ef3b10c168ba5f Mon Sep 17 00:00:00 2001 From: Andrew Sims Date: Tue, 10 Mar 2020 22:12:01 +1100 Subject: [PATCH 2/2] Changes following PR review * Internationalisation for admin fields * Correct typos * Additional tests * Replace ternary with if-then statement --- .../admin/signature_sheets_controller.rb | 7 +++++- app/models/signature_sheet.rb | 6 ++++- config/locales/en/activerecord.yml | 1 + config/locales/es/activerecord.yml | 1 + spec/factories/proposals.rb | 2 -- spec/features/admin/signature_sheets_spec.rb | 2 +- spec/models/signature_sheet_spec.rb | 22 +++++++++++++++++-- 7 files changed, 34 insertions(+), 7 deletions(-) diff --git a/app/controllers/admin/signature_sheets_controller.rb b/app/controllers/admin/signature_sheets_controller.rb index a4e01b97e..11dbce3ed 100644 --- a/app/controllers/admin/signature_sheets_controller.rb +++ b/app/controllers/admin/signature_sheets_controller.rb @@ -26,6 +26,11 @@ class Admin::SignatureSheetsController < Admin::BaseController private def signature_sheet_params - params.require(:signature_sheet).permit(:signable_type, :signable_id, :document_numbers, :title, :required_fields_to_verify) + params.require(:signature_sheet).permit( + :signable_type, + :signable_id, + :title, + :required_fields_to_verify + ) end end diff --git a/app/models/signature_sheet.rb b/app/models/signature_sheet.rb index 538f27a65..c767c0bf1 100644 --- a/app/models/signature_sheet.rb +++ b/app/models/signature_sheet.rb @@ -13,7 +13,11 @@ class SignatureSheet < ApplicationRecord validate :signable_found def name - title ? "#{signable_name} #{signable_id}: #{title}" : "#{signable_name} #{signable_id}" + if title.present? + "#{signable_name} #{signable_id}: #{title}" + else + "#{signable_name} #{signable_id}" + end end def signable_name diff --git a/config/locales/en/activerecord.yml b/config/locales/en/activerecord.yml index d30a8785b..cd9b4c6f7 100644 --- a/config/locales/en/activerecord.yml +++ b/config/locales/en/activerecord.yml @@ -300,6 +300,7 @@ en: body: "Message" title: "Title" signature_sheet: + title: "Title" signable_type: "Signable type" signable_id: "Signable ID" document_numbers: "Documents numbers" diff --git a/config/locales/es/activerecord.yml b/config/locales/es/activerecord.yml index ceef5bb12..0e1f39c92 100644 --- a/config/locales/es/activerecord.yml +++ b/config/locales/es/activerecord.yml @@ -302,6 +302,7 @@ es: body: "Mensaje" title: "Título" signature_sheet: + title: "Título" signable_type: "Tipo de hoja de firmas" signable_id: "ID Propuesta ciudadana/Proyecto de gasto" document_numbers: "Números de documentos" diff --git a/spec/factories/proposals.rb b/spec/factories/proposals.rb index a36891e54..e4e31278f 100644 --- a/spec/factories/proposals.rb +++ b/spec/factories/proposals.rb @@ -118,12 +118,10 @@ FactoryBot.define do association :signable, factory: :proposal association :author, factory: :user required_fields_to_verify { "123A, 456B, 789C" } - document_numbers "123A, 456B, 789C" trait :with_title do title { Faker::Lorem.sentence } end - end factory :signature do diff --git a/spec/features/admin/signature_sheets_spec.rb b/spec/features/admin/signature_sheets_spec.rb index 65b30463f..57d57d350 100644 --- a/spec/features/admin/signature_sheets_spec.rb +++ b/spec/features/admin/signature_sheets_spec.rb @@ -145,7 +145,7 @@ describe "Signature sheets" do visit admin_signature_sheet_path(signature_sheet) expect(page).to have_content "Citizen proposal #{proposal.id}: #{signature_sheet.title}" - expect(page).to have_content "12345678Z, 123A, 123B" + expect(page).to have_content "12345678Z; 123A; 123B" expect(page).to have_content signature_sheet.created_at.strftime("%B %d, %Y %H:%M") expect(page).to have_content user.name diff --git a/spec/models/signature_sheet_spec.rb b/spec/models/signature_sheet_spec.rb index cc2bf9b19..8eb16b6d3 100644 --- a/spec/models/signature_sheet_spec.rb +++ b/spec/models/signature_sheet_spec.rb @@ -38,7 +38,7 @@ describe SignatureSheet do end describe "#name" do - context "when name is nil" do + context "when title is nil" do it "returns name for proposal signature sheets" do proposal = create(:proposal) signature_sheet.signable = proposal @@ -54,7 +54,7 @@ describe SignatureSheet do end end - context "when name is not nil" do + context "when title is not nil" do let(:signature_sheet) { build(:signature_sheet, :with_title) } it "returns name for proposal signature sheets" do @@ -71,6 +71,24 @@ describe SignatureSheet do expect(signature_sheet.name).to eq("Investment #{budget_investment.id}: #{signature_sheet.title}") end end + + context "when title is an empty string" do + let(:signature_sheet) { build(:signature_sheet, title: "") } + + it "returns name for proposal signature sheets" do + proposal = create(:proposal) + signature_sheet.signable = proposal + + expect(signature_sheet.name).to eq("Citizen proposal #{proposal.id}") + end + + it "returns name for budget investment signature sheets" do + budget_investment = create(:budget_investment) + signature_sheet.signable = budget_investment + + expect(signature_sheet.name).to eq("Investment #{budget_investment.id}") + end + end end describe "#verify_signatures" do