From 2c2831beb051c0e9fb403d9418a0fe694544fde7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Mon, 12 Nov 2018 19:27:14 +0100 Subject: [PATCH 1/5] Use polymorphic paths for milestones --- .../budget_investment_milestones_controller.rb | 15 ++++++++------- .../budget_investment_milestones/_form.html.erb | 2 +- .../budget_investment_milestones/edit.html.erb | 2 +- .../budget_investment_milestones/new.html.erb | 2 +- .../admin/budget_investments/_milestones.html.erb | 9 +++------ app/views/admin/budget_investments/show.html.erb | 3 ++- config/initializers/routes_hierarchy.rb | 2 +- 7 files changed, 17 insertions(+), 18 deletions(-) diff --git a/app/controllers/admin/budget_investment_milestones_controller.rb b/app/controllers/admin/budget_investment_milestones_controller.rb index 23ef679cc..c8685de18 100644 --- a/app/controllers/admin/budget_investment_milestones_controller.rb +++ b/app/controllers/admin/budget_investment_milestones_controller.rb @@ -4,20 +4,20 @@ class Admin::BudgetInvestmentMilestonesController < Admin::BaseController before_action :load_budget_investment, only: [:index, :new, :create, :edit, :update, :destroy] before_action :load_milestone, only: [:edit, :update, :destroy] before_action :load_statuses, only: [:index, :new, :create, :edit, :update] + helper_method :milestoneable_path def index end def new - @milestone = Milestone.new + @milestone = @investment.milestones.new end def create @milestone = Milestone.new(milestone_params) @milestone.milestoneable = @investment if @milestone.save - redirect_to admin_budget_budget_investment_path(@investment.budget, @investment), - notice: t('admin.milestones.create.notice') + redirect_to milestoneable_path, notice: t('admin.milestones.create.notice') else render :new end @@ -28,8 +28,7 @@ class Admin::BudgetInvestmentMilestonesController < Admin::BaseController def update if @milestone.update(milestone_params) - redirect_to admin_budget_budget_investment_path(@investment.budget, @investment), - notice: t('admin.milestones.update.notice') + redirect_to milestoneable_path, notice: t('admin.milestones.update.notice') else render :edit end @@ -37,8 +36,7 @@ class Admin::BudgetInvestmentMilestonesController < Admin::BaseController def destroy @milestone.destroy - redirect_to admin_budget_budget_investment_path(@investment.budget, @investment), - notice: t('admin.milestones.delete.notice') + redirect_to milestoneable_path, notice: t('admin.milestones.delete.notice') end private @@ -73,4 +71,7 @@ class Admin::BudgetInvestmentMilestonesController < Admin::BaseController @statuses = Milestone::Status.all end + def milestoneable_path + polymorphic_path([:admin, *resource_hierarchy_for(@milestone.milestoneable)]) + end end diff --git a/app/views/admin/budget_investment_milestones/_form.html.erb b/app/views/admin/budget_investment_milestones/_form.html.erb index c62ba3521..5006b8fa1 100644 --- a/app/views/admin/budget_investment_milestones/_form.html.erb +++ b/app/views/admin/budget_investment_milestones/_form.html.erb @@ -1,6 +1,6 @@ <%= render "admin/shared/globalize_locales", resource: @milestone %> -<%= translatable_form_for [:admin, @investment.budget, @investment, @milestone] do |f| %> +<%= translatable_form_for [:admin, *resource_hierarchy_for(@milestone)] do |f| %>
<%= f.select :status_id, diff --git a/app/views/admin/budget_investment_milestones/edit.html.erb b/app/views/admin/budget_investment_milestones/edit.html.erb index d15b5a66a..7a3a1f175 100644 --- a/app/views/admin/budget_investment_milestones/edit.html.erb +++ b/app/views/admin/budget_investment_milestones/edit.html.erb @@ -1,4 +1,4 @@ -<%= back_link_to admin_budget_budget_investment_path(@investment.budget, @investment) %> +<%= back_link_to milestoneable_path %>

<%= t("admin.milestones.edit.title") %>

diff --git a/app/views/admin/budget_investment_milestones/new.html.erb b/app/views/admin/budget_investment_milestones/new.html.erb index 7e065a605..aa55123c9 100644 --- a/app/views/admin/budget_investment_milestones/new.html.erb +++ b/app/views/admin/budget_investment_milestones/new.html.erb @@ -1,7 +1,7 @@
- <%= back_link_to admin_budget_budget_investment_path(@investment.budget, @investment) %> + <%= back_link_to milestoneable_path %>

