Merge pull request #1822 from DMPRoadmap/issue-1570

Refactor routes for better maintainability

app/controllers/application_controller.rb

@@ -4,14 +4,18 @@ class ApplicationController < ActionController::Base
   # Look for template overrides before rendering
   before_filter :prepend_view_paths
 
+  before_filter :set_gettext_locale
+
+  after_filter :store_location
 
   include GlobalHelpers
   include Pundit
   helper_method GlobalHelpers.instance_methods
 
-
   rescue_from Pundit::NotAuthorizedError, with: :user_not_authorized
 
+  private
+
   def user_not_authorized
     if user_signed_in?
       redirect_to plans_url, alert: _('You are not authorized to perform this action.')
@@ -20,23 +24,11 @@ def user_not_authorized
     end
   end
 
-  before_filter :set_gettext_locale
-
-  after_filter :store_location
-
   # Sets FastGettext locale for every request made
   def set_gettext_locale
     FastGettext.locale = session[:locale] || FastGettext.default_locale
   end
 
-  # PATCH /locale/:locale REST method
-  def set_locale_session
-    if FastGettext.default_available_locales.include?(params[:locale])
-      session[:locale] = params[:locale]
-    end
-    redirect_to(request.referer || root_path) #redirects the user to URL where she/he was when the request to this resource was made or root if none is encountered
-  end
-
   def store_location
     # store last url - this is needed for post-login redirect to whatever the user last visited.
     unless ["/users/sign_in",
@@ -96,8 +88,6 @@ def success_message(obj_name, action)
     "#{_('Successfully %{action} your %{object}.') % {object: obj_name, action: action}}"
   end
 
-  private
-
   # Override rails default render action to look for a branded version of a
   # template instead of using the default one. If no override exists, the
   # default version in ./app/views/[:controller]/[:action] will be used

app/controllers/concerns/paginable.rb

@@ -1,21 +1,31 @@
+# frozen_string_literal: true
+
 module Paginable
   extend ActiveSupport::Concern
 
-  # frozen_string_literal: true
-
   ##
   # Regex to validate sort_field param is safe
   SORT_COLUMN_FORMAT = /[\w\_]+\.[\w\_]/
 
+  private
 
   # Renders paginable layout with the partial view passed
-  # partial {String} - Represents a path to where the partial view is stored
-  # controller {String} - Represents the name of the controller to handles the pagination
-  # action {String} - Represents the method name within the controller
-  # path_params {Hash} - A hash of additional URL path parameters (e.g. path_paths = { id: 'foo' } for /paginable/templates/:id/history/:page)
-  # query_params {Hash} - A hash of query parameters used to merge with params object from the controller for which this concern is included
-  # scope {ActiveRecord::Relation} - Represents scope variable
-  # locals {Hash} - A hash objects with any additional local variables to be passed to the partial view
+  #
+  # partial       - A String, represents a path to where the partial view is stored
+  # controller    - A String, represents the name of the controller to handles the
+  #                 pagination
+  # action        - A String, represents the action name within the controller
+  # path_params   - A Hash of additional URL path parameters
+  #                 (e.g. path_paths = { id: 'foo' } for
+  #                 /paginable/templates/:id/history/:page)
+  # query_params  - A hash of query parameters used to merge with params object
+  #                 from the controller for which this concern is included
+  # scope         - An {ActiveRecord::Relation}, represents scope variable
+  # locals        - A Hash objects with any additional local variables to be passed to
+  #                 the partial view
+  #
+  # Returns String of valid HTML
+  # Raises ArgumentError
   def paginable_renderise(partial: nil, controller: nil, action: nil, path_params: {}, query_params: {}, scope: nil, locals: {}, **options)
     raise ArgumentError, _('scope should be an ActiveRecord::Relation object') unless scope.is_a?(ActiveRecord::Relation)
     raise ArgumentError, _('path_params should be a Hash object') unless path_params.is_a?(Hash)
@@ -29,8 +39,10 @@ def paginable_renderise(partial: nil, controller: nil, action: nil, path_params:
     @paginable_params = params.symbolize_keys
     @paginable_params[:controller] = controller if controller
     @paginable_params[:action] = action if action
-    @paginable_params = query_params.symbolize_keys.merge(@paginable_params) # if duplicate keys, those from @paginable_params take precedence
-    # Additional path_params passed to this function got special treatment (e.g. it is taking into account when building base_url)
+    # if duplicate keys, those from @paginable_params take precedence
+    @paginable_params = query_params.symbolize_keys.merge(@paginable_params)
+    # Additional path_params passed to this function got special treatment (e.g. it is
+    # taking into account when building base_url)
     @paginable_path_params = path_params.symbolize_keys
     if @paginable_params[:page] == 'ALL' && @paginable_params[:search].blank? && @paginable_options[:view_all] == false
       render(status: :forbidden, html: _('Restricted access to View All the records'))
@@ -50,8 +62,9 @@ def paginable_base_url(page = 1)
       action: @paginable_params[:action], page: page }))
   end
 
