From 5ab8587ad22fa206c1956faa241762f80ebfc484 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 21 Sep 2018 19:47:26 +0200 Subject: [PATCH 1/5] Use absolute translation paths Using `t(.empty_proposals)` IMHO is a bit dangerous because we'd need to change the locale files if we move the code to a partial or rename the file. --- app/views/legislation/processes/proposals.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/legislation/processes/proposals.html.erb b/app/views/legislation/processes/proposals.html.erb index 0f6159845..d78f9964c 100644 --- a/app/views/legislation/processes/proposals.html.erb +++ b/app/views/legislation/processes/proposals.html.erb @@ -11,7 +11,7 @@
<% if @proposals.empty? %>
-

<%= t('.empty_proposals') %>

+

<%= t("legislation.processes.proposals.empty_proposals") %>

<% else %> <%= render @proposals %> From 8064b53aade5a6f164ab0366bea98f086af687c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 21 Sep 2018 20:51:17 +0200 Subject: [PATCH 2/5] Add variety to legislation process dev seeds This way different processes will be in different phases and it will be easier to test the application locally. --- db/dev_seeds/legislation_processes.rb | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/db/dev_seeds/legislation_processes.rb b/db/dev_seeds/legislation_processes.rb index fd4c5a190..4c71763a8 100644 --- a/db/dev_seeds/legislation_processes.rb +++ b/db/dev_seeds/legislation_processes.rb @@ -1,20 +1,20 @@ section "Creating legislation processes" do - 5.times do + 9.times do |i| Legislation::Process.create!(title: Faker::Lorem.sentence(3).truncate(60), description: Faker::Lorem.paragraphs.join("\n\n"), summary: Faker::Lorem.paragraph, additional_info: Faker::Lorem.paragraphs.join("\n\n"), proposals_description: Faker::Lorem.paragraph, - start_date: Date.current - 3.days, - end_date: Date.current + 3.days, - debate_start_date: Date.current - 3.days, - debate_end_date: Date.current - 1.day, - proposals_phase_start_date: Date.current - 3.days, - proposals_phase_end_date: Date.current - 1.day, - draft_publication_date: Date.current + 1.day, - allegations_start_date: Date.current + 2.days, - allegations_end_date: Date.current + 3.days, - result_publication_date: Date.current + 4.days, + start_date: Date.current + (i - 7).days, + end_date: Date.current + (i - 1).days, + debate_start_date: Date.current + (i - 7).days, + debate_end_date: Date.current + (i - 5).days, + proposals_phase_start_date: Date.current + (i - 7).days, + proposals_phase_end_date: Date.current + (i - 5).days, + draft_publication_date: Date.current + (i - 3).days, + allegations_start_date: Date.current + (i - 2).days, + allegations_end_date: Date.current + (i - 1).days, + result_publication_date: Date.current + i.days, debate_phase_enabled: true, allegations_phase_enabled: true, draft_publication_enabled: true, From ec18743251b98d2288389d23a0b0973f5b84f38c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 21 Sep 2018 21:04:36 +0200 Subject: [PATCH 3/5] Authorize resource after authenticating user Just like it's done everywhere else in the application. Not doing so means users who aren't logged in receive a "you aren't authorized" message when they try to create a new legislation proposal instead of being redirected to the login page. --- app/controllers/legislation/proposals_controller.rb | 6 +++--- spec/features/legislation/processes_spec.rb | 10 ++++++++++ 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/app/controllers/legislation/proposals_controller.rb b/app/controllers/legislation/proposals_controller.rb index 157c1f8a9..eb5ce4e2f 100644 --- a/app/controllers/legislation/proposals_controller.rb +++ b/app/controllers/legislation/proposals_controller.rb @@ -2,13 +2,13 @@ class Legislation::ProposalsController < Legislation::BaseController include CommentableActions include FlagActions - load_and_authorize_resource :process, class: "Legislation::Process" - load_and_authorize_resource :proposal, class: "Legislation::Proposal", through: :process - before_action :parse_tag_filter, only: :index before_action :load_categories, only: [:index, :new, :create, :edit, :map, :summary] before_action :load_geozones, only: [:edit, :map, :summary] + before_action :authenticate_user!, except: [:index, :show, :map, :summary] + load_and_authorize_resource :process, class: "Legislation::Process" + load_and_authorize_resource :proposal, class: "Legislation::Proposal", through: :process invisible_captcha only: [:create, :update], honeypot: :subtitle diff --git a/spec/features/legislation/processes_spec.rb b/spec/features/legislation/processes_spec.rb index 92a78499d..9989c5ee8 100644 --- a/spec/features/legislation/processes_spec.rb +++ b/spec/features/legislation/processes_spec.rb @@ -255,6 +255,16 @@ feature 'Legislation' do expect(page).to have_content("There are no proposals") end + scenario 'create proposal button redirects to register path if user is not logged in' do + process = create(:legislation_process, :in_proposals_phase) + + visit legislation_process_proposals_path(process) + click_link "Create a proposal" + + expect(page).to have_current_path new_user_session_path + expect(page).to have_content "You must sign in or register to continue" + end + include_examples "not published permissions", :legislation_process_proposals_path end end From f93281fd02547a615cfad8815b31c494f499067f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 21 Sep 2018 21:56:21 +0200 Subject: [PATCH 4/5] Extract link text to a helper This way we can use the same code in custom views which won't use the same text. --- app/helpers/legislation_helper.rb | 4 ++++ app/views/legislation/processes/proposals.html.erb | 5 ++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/app/helpers/legislation_helper.rb b/app/helpers/legislation_helper.rb index a93c2fa47..d8dc77f87 100644 --- a/app/helpers/legislation_helper.rb +++ b/app/helpers/legislation_helper.rb @@ -6,4 +6,8 @@ module LegislationHelper def format_date_for_calendar_form(date) l(date, format: "%d/%m/%Y") if date end + + def new_legislation_proposal_link_text(process) + t("proposals.index.start_proposal") + end end diff --git a/app/views/legislation/processes/proposals.html.erb b/app/views/legislation/processes/proposals.html.erb index d78f9964c..f2e8b14dd 100644 --- a/app/views/legislation/processes/proposals.html.erb +++ b/app/views/legislation/processes/proposals.html.erb @@ -22,7 +22,10 @@
<% if @process.proposals_phase.open? %> -

<%= link_to t("proposals.index.start_proposal"), new_legislation_process_proposal_path(@process), class: 'button expanded' %>

+

<%= link_to new_legislation_proposal_link_text(@process), + new_legislation_process_proposal_path(@process), + class: 'button expanded', + id: 'create-new-proposal' %>

<% end %> <%= render 'legislation/proposals/categories', taggable: @process %>
From acbf20c938789a8f219b0f354c95a12857271f6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 21 Sep 2018 22:04:11 +0200 Subject: [PATCH 5/5] Extract partial for proposals content --- .../processes/_proposals_content.html.erb | 28 ++++++++++++++++++ .../legislation/processes/proposals.html.erb | 29 +------------------ 2 files changed, 29 insertions(+), 28 deletions(-) create mode 100644 app/views/legislation/processes/_proposals_content.html.erb diff --git a/app/views/legislation/processes/_proposals_content.html.erb b/app/views/legislation/processes/_proposals_content.html.erb new file mode 100644 index 000000000..63a7ac926 --- /dev/null +++ b/app/views/legislation/processes/_proposals_content.html.erb @@ -0,0 +1,28 @@ +
+
+
+
+
+ <% if proposals.empty? %> +
+

<%= t("legislation.processes.proposals.empty_proposals") %>

+
+ <% else %> + <%= render proposals %> + <%= paginate proposals %> + <% end %> +
+
+ +
+ <% if process.proposals_phase.open? %> +

<%= link_to new_legislation_proposal_link_text(process), + new_legislation_process_proposal_path(process), + class: 'button expanded', + id: 'create-new-proposal' %>

+ <% end %> + <%= render 'legislation/proposals/categories', taggable: process %> +
+
+
+
diff --git a/app/views/legislation/processes/proposals.html.erb b/app/views/legislation/processes/proposals.html.erb index f2e8b14dd..eb8caa153 100644 --- a/app/views/legislation/processes/proposals.html.erb +++ b/app/views/legislation/processes/proposals.html.erb @@ -4,31 +4,4 @@ <%= render 'key_dates', process: @process, phase: :proposals %> -
-
-
-
-
- <% if @proposals.empty? %> -
-

<%= t("legislation.processes.proposals.empty_proposals") %>

-
- <% else %> - <%= render @proposals %> - <%= paginate @proposals %> - <% end %> -
-
- -
- <% if @process.proposals_phase.open? %> -

<%= link_to new_legislation_proposal_link_text(@process), - new_legislation_process_proposal_path(@process), - class: 'button expanded', - id: 'create-new-proposal' %>

- <% end %> - <%= render 'legislation/proposals/categories', taggable: @process %> -
-
-
-
+<%= render 'proposals_content', process: @process, proposals: @proposals %>