<%= t("admin.milestones.new.creating") %>

diff --git a/app/views/admin/budget_investments/_milestones.html.erb b/app/views/admin/budget_investments/_milestones.html.erb index 20a870d71..1bba1bc18 100644 --- a/app/views/admin/budget_investments/_milestones.html.erb +++ b/app/views/admin/budget_investments/_milestones.html.erb @@ -18,9 +18,8 @@ <%= milestone.id %> <%= link_to milestone.title, - edit_admin_budget_budget_investment_milestone_path(@investment.budget, - @investment, - milestone) %> + polymorphic_path([:admin, *resource_hierarchy_for(milestone)], + action: :edit) %> <%= milestone.description %> @@ -46,9 +45,7 @@ <%= link_to t("admin.milestones.index.delete"), - admin_budget_budget_investment_milestone_path(@investment.budget, - @investment, - milestone), + polymorphic_path([:admin, *resource_hierarchy_for(milestone)]), method: :delete, class: "button hollow alert expanded" %> diff --git a/app/views/admin/budget_investments/show.html.erb b/app/views/admin/budget_investments/show.html.erb index cb44745a1..c6462ff52 100644 --- a/app/views/admin/budget_investments/show.html.erb +++ b/app/views/admin/budget_investments/show.html.erb @@ -63,6 +63,7 @@

<%= link_to t("admin.budget_investments.show.new_milestone"), - new_admin_budget_budget_investment_milestone_path(@budget, @investment), + polymorphic_path([:admin, *resource_hierarchy_for(@investment.milestones.new)], + action: :new), class: "button hollow" %>