-  # Returns the base url of the paginable router for a given page passed together with its query_params.
-  # It is used to retain context, i.e. search, sort_field, sort_direction, etc
+  # Returns the base url of the paginable router for a given page passed together with
+  # its query_params. It is used to retain context, i.e. search, sort_field,
+  # sort_direction, etc
   def paginable_base_url_with_query_params(page: 1, **stringify_query_params_options)
     base_url = paginable_base_url(page)
     stringified_query_params = stringify_query_params(stringify_query_params_options)
@@ -77,14 +90,13 @@ def paginable?
     return @refined_scope.respond_to?(:total_pages)
   end
 
-  private
-
-  # Returns the upcase string (e.g ASC or DESC) if sort_direction param is present in any of the forms 'asc', 'desc', 'ASC', 'DESC'
-  # otherwise returns ASC
+  # Returns the upcase string (e.g ASC or DESC) if sort_direction param is present in any
+  # of the forms 'asc', 'desc', 'ASC', 'DESC' otherwise returns ASC
   def upcasing_sort_direction(direction = @paginable_params[:sort_direction])
     directions = ['asc', 'desc', 'ASC', 'DESC']
     return directions.include?(direction) ? direction.upcase : 'ASC'
   end
+
   # Returns DESC when ASC is passed and vice versa, otherwise nil
   def swap_sort_direction(direction = @paginable_params[:sort_direction])
     direction_upcased = upcasing_sort_direction(direction)

app/controllers/concerns/versionable.rb

@@ -1,5 +1,7 @@
 module Versionable
 
+  private
+
   # Takes in a Template, phase, Section, Question, or Annotaion
   # IF the template is published, generates a new template
   # finds the passed object in the new template
@@ -72,8 +74,6 @@ def get_new(obj)
     return obj
   end
 
-  private
-
   # Locates an object (e.g. phase, section, question, annotation) in a
   # search_space
   # (e.g. phases/sections/questions/annotations) by comparing either the number

app/controllers/plans_controller.rb

@@ -3,7 +3,6 @@ class PlansController < ApplicationController
   include ConditionalUserMailer
   helper PaginableHelper
   helper SettingsTemplateHelper
-  include FeedbacksHelper
 
   after_action :verify_authorized, except: [:overview]
 

app/controllers/session_locales_controller.rb

@@ -0,0 +1,8 @@
+class SessionLocalesController < ApplicationController
+  def update
+    if FastGettext.default_available_locales.include?(params[:locale])
+      session[:locale] = params[:locale]
+    end
+    redirect_to(:back)
+  end
+end

app/controllers/static_pages_controller.rb

@@ -1,22 +1,26 @@
 class StaticPagesController < ApplicationController
 
   def about_us
-		dcc_news_feed_url = "http://www.dcc.ac.uk/news/dmponline-0/feed"
-		@dcc_news_feed = Feedjira::Feed.fetch_and_parse dcc_news_feed_url
-		respond_to do |format|
-			format.rss { redirect_to dcc_news_feed_url }
-			format.html
-		end
+    dcc_news_feed_url = "http://www.dcc.ac.uk/news/dmponline-0/feed"
+    @dcc_news_feed = Feedjira::Feed.fetch_and_parse dcc_news_feed_url
+    respond_to do |format|
+      format.rss { redirect_to dcc_news_feed_url }
+      format.html
+    end
   end
 
   def contact_us
   end
 
   def roadmap
-
   end
 
   def privacy
   end
 
+  def termsuse
+  end
+
+  def help
+  end
 end

app/policies/guidance_policy.rb

@@ -46,20 +46,4 @@ def admin_publish?
   def admin_unpublish?
     user.can_modify_guidance?
   end
-
-  def update_phases?
-    user.can_modify_guidance?
-  end
-
-  def update_versions?
-    user.can_modify_guidance?
-  end
-
-  def update_sections?
-    user.can_modify_guidance?
-  end
-
-  def update_questions?
-    user.can_modify_guidance?
-  end
 end

app/policies/plan_policy.rb

@@ -3,7 +3,7 @@ class PlanPolicy < ApplicationPolicy
   attr_reader :plan
 
   def initialize(user, plan)
-    raise Pundit::NotAuthorizedError, _("must be logged in") unless user 
+    raise Pundit::NotAuthorizedError, _("must be logged in") unless user
     raise Pundit::NotAuthorizedError, _("are not authorized to view that plan") unless plan || plan.publicly_visible?
     @user = user
     @plan = plan
@@ -14,7 +14,7 @@ def show?
   end
 
   def share?
-    @plan.editable_by?(@user.id) 
+    @plan.editable_by?(@user.id)
   end
 
   def export?
@@ -72,8 +72,4 @@ def select_guidances_list?
   def update_guidances_list?
     @plan.editable_by?(@user.id)
   end
-
-  def phase_status?
-    @plan.readable_by?(@user.id)
-  end
 end