From 9d627f2db99e5cf8823a10b468c9829f9c02caf2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sun, 22 Sep 2019 00:14:13 +0200 Subject: [PATCH 1/8] Remove redundant I18nContent scope Since two records cannot have the same key, having a scope that will always return just one record is the same as using `find_by_key`. --- app/models/i18n_content.rb | 3 --- config/initializers/i18n_translation.rb | 2 +- spec/models/i18n_content_spec.rb | 15 --------------- 3 files changed, 1 insertion(+), 19 deletions(-) diff --git a/app/models/i18n_content.rb b/app/models/i18n_content.rb index 79a04b20a..163f9329a 100644 --- a/app/models/i18n_content.rb +++ b/app/models/i18n_content.rb @@ -1,7 +1,4 @@ class I18nContent < ApplicationRecord - - scope :by_key, ->(key) { where(key: key) } - validates :key, uniqueness: true translates :value, touch: true diff --git a/config/initializers/i18n_translation.rb b/config/initializers/i18n_translation.rb index fb23270cd..aa500468c 100644 --- a/config/initializers/i18n_translation.rb +++ b/config/initializers/i18n_translation.rb @@ -9,7 +9,7 @@ module ActionView def t(key, options = {}) current_locale = options[:locale].presence || I18n.locale - i18_content = I18nContent.by_key(key).first + i18_content = I18nContent.find_by_key(key) translation = I18nContentTranslation.where(i18n_content_id: i18_content&.id, locale: current_locale).first&.value translation.presence || translate(key, options) diff --git a/spec/models/i18n_content_spec.rb b/spec/models/i18n_content_spec.rb index 974218605..aade19cfc 100644 --- a/spec/models/i18n_content_spec.rb +++ b/spec/models/i18n_content_spec.rb @@ -14,21 +14,6 @@ RSpec.describe I18nContent, type: :model do expect(i18n_content.errors.size).to eq(1) end - context "Scopes" do - it "return one record when #by_key is used" do - content = create(:i18n_content) - key = "debates.form.debate_title" - debate_title = create(:i18n_content, key: key) - - expect(I18nContent.all.size).to eq(2) - - query = I18nContent.by_key(key) - - expect(query.size).to eq(1) - expect(query).to eq([debate_title]) - end - end - context "Globalize" do it "translates key into multiple languages" do key = "devise_views.mailer.confirmation_instructions.welcome" From fe5b45ed18555cefdcf8a48044f8181ccbb95da5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sun, 22 Sep 2019 23:38:28 +0200 Subject: [PATCH 2/8] Remove unnecessary poll creating assignments The factory creating assignments automatically assigns a poll to it, so we don't use the poll for anything else, there's no need to explicitely create it. --- spec/features/budget_polls/officing_spec.rb | 3 +-- spec/models/poll/shift_spec.rb | 6 ++---- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/spec/features/budget_polls/officing_spec.rb b/spec/features/budget_polls/officing_spec.rb index d9ae81cac..5dab07e05 100644 --- a/spec/features/budget_polls/officing_spec.rb +++ b/spec/features/budget_polls/officing_spec.rb @@ -3,9 +3,8 @@ require "rails_helper" describe "Budget Poll Officing" do scenario "Show sidebar menus if officer has shifts assigned" do - poll = create(:poll) booth = create(:poll_booth) - booth_assignment = create(:poll_booth_assignment, poll: poll, booth: booth) + booth_assignment = create(:poll_booth_assignment, booth: booth) user = create(:user) officer = create(:poll_officer, user: user) diff --git a/spec/models/poll/shift_spec.rb b/spec/models/poll/shift_spec.rb index 5baaede91..539a5f988 100644 --- a/spec/models/poll/shift_spec.rb +++ b/spec/models/poll/shift_spec.rb @@ -57,10 +57,8 @@ describe Poll::Shift do describe "officer_assignments" do it "creates and destroy corresponding officer_assignments" do - poll2 = create(:poll) - - booth_assignment1 = create(:poll_booth_assignment, poll: poll, booth: booth) - booth_assignment2 = create(:poll_booth_assignment, poll: poll2, booth: booth) + booth_assignment1 = create(:poll_booth_assignment, booth: booth) + booth_assignment2 = create(:poll_booth_assignment, booth: booth) expect { create(:poll_shift, booth: booth, officer: officer, date: Date.current) }.to change { Poll::OfficerAssignment.all.count }.by(2) From 7db32b337b0b9e6a2800aa9533c58d7244d62b4c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Mon, 23 Sep 2019 18:25:08 +0200 Subject: [PATCH 3/8] Remove unnecessary author variables in specs The factories creating proposals and debates automatically create an author. --- spec/features/proposal_notifications_spec.rb | 15 +++++---------- spec/lib/reply_email_spec.rb | 4 +--- 2 files changed, 6 insertions(+), 13 deletions(-) diff --git a/spec/features/proposal_notifications_spec.rb b/spec/features/proposal_notifications_spec.rb index 55230ba25..cbf542f58 100644 --- a/spec/features/proposal_notifications_spec.rb +++ b/spec/features/proposal_notifications_spec.rb @@ -29,8 +29,7 @@ describe "Proposal Notifications" do end scenario "Send a notification (Active voter)" do - author = create(:user) - proposal = create(:proposal, author: author) + proposal = create(:proposal) voter = create(:user, :level_two) create(:vote, voter: voter, votable: proposal) @@ -41,8 +40,7 @@ describe "Proposal Notifications" do end scenario "Send a notification (Follower)" do - author = create(:user) - proposal = create(:proposal, author: author) + proposal = create(:proposal) user_follower = create(:user) create(:follow, :followed_proposal, user: user_follower, followable: proposal) @@ -52,8 +50,7 @@ describe "Proposal Notifications" do end scenario "Send a notification (Follower and Voter)" do - author = create(:user) - proposal = create(:proposal, author: author) + proposal = create(:proposal) user_voter_follower = create(:user) create(:follow, :followed_proposal, user: user_voter_follower, followable: proposal) @@ -68,8 +65,7 @@ describe "Proposal Notifications" do end scenario "Send a notification (Blocked voter)" do - author = create(:user) - proposal = create(:proposal, author: author) + proposal = create(:proposal) voter = create(:user, :level_two) create(:vote, voter: voter, votable: proposal) @@ -81,8 +77,7 @@ describe "Proposal Notifications" do end scenario "Send a notification (Erased voter)" do - author = create(:user) - proposal = create(:proposal, author: author) + proposal = create(:proposal) voter = create(:user, :level_two) create(:vote, voter: voter, votable: proposal) diff --git a/spec/lib/reply_email_spec.rb b/spec/lib/reply_email_spec.rb index 87819e3d1..68054e08f 100644 --- a/spec/lib/reply_email_spec.rb +++ b/spec/lib/reply_email_spec.rb @@ -1,9 +1,7 @@ require "rails_helper" describe ReplyEmail do - - let(:author) { create(:user) } - let(:debate) { create(:debate, author: author) } + let(:debate) { create(:debate) } let(:commenter) { create(:user, email: "email@commenter.org") } let(:comment) { create(:comment, commentable: debate, user: commenter) } let(:replier) { create(:user) } From adf59cc963e766ae2e7fc735b1e6a8287f1f6860 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Tue, 24 Sep 2019 03:19:49 +0200 Subject: [PATCH 4/8] Move related tests together --- spec/features/polls/polls_spec.rb | 34 +++++++++++++++---------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/spec/features/polls/polls_spec.rb b/spec/features/polls/polls_spec.rb index 02ba30eb5..a8a86951a 100644 --- a/spec/features/polls/polls_spec.rb +++ b/spec/features/polls/polls_spec.rb @@ -483,23 +483,6 @@ describe "Polls" do expect(page).to have_content("You do not have permission to carry out the action 'stats' on poll.") end - scenario "Don't show poll results and stats if is not expired" do - poll = create(:poll, :current, results_enabled: true, stats_enabled: true) - user = create(:user) - - login_as user - visit poll_path(poll) - - expect(page).not_to have_content("Poll results") - expect(page).not_to have_content("Participation statistics") - - visit results_poll_path(poll) - expect(page).to have_content("You do not have permission to carry out the action 'results' on poll.") - - visit stats_poll_path(poll) - expect(page).to have_content("You do not have permission to carry out the action 'stats' on poll.") - end - scenario "Do not show poll results or stats if are disabled" do poll = create(:poll, :expired, results_enabled: false, stats_enabled: false) question1 = create(:poll_question, poll: poll) @@ -524,6 +507,23 @@ describe "Polls" do expect(page).not_to have_content("Participation statistics") end + scenario "Don't show poll results and stats if is not expired" do + poll = create(:poll, :current, results_enabled: true, stats_enabled: true) + user = create(:user) + + login_as user + visit poll_path(poll) + + expect(page).not_to have_content("Poll results") + expect(page).not_to have_content("Participation statistics") + + visit results_poll_path(poll) + expect(page).to have_content("You do not have permission to carry out the action 'results' on poll.") + + visit stats_poll_path(poll) + expect(page).to have_content("You do not have permission to carry out the action 'stats' on poll.") + end + scenario "Generates navigation links for polls without a slug" do poll = create(:poll, :expired, results_enabled: true, stats_enabled: true) poll.update_column(:slug, nil) From ef0330f671991e76ccc366a4a2f7982b300dd193 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Tue, 24 Sep 2019 03:21:10 +0200 Subject: [PATCH 5/8] Remove duplicate test One test was testing regular users can't access results, and another one was testing neither regular users nor managers can. So the second test can just test the admin scenario, and we're still covering everything. --- spec/features/polls/polls_spec.rb | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/spec/features/polls/polls_spec.rb b/spec/features/polls/polls_spec.rb index a8a86951a..415c2eee3 100644 --- a/spec/features/polls/polls_spec.rb +++ b/spec/features/polls/polls_spec.rb @@ -483,7 +483,7 @@ describe "Polls" do expect(page).to have_content("You do not have permission to carry out the action 'stats' on poll.") end - scenario "Do not show poll results or stats if are disabled" do + scenario "Do not show poll results or stats to admins if disabled" do poll = create(:poll, :expired, results_enabled: false, stats_enabled: false) question1 = create(:poll_question, poll: poll) create(:poll_question_answer, question: question1, title: "Han Solo") @@ -491,15 +491,8 @@ describe "Polls" do question2 = create(:poll_question, poll: poll) create(:poll_question_answer, question: question2, title: "Leia") create(:poll_question_answer, question: question2, title: "Luke") - user = create(:user) admin = create(:administrator).user - login_as user - visit poll_path(poll) - - expect(page).not_to have_content("Poll results") - expect(page).not_to have_content("Participation statistics") - login_as admin visit poll_path(poll) From e1c09769a8e2e7b6226bb98b80b121495aac8250 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Tue, 24 Sep 2019 03:21:45 +0200 Subject: [PATCH 6/8] Remove unnecessary data in polls spec This data was added in commit 62088bc6, but the changes in that commit don't seem to be related to the number of questions in a poll. --- spec/features/polls/polls_spec.rb | 6 ------ 1 file changed, 6 deletions(-) diff --git a/spec/features/polls/polls_spec.rb b/spec/features/polls/polls_spec.rb index 415c2eee3..7d392bbab 100644 --- a/spec/features/polls/polls_spec.rb +++ b/spec/features/polls/polls_spec.rb @@ -485,12 +485,6 @@ describe "Polls" do scenario "Do not show poll results or stats to admins if disabled" do poll = create(:poll, :expired, results_enabled: false, stats_enabled: false) - question1 = create(:poll_question, poll: poll) - create(:poll_question_answer, question: question1, title: "Han Solo") - create(:poll_question_answer, question: question1, title: "Chewbacca") - question2 = create(:poll_question, poll: poll) - create(:poll_question_answer, question: question2, title: "Leia") - create(:poll_question_answer, question: question2, title: "Luke") admin = create(:administrator).user login_as admin From f5849cb5d836a1301aaddf063e370e833a9dbce3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Mon, 23 Sep 2019 06:17:45 +0200 Subject: [PATCH 7/8] Remove unnecessary question creating answers Questions are automatically created by the poll_question_answer factory. --- .../questions/answers/documents/documents_spec.rb | 6 +++--- .../poll/questions/answers/images/images_spec.rb | 12 ++++-------- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/spec/features/admin/poll/questions/answers/documents/documents_spec.rb b/spec/features/admin/poll/questions/answers/documents/documents_spec.rb index 17340d98d..c8c59cd79 100644 --- a/spec/features/admin/poll/questions/answers/documents/documents_spec.rb +++ b/spec/features/admin/poll/questions/answers/documents/documents_spec.rb @@ -9,7 +9,7 @@ describe "Documents" do context "Index" do scenario "Answer with no documents" do - answer = create(:poll_question_answer, question: create(:poll_question)) + answer = create(:poll_question_answer) document = create(:document) visit admin_answer_documents_path(answer) @@ -18,7 +18,7 @@ describe "Documents" do end scenario "Answer with documents" do - answer = create(:poll_question_answer, question: create(:poll_question)) + answer = create(:poll_question_answer) document = create(:document, documentable: answer) visit admin_answer_documents_path(answer) @@ -28,7 +28,7 @@ describe "Documents" do end scenario "Remove document from answer", :js do - answer = create(:poll_question_answer, question: create(:poll_question)) + answer = create(:poll_question_answer) document = create(:document, documentable: answer) visit admin_answer_documents_path(answer) diff --git a/spec/features/admin/poll/questions/answers/images/images_spec.rb b/spec/features/admin/poll/questions/answers/images/images_spec.rb index 474b63a5c..db8b7a26d 100644 --- a/spec/features/admin/poll/questions/answers/images/images_spec.rb +++ b/spec/features/admin/poll/questions/answers/images/images_spec.rb @@ -9,8 +9,7 @@ describe "Images" do context "Index" do scenario "Answer with no images" do - answer = create(:poll_question_answer, - question: create(:poll_question)) + answer = create(:poll_question_answer) visit admin_answer_images_path(answer) @@ -18,8 +17,7 @@ describe "Images" do end scenario "Answer with images" do - answer = create(:poll_question_answer, - question: create(:poll_question)) + answer = create(:poll_question_answer) image = create(:image, imageable: answer) visit admin_answer_images_path(answer) @@ -30,8 +28,7 @@ describe "Images" do end scenario "Add image to answer", :js do - answer = create(:poll_question_answer, - question: create(:poll_question)) + answer = create(:poll_question_answer) image = create(:image) visit admin_answer_images_path(answer) @@ -47,8 +44,7 @@ describe "Images" do end scenario "Remove image from answer", :js do - answer = create(:poll_question_answer, - question: create(:poll_question)) + answer = create(:poll_question_answer) image = create(:image, imageable: answer) visit admin_answer_images_path(answer) From 01862d66642a27f435d875229c1f30732bc6741b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Tue, 24 Sep 2019 04:26:35 +0200 Subject: [PATCH 8/8] Remove unused attributes creating a heading The "name" attribute is automatically generated by the budget heading factory. And the "price" attribute is out of context and not needed since this test doesn't create investments. --- spec/features/budgets/ballots_spec.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/spec/features/budgets/ballots_spec.rb b/spec/features/budgets/ballots_spec.rb index 707dbf0da..11e5b3eaa 100644 --- a/spec/features/budgets/ballots_spec.rb +++ b/spec/features/budgets/ballots_spec.rb @@ -434,8 +434,7 @@ describe "Ballots" do scenario "Display links to vote on groups with no investments voted yet" do group = create(:budget_group, budget: budget) - heading = create(:budget_heading, name: "District 1", group: group, price: 100) - + heading = create(:budget_heading, group: group) ballot = create(:budget_ballot, user: user, budget: budget) login_as(user)