diff --git a/config/initializers/routes_hierarchy.rb b/config/initializers/routes_hierarchy.rb index e9892ef38..0a712f923 100644 --- a/config/initializers/routes_hierarchy.rb +++ b/config/initializers/routes_hierarchy.rb @@ -8,7 +8,7 @@ module ActionDispatch::Routing::UrlFor when "Budget::Investment" [resource.budget, resource] when "Milestone" - [resource.milestoneable.budget, resource.milestoneable, resource] + [*resource_hierarchy_for(resource.milestoneable), resource] when "Legislation::Annotation" [resource.draft_version.process, resource.draft_version, resource] when "Legislation::Proposal", "Legislation::Question", "Legislation::DraftVersion" From 2e778b4073f23dd3245b6a565a61a773b07e9d22 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Mon, 12 Nov 2018 19:28:20 +0100 Subject: [PATCH 2/5] Use `milestoneable` instead of `investment` --- .../admin/budget_investment_milestones_controller.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/app/controllers/admin/budget_investment_milestones_controller.rb b/app/controllers/admin/budget_investment_milestones_controller.rb index c8685de18..da023499a 100644 --- a/app/controllers/admin/budget_investment_milestones_controller.rb +++ b/app/controllers/admin/budget_investment_milestones_controller.rb @@ -1,7 +1,7 @@ class Admin::BudgetInvestmentMilestonesController < Admin::BaseController include Translatable - before_action :load_budget_investment, only: [:index, :new, :create, :edit, :update, :destroy] + before_action :load_milestoneable, only: [:index, :new, :create, :edit, :update, :destroy] before_action :load_milestone, only: [:edit, :update, :destroy] before_action :load_statuses, only: [:index, :new, :create, :edit, :update] helper_method :milestoneable_path @@ -10,12 +10,12 @@ class Admin::BudgetInvestmentMilestonesController < Admin::BaseController end def new - @milestone = @investment.milestones.new + @milestone = @milestoneable.milestones.new end def create @milestone = Milestone.new(milestone_params) - @milestone.milestoneable = @investment + @milestone.milestoneable = @milestoneable if @milestone.save redirect_to milestoneable_path, notice: t('admin.milestones.create.notice') else @@ -51,8 +51,8 @@ class Admin::BudgetInvestmentMilestonesController < Admin::BaseController params.require(:milestone).permit(*attributes) end - def load_budget_investment - @investment = Budget::Investment.find(params[:budget_investment_id]) + def load_milestoneable + @milestoneable = Budget::Investment.find(params[:budget_investment_id]) end def load_milestone From c4448faf70c29e371505d9cb1a8c0acdffad8a7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Mon, 12 Nov 2018 19:43:50 +0100 Subject: [PATCH 3/5] Move milestones code to admin/milestones All milestone controllers will inherit from `AdminMilestonesController`, and all views will render the same content. --- ...budget_investment_milestones_controller.rb | 75 +---------------- .../admin/milestones_controller.rb | 81 +++++++++++++++++++ .../_form.html.erb | 0 .../edit.html.erb | 2 +- .../new.html.erb | 0 5 files changed, 85 insertions(+), 73 deletions(-) create mode 100644 app/controllers/admin/milestones_controller.rb rename app/views/admin/{budget_investment_milestones => milestones}/_form.html.erb (100%) rename app/views/admin/{budget_investment_milestones => milestones}/edit.html.erb (68%) rename app/views/admin/{budget_investment_milestones => milestones}/new.html.erb (100%) diff --git a/app/controllers/admin/budget_investment_milestones_controller.rb b/app/controllers/admin/budget_investment_milestones_controller.rb index da023499a..f354d42fa 100644 --- a/app/controllers/admin/budget_investment_milestones_controller.rb +++ b/app/controllers/admin/budget_investment_milestones_controller.rb @@ -1,77 +1,8 @@ -class Admin::BudgetInvestmentMilestonesController < Admin::BaseController - include Translatable - - before_action :load_milestoneable, only: [:index, :new, :create, :edit, :update, :destroy] - before_action :load_milestone, only: [:edit, :update, :destroy] - before_action :load_statuses, only: [:index, :new, :create, :edit, :update] - helper_method :milestoneable_path - - def index - end - - def new - @milestone = @milestoneable.milestones.new - end - - def create - @milestone = Milestone.new(milestone_params) - @milestone.milestoneable = @milestoneable - if @milestone.save - redirect_to milestoneable_path, notice: t('admin.milestones.create.notice') - else - render :new - end - end - - def edit - end - - def update - if @milestone.update(milestone_params) - redirect_to milestoneable_path, notice: t('admin.milestones.update.notice') - else - render :edit - end - end - - def destroy - @milestone.destroy - redirect_to milestoneable_path, notice: t('admin.milestones.delete.notice') - end +class Admin::BudgetInvestmentMilestonesController < Admin::MilestonesController private - def milestone_params - image_attributes = [:id, :title, :attachment, :cached_attachment, :user_id, :_destroy] - documents_attributes = [:id, :title, :attachment, :cached_attachment, :user_id, :_destroy] - attributes = [:publication_date, :budget_investment_id, :status_id, - translation_params(Milestone), - image_attributes: image_attributes, documents_attributes: documents_attributes] - - params.require(:milestone).permit(*attributes) - end - - def load_milestoneable - @milestoneable = Budget::Investment.find(params[:budget_investment_id]) - end - - def load_milestone - @milestone = get_milestone - end - - def get_milestone - Milestone.find(params[:id]) - end - - def resource - get_milestone - end - - def load_statuses - @statuses = Milestone::Status.all - end - - def milestoneable_path - polymorphic_path([:admin, *resource_hierarchy_for(@milestone.milestoneable)]) + def milestoneable + Budget::Investment.find(params[:budget_investment_id]) end end diff --git a/app/controllers/admin/milestones_controller.rb b/app/controllers/admin/milestones_controller.rb new file mode 100644 index 000000000..dde88512b --- /dev/null +++ b/app/controllers/admin/milestones_controller.rb @@ -0,0 +1,81 @@ +class Admin::MilestonesController < Admin::BaseController + include Translatable + + before_action :load_milestoneable, only: [:index, :new, :create, :edit, :update, :destroy] + before_action :load_milestone, only: [:edit, :update, :destroy] + before_action :load_statuses, only: [:index, :new, :create, :edit, :update] + helper_method :milestoneable_path + + def index + end + + def new + @milestone = @milestoneable.milestones.new + end + + def create + @milestone = Milestone.new(milestone_params) + @milestone.milestoneable = @milestoneable + if @milestone.save + redirect_to milestoneable_path, notice: t('admin.milestones.create.notice') + else + render :new + end + end + + def edit + end + + def update + if @milestone.update(milestone_params) + redirect_to milestoneable_path, notice: t('admin.milestones.update.notice') + else + render :edit + end + end + + def destroy + @milestone.destroy + redirect_to milestoneable_path, notice: t('admin.milestones.delete.notice') + end + + private + + def milestone_params + image_attributes = [:id, :title, :attachment, :cached_attachment, :user_id, :_destroy] + documents_attributes = [:id, :title, :attachment, :cached_attachment, :user_id, :_destroy] + attributes = [:publication_date, :status_id, + translation_params(Milestone), + image_attributes: image_attributes, documents_attributes: documents_attributes] + + params.require(:milestone).permit(*attributes) + end + + def load_milestoneable + @milestoneable = milestoneable + end + + def milestoneable + raise "Implement in subclass" + end + + def load_milestone + @milestone = get_milestone + end + + def get_milestone + Milestone.find(params[:id]) + end + + def load_statuses + @statuses = Milestone::Status.all + end + + def resource + get_milestone + end + + def milestoneable_path + polymorphic_path([:admin, *resource_hierarchy_for(@milestone.milestoneable)]) + end +end diff --git a/app/views/admin/budget_investment_milestones/_form.html.erb b/app/views/admin/milestones/_form.html.erb similarity index 100% rename from app/views/admin/budget_investment_milestones/_form.html.erb rename to app/views/admin/milestones/_form.html.erb diff --git a/app/views/admin/budget_investment_milestones/edit.html.erb b/app/views/admin/milestones/edit.html.erb similarity index 68% rename from app/views/admin/budget_investment_milestones/edit.html.erb rename to app/views/admin/milestones/edit.html.erb index 7a3a1f175..297ebccb4 100644 --- a/app/views/admin/budget_investment_milestones/edit.html.erb +++ b/app/views/admin/milestones/edit.html.erb @@ -3,5 +3,5 @@

