Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor routes for better maintainability #1822

Merged
merged 2 commits into from
Aug 20, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 5 additions & 15 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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.')
Expand All @@ -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",
Expand Down Expand Up @@ -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
Expand Down
46 changes: 29 additions & 17 deletions app/controllers/concerns/paginable.rb
Original file line number Diff line number Diff line change
@@ -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)
Expand All @@ -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'))
Expand All @@ -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)
Expand All @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions app/controllers/concerns/versionable.rb
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion app/controllers/plans_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ class PlansController < ApplicationController
include ConditionalUserMailer
helper PaginableHelper
helper SettingsTemplateHelper
include FeedbacksHelper

after_action :verify_authorized, except: [:overview]

Expand Down
8 changes: 8 additions & 0 deletions app/controllers/session_locales_controller.rb
Original file line number Diff line number Diff line change
@@ -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
18 changes: 11 additions & 7 deletions app/controllers/static_pages_controller.rb
Original file line number Diff line number Diff line change
@@ -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
16 changes: 0 additions & 16 deletions app/policies/guidance_policy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
8 changes: 2 additions & 6 deletions app/policies/plan_policy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -14,7 +14,7 @@ def show?
end

def share?
@plan.editable_by?(@user.id)
@plan.editable_by?(@user.id)
end

def export?
Expand Down Expand Up @@ -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
Loading