<%= t("admin.milestones.edit.title") %>

- <%= render '/admin/budget_investment_milestones/form' %> + <%= render "form" %>
diff --git a/app/views/admin/budget_investment_milestones/new.html.erb b/app/views/admin/milestones/new.html.erb similarity index 100% rename from app/views/admin/budget_investment_milestones/new.html.erb rename to app/views/admin/milestones/new.html.erb From ae22cd247a5c3bf62fa53839722fa397a4df53b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Tue, 13 Nov 2018 11:13:59 +0100 Subject: [PATCH 4/5] Use milestoneable to find/create a milestone This way we simplify the code and avoid strange cases like `params[:id]` having an ID which doesn't belong to the current milestoneable. --- app/controllers/admin/milestones_controller.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/app/controllers/admin/milestones_controller.rb b/app/controllers/admin/milestones_controller.rb index dde88512b..9a29f6015 100644 --- a/app/controllers/admin/milestones_controller.rb +++ b/app/controllers/admin/milestones_controller.rb @@ -14,8 +14,7 @@ class Admin::MilestonesController < Admin::BaseController end def create - @milestone = Milestone.new(milestone_params) - @milestone.milestoneable = @milestoneable + @milestone = @milestoneable.milestones.new(milestone_params) if @milestone.save redirect_to milestoneable_path, notice: t('admin.milestones.create.notice') else @@ -64,7 +63,7 @@ class Admin::MilestonesController < Admin::BaseController end def get_milestone - Milestone.find(params[:id]) + @milestoneable.milestones.find(params[:id]) end def load_statuses From 35c18688e5d06e68216565193de246af5223f674 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Thu, 15 Nov 2018 12:08:04 +0100 Subject: [PATCH 5/5] Remove obsolete method It was used in the `Translatable` concern, but it isn't used there anymore. --- app/controllers/admin/milestones_controller.rb | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/app/controllers/admin/milestones_controller.rb b/app/controllers/admin/milestones_controller.rb index 9a29f6015..13a277957 100644 --- a/app/controllers/admin/milestones_controller.rb +++ b/app/controllers/admin/milestones_controller.rb @@ -59,21 +59,13 @@ class Admin::MilestonesController < Admin::BaseController end def load_milestone - @milestone = get_milestone - end - - def get_milestone - @milestoneable.milestones.find(params[:id]) + @milestone = @milestoneable.milestones.find(params[:id]) end def load_statuses @statuses = Milestone::Status.all end - def resource - get_milestone - end - def milestoneable_path polymorphic_path([:admin, *resource_hierarchy_for(@milestone.milestoneable)]) end