diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 85f55f1..8071ec0 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -5,7 +5,7 @@ * If you would like to contribute to the project, please follow these steps to submit a contribution: * Comment on the Github issue (or create one if one does not exist) and let us know that you're working on it. - * Fork the project (if you have not already) or rebase your fork so that it is up to date with the current repository's **`development`** branch + * Fork the project (if you have not already) or rebase your fork so that it is up to date with the current repository's **`contributions`** branch * Create a new branch in your fork. This will ensure that you are able to work at your own pace and continue to pull in any updates made to this project. * Make your changes in the new branch. When you have finished your work (e.g. 3 commits), squash all the commits on the branch that you are working on: ```bash @@ -28,15 +28,15 @@ # Rebase 6c51182..59df9aa onto 6c51182 ``` Then, change `pick` to `squash` for the 2nd and 3rd commits (to squash them into the single first commit). - * To make sure that your version of the **`development`** branch is still up to date with this project, switch to it and synchronise: + * To make sure that your version of the **`contributions`** branch is still up to date with this project, switch to it and synchronise: ```bash - git checkout development - git pull origin development + git checkout contributions + git pull origin contributions ``` * Switch back to your feature branch and rebase: ```bash git checkout - git rebase development + git rebase contributions ``` * Fix merge conflicts (if any encountered) and then push to your fork: ```bash @@ -55,7 +55,7 @@ ## GitHub Workflow A contribution consists of any work that is voluntarily submitted to the project. This includes bug fixes, enhancements and documentation that is intended as an improvement to the DMP Roadmap system. -Any individual with a GitHub account may propose a Contribution by submitting a Pull Request (PR) to this project's **`contributions`** branch. The project team will evaluate each PR as time permits and communicate with the contributor via comments on the PR. We will not accept a contribution until it adheres to the guidelines outlined in this document. If your contribution fits well with the development roadmap, the team will merge it into the project and schedule it for the next upcoming release. +Any individual with a GitHub account may propose a Contribution by submitting a Pull Request (PR) to this project's **`contributions`** branch. The project team will evaluate each PR as time permits and communicate with the contributor via comments on the PR. We will not accept a contribution until it adheres to the guidelines outlined in this document. If your contribution fits well with the project roadmap, the team will merge it into the project and schedule it for the next upcoming release. ![GitHub Workflow ](https://github.com/DMPRoadmap/roadmap/blob/master/public/github-contributor-infographic-final.png) diff --git a/Gemfile b/Gemfile index d7851c3..b93b341 100644 --- a/Gemfile +++ b/Gemfile @@ -20,11 +20,6 @@ gem 'flag_shih_tzu', '~> 0.3' # Allows for bitfields in activereccord # ------------------------------------------------ -# JS <-> RUBY BRIDGE -gem 'libv8', '~> 3.16' -gem 'therubyracer', '>=0.11.4', platforms: :ruby - -# ------------------------------------------------ # JSON DSL - USED BY API gem 'jbuilder', '~> 2.6.0' diff --git a/Gemfile.lock b/Gemfile.lock index 8365c2a..48b0ac9 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -168,7 +168,6 @@ kaminari-core (1.0.1) ledermann-rails-settings (2.4.2) activerecord (>= 3.1) - libv8 (3.16.14.15) locale (2.1.2) loofah (2.0.3) nokogiri (>= 1.5.9) @@ -264,7 +263,6 @@ recaptcha (4.1.0) json redcarpet (3.3.4) - ref (2.0.0) responders (2.3.0) railties (>= 4.2.0, < 5.1) rolify (5.1.0) @@ -298,9 +296,6 @@ activesupport (>= 3) rails (>= 3) text (1.3.1) - therubyracer (0.12.2) - libv8 (~> 3.16.14.0) - ref thin (1.7.0) daemons (~> 1.0, >= 1.0.9) eventmachine (~> 1.0, >= 1.0.4) @@ -350,7 +345,6 @@ jbuilder (~> 2.6.0) kaminari (>= 1.0) ledermann-rails-settings (~> 2.4.2) - libv8 (~> 3.16) minitest-rails-capybara (~> 2.1.2) minitest-reporters (~> 1.1.11) mysql2 (~> 0.3.18) @@ -372,7 +366,6 @@ simplecov (~> 0.12) sqlite3 (~> 1.3.12) swagger-docs (>= 0.2.9) - therubyracer (>= 0.11.4) thin (~> 1.7) web-console (~> 2.3.0) webmock (~> 2.1.0) @@ -385,4 +378,4 @@ ruby 2.2.2p95 BUNDLED WITH - 1.13.7 + 1.16.1 diff --git a/app/controllers/concerns/paginable.rb b/app/controllers/concerns/paginable.rb index 76340d3..2d900a7 100644 --- a/app/controllers/concerns/paginable.rb +++ b/app/controllers/concerns/paginable.rb @@ -10,30 +10,48 @@ # 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 - def paginable_renderise(partial: nil, controller: params[:controller], action: params[:action], path_params: {}, query_params: {}, scope: nil, locals: {}) - 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) - raise ArgumentError, 'query_params should be a Hash object' unless query_params.is_a?(Hash) - raise ArgumentError, 'locals should be a Hash object' unless locals.is_a?(Hash) - @paginable_controller = controller - @paginable_action = action - @paginable_path_params = path_params - merge_query_params(query_params) - refined_scope = refine_query(scope) - render(layout: "/layouts/paginable", partial: partial, locals: { - controller: controller, - action: action, - paginable: refined_scope.respond_to?(:total_pages), - scope: refined_scope }.merge(locals)) + 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) + raise ArgumentError, _('query_params should be a Hash object') unless query_params.is_a?(Hash) + raise ArgumentError, _('locals should be a Hash object') unless locals.is_a?(Hash) + + # Default options + @paginable_options = {}.merge(options) + @paginable_options[:view_all] = options.fetch(:view_all, true) + # Assignment for paginable_params based on arguments passed to the method + @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) + @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')) + else + @refined_scope = refine_query(scope) + render(layout: "/layouts/paginable", + partial: partial, + locals: locals.merge({ + scope: @refined_scope, + search_term: @paginable_params[:search] })) + end end # Returns the base url of the paginable route for a given page passed def paginable_base_url(page = 1) - options = { controller: @paginable_controller || params[:controller], - action: @paginable_action || params[:action], page: page } - if @paginable_path_params.present? - options = @paginable_path_params.merge(options) + return url_for(@paginable_path_params.merge({ controller: @paginable_params[:controller], + 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 + 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) + if stringified_query_params.present? + return "#{base_url}?#{stringified_query_params}" end - return url_for(options) + return base_url end # Generates an HTML link to sort given a sort field. # sort_field {String} - Represents the column name for a table @@ -42,76 +60,71 @@ end # Determines whether or not the latest request included the search functionality def searchable? - return params[:search].present? + return @paginable_params[:search].present? end - # Generates an HTML link with search functionality (if latest request included the search functionality) - # text {String} - Represents the text for the searchable link - # page {String | Fixnum } - Represents the page to request for a search term - def paginable_search_link(text = _('link name'), page = 1) - url = paginable_base_url(page) - url+= "?search=#{params[:search]}" if searchable? - return link_to(text, url, 'data-remote': true) + # Determines whether or not the scoped query is paginated or not + def paginable? + return @refined_scope.respond_to?(:total_pages) end end private - # Attemps to merge query_params into params hash unless a key is already present at params - def merge_query_params(query_params = {}) - query_params.each_pair.reduce(params) do |m, o| - key = o[0].to_sym - if m[key].nil? - m[key] = o[1].to_s - end - m - end - end # 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 nil - def sort_direction - if params[:sort_direction].present? - directions = ['asc', 'desc', 'ASC', 'DESC'] - return directions.include?(params[:sort_direction]) ? params[:sort_direction].upcase : 'ASC' - end - return nil + # 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) - return 'DESC' if direction == 'ASC' - return 'ASC' if direction == 'DESC' + def swap_sort_direction(direction = @paginable_params[:sort_direction]) + direction_upcased = upcasing_sort_direction(direction) + return 'DESC' if direction_upcased == 'ASC' + return 'ASC' if direction_upcased == 'DESC' end # Refine a scope passed to this concern if any of the params (search, sort_field or page) are present def refine_query(scope) - scope = scope.search(params[:search]) if params[:search].present? # Can raise NoMethodError if the scope does not define a search method - if params[:sort_field].present? - direction = sort_direction - scope = direction.present? ? scope.order("#{params[:sort_field]} #{direction}") : scope.order("#{params[:sort_field]}") # Can raise ActiveRecord::StatementInvalid (e.g. column does not exist, ambiguity on column, etc) + scope = scope.search(@paginable_params[:search]) if @paginable_params[:search].present? # Can raise NoMethodError if the scope does not define a search method + if @paginable_params[:sort_field].present? + scope = scope.order("#{@paginable_params[:sort_field]} #{upcasing_sort_direction}") # Can raise ActiveRecord::StatementInvalid (e.g. column does not exist, ambiguity on column, etc) end - if params[:page] != 'ALL' - scope = scope.page(params[:page]) # Can raise error if page is not a number + if @paginable_params[:page] != 'ALL' + scope = scope.page(@paginable_params[:page]) # Can raise error if page is not a number end return scope end # Returns the sort link name for a given sort_field. The link name includes html prevented of being escaped def sort_link_name(sort_field) className = 'fa-sort' - direction = sort_direction - if direction.present? && params[:sort_field] == sort_field - className = direction == 'ASC'? 'fa-sort-asc' : 'fa-sort-desc' + if @paginable_params[:sort_field] == sort_field + className = upcasing_sort_direction == 'ASC'? 'fa-sort-asc' : 'fa-sort-desc' end return raw("") end # Returns the sort url for a given sort_field. def sort_link_url(sort_field) - page = params[:page] == 'ALL' ? 'ALL' : 1 # Retain ALL param page if latest request included it - url = paginable_base_url(page)+"?" - query_string = [] - query_string << "search=#{params[:search]}" if params[:search].present? - direction = sort_direction - if direction.present? && params[:sort_field] == sort_field - query_string << "sort_direction=#{swap_sort_direction(direction)}" + page = @paginable_params[:page] == 'ALL' ? 'ALL' : 1 + if @paginable_params[:sort_field] == sort_field + return paginable_base_url_with_query_params( + page: page, + sort_field: sort_field, + sort_direction: swap_sort_direction) else - query_string << "sort_direction=ASC" + return paginable_base_url_with_query_params( + page: page, + sort_field: sort_field) end - query_string << "sort_field=#{sort_field}" - return url+query_string.join('&') + end + def stringify_query_params( + search: @paginable_params[:search], + sort_field: @paginable_params[:sort_field], + sort_direction: nil) + + query_string = [] + query_string << "search=#{search}" if search.present? + if sort_field.present? + query_string << "sort_field=#{sort_field}" + direction = sort_direction || upcasing_sort_direction + query_string << "sort_direction=#{direction}" + end + return query_string.join('&') end end \ No newline at end of file diff --git a/app/controllers/paginable/plans_controller.rb b/app/controllers/paginable/plans_controller.rb index 8d29990..d691dda 100644 --- a/app/controllers/paginable/plans_controller.rb +++ b/app/controllers/paginable/plans_controller.rb @@ -9,8 +9,7 @@ def organisationally_or_publicly_visible raise Pundit::NotAuthorizedError unless Paginable::PlanPolicy.new(current_user).organisationally_or_publicly_visible? paginable_renderise(partial: 'organisationally_or_publicly_visible', - scope: Plan.organisationally_or_publicly_visible(current_user), - query_params: { sort_field: 'plans.title', sort_direction: :asc }) + scope: Plan.organisationally_or_publicly_visible(current_user)) end # GET /paginable/plans/publicly_visible/:page def publicly_visible diff --git a/app/controllers/paginable/users_controller.rb b/app/controllers/paginable/users_controller.rb index a39eb75..d2a8110 100644 --- a/app/controllers/paginable/users_controller.rb +++ b/app/controllers/paginable/users_controller.rb @@ -8,6 +8,6 @@ else scope = current_user.org.users.includes(:roles) end - paginable_renderise(partial: 'index', scope: scope) + paginable_renderise(partial: 'index', scope: scope, view_all: !current_user.can_super_admin?) end end \ No newline at end of file diff --git a/app/controllers/plans_controller.rb b/app/controllers/plans_controller.rb index ac9ec75..de4c2d5 100644 --- a/app/controllers/plans_controller.rb +++ b/app/controllers/plans_controller.rb @@ -164,7 +164,7 @@ @plan.save if @plan.update_attributes(attrs) - format.html { redirect_to @plan, :editing => false, notice: success_message(_('plan'), _('saved')) } + format.html { redirect_to overview_plan_path(@plan), notice: success_message(_('plan'), _('saved')) } format.json {render json: {code: 1, msg: success_message(_('plan'), _('saved'))}} else flash[:alert] = failed_update_error(@plan, _('plan')) diff --git a/app/controllers/public_pages_controller.rb b/app/controllers/public_pages_controller.rb index 9307c4a..f64e61d 100644 --- a/app/controllers/public_pages_controller.rb +++ b/app/controllers/public_pages_controller.rb @@ -29,7 +29,7 @@ render pdf: file_name, margin: @formatting[:margin], footer: { - center: _('Template created using the %{application_name}. Last modified %{date}') % {application_name: Rails.configuration.branding[:application][:name], date: l(@template.updated_at.to_date, formats: :short)}, + center: _('Template created using the %{application_name} service. Last modified %{date}') % {application_name: Rails.configuration.branding[:application][:name], date: l(@template.updated_at.to_date, formats: :short)}, font_size: 8, spacing: (@formatting[:margin][:bottom] / 2) - 4, right: '[page] of [topage]' @@ -69,7 +69,7 @@ render pdf: file_name, margin: @formatting[:margin], footer: { - center: _('Created using the %{application_name}. Last modified %{date}') % {application_name: Rails.configuration.branding[:application][:name], date: l(@plan.updated_at.to_date, formats: :short)}, + center: _('Created using the %{application_name} service. Last modified %{date}') % {application_name: Rails.configuration.branding[:application][:name], date: l(@plan.updated_at.to_date, formats: :short)}, font_size: 8, spacing: (@formatting[:margin][:bottom] / 2) - 4, right: '[page] of [topage]' diff --git a/app/controllers/super_admin/orgs_controller.rb b/app/controllers/super_admin/orgs_controller.rb index 72d338b..a208f67 100644 --- a/app/controllers/super_admin/orgs_controller.rb +++ b/app/controllers/super_admin/orgs_controller.rb @@ -21,6 +21,8 @@ org.logo = params[:logo] if params[:logo] if params[:org_links].present? org.links = JSON.parse(params[:org_links]) + else + org.links = { org: [] } end begin diff --git a/app/controllers/super_admin/users_controller.rb b/app/controllers/super_admin/users_controller.rb new file mode 100644 index 0000000..051ce42 --- /dev/null +++ b/app/controllers/super_admin/users_controller.rb @@ -0,0 +1,47 @@ +module SuperAdmin + class UsersController < ApplicationController + + after_action :verify_authorized + + def edit + user = User.find(params[:id]) + if user.present? + authorize user + languages = Language.sorted_by_abbreviation + orgs = Org.where(parent_id: nil).order("name") + identifier_schemes = IdentifierScheme.where(active: true).order(:name) + + render 'super_admin/users/edit', + locals: { user: user, + languages: languages, + orgs: orgs, + identifier_schemes: identifier_schemes, + default_org: user.org } + else + redirect_to admin_index_users_path, alert: _('User not found.') + end + end + + def update + user = User.find(params[:id]) + if user.present? + authorize user + topic = _('%{username}\'s profile') % { username: user.name(false) } + if user.update_attributes(user_params) + redirect_to edit_super_admin_user_path(user), + notice: _('Successfully updated %{username}') % { username: topic } + else + redirect_to edit_super_admin_user_path(user), + alert: _('Unable to update %{username}') % { username: topic } + end + else + redirect_to edit_super_admin_user_path(user), alert: _('User not found.') + end + end + + private + def user_params + params.require(:super_admin_user).permit(:email, :firstname, :surname, :org_id, :language_id) + end + end +end \ No newline at end of file diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 4737072..5da6539 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -52,6 +52,7 @@ authorize @user perms_ids = params[:perm_ids].blank? ? [] : params[:perm_ids].map(&:to_i) perms = Perm.where( id: perms_ids) + privileges_changed = false current_user.perms.each do |perm| if @user.perms.include? perm if ! perms.include? perm @@ -59,20 +60,24 @@ if perm.id == Perm.use_api.id @user.remove_token! end + privileges_changed = true end else if perms.include? perm @user.perms << perm if perm.id == Perm.use_api.id @user.keep_or_generate_token! + privileges_changed = true end end end end if @user.save! - deliver_if(recipients: @user, key: 'users.admin_privileges') do |r| - UserMailer.admin_privileges(r).deliver_now + if privileges_changed + deliver_if(recipients: @user, key: 'users.admin_privileges') do |r| + UserMailer.admin_privileges(r).deliver_now + end end render(json: { code: 1, @@ -123,6 +128,29 @@ end end + # PUT /users/:id/activate + # ----------------------------------------------------- + def activate + authorize current_user + + user = User.find(params[:id]) + if user.present? + begin + user.active = !user.active + user.save! + render json: { + code: 1, + msg: _('Successfully %{action} %{username}\'s account.') % { action: user.active ? _('activated') : _('deactivated'), username: user.name(false) } + } + rescue Exception + render json: { + code: 0, + msg: _('Unable to %{action} %{username}') % { action: user.active ? _('activate') : _('deactivate'), username: user.name(false) } + } + end + end + end + private def org_swap_params params.require(:user).permit(:org_id, :org_name) diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb index b54bf09..6fa76b0 100644 --- a/app/mailers/user_mailer.rb +++ b/app/mailers/user_mailer.rb @@ -23,9 +23,11 @@ def permissions_change_notification(role, user) @role = role @user = user - FastGettext.with_locale FastGettext.default_locale do - mail(to: @role.user.email, - subject: _('Changed permissions on a Data Management Plan in %{tool_name}') %{ :tool_name => Rails.configuration.branding[:application][:name] }) + if user.active? + FastGettext.with_locale FastGettext.default_locale do + mail(to: @role.user.email, + subject: _('Changed permissions on a Data Management Plan in %{tool_name}') %{ :tool_name => Rails.configuration.branding[:application][:name] }) + end end end @@ -33,24 +35,18 @@ @user = user @plan = plan @current_user = current_user - FastGettext.with_locale FastGettext.default_locale do - mail(to: @user.email, - subject: "#{_('Permissions removed on a DMP in %{tool_name}') %{ :tool_name => Rails.configuration.branding[:application][:name] }}") - end - end - - def api_token_granted_notification(user) - @user = user + if user.active? FastGettext.with_locale FastGettext.default_locale do mail(to: @user.email, - subject: _('API rights in %{tool_name}') %{ :tool_name => Rails.configuration.branding[:application][:name] }) + subject: "#{_('Permissions removed on a DMP in %{tool_name}') %{ :tool_name => Rails.configuration.branding[:application][:name] }}") end + end end def feedback_notification(recipient, plan, requestor) @user = requestor - if @user.org.present? + if @user.org.present? && recipient.active? @org = @user.org @plan = plan @recipient = recipient @@ -67,16 +63,18 @@ @user = recipient @plan = plan - FastGettext.with_locale FastGettext.default_locale do - mail(to: recipient.email, - subject: _("%{application_name}: Expert feedback has been provided for %{plan_title}") % {application_name: Rails.configuration.branding[:application][:name], plan_title: @plan.title}) + if recipient.active? + FastGettext.with_locale FastGettext.default_locale do + mail(to: recipient.email, + subject: _("%{application_name}: Expert feedback has been provided for %{plan_title}") % {application_name: Rails.configuration.branding[:application][:name], plan_title: @plan.title}) + end end end def feedback_confirmation(recipient, plan, requestor) user = requestor - if user.org.present? + if user.org.present? && recipient.active? org = user.org plan = plan @@ -96,9 +94,11 @@ def plan_visibility(user, plan) @user = user @plan = plan - FastGettext.with_locale FastGettext.default_locale do - mail(to: @user.email, - subject: _('DMP Visibility Changed: %{plan_title}') %{ :plan_title => @plan.title }) + if user.active? + FastGettext.with_locale FastGettext.default_locale do + mail(to: @user.email, + subject: _('DMP Visibility Changed: %{plan_title}') %{ :plan_title => @plan.title }) + end end end @@ -106,7 +106,8 @@ # @param plan - Plan for which the comment is associated to def new_comment(commenter, plan) if commenter.is_a?(User) && plan.is_a?(Plan) - if plan.owner.present? + owner = plan.owner + if owner.present? && owner.active? @commenter = commenter @plan = plan FastGettext.with_locale FastGettext.default_locale do @@ -119,9 +120,11 @@ def admin_privileges(user) @user = user - FastGettext.with_locale FastGettext.default_locale do - mail(to: user.email, subject: - _('Administrator privileges granted in %{tool_name}') %{ :tool_name => Rails.configuration.branding[:application][:name] }) + if user.active? + FastGettext.with_locale FastGettext.default_locale do + mail(to: user.email, subject: + _('Administrator privileges granted in %{tool_name}') %{ :tool_name => Rails.configuration.branding[:application][:name] }) + end end end end diff --git a/app/models/plan.rb b/app/models/plan.rb index 229cb24..92fc9ed 100644 --- a/app/models/plan.rb +++ b/app/models/plan.rb @@ -283,8 +283,8 @@ def guidance_by_question_as_hash # Get all of the selected guidance groups for the plan guidance_groups_ids = self.guidance_groups.collect(&:id) - guidance_groups = GuidanceGroup.joins(:org).where("guidance_groups.published = ? AND guidance_groups.id IN (?) AND orgs.id != ?", - true, guidance_groups_ids, self.template.org.id) + guidance_groups = GuidanceGroup.joins(:org).where("guidance_groups.published = ? AND guidance_groups.id IN (?)", + true, guidance_groups_ids) # Gather all of the Themes used in the plan as a hash # { diff --git a/app/models/user.rb b/app/models/user.rb index 4fca8fc..0bee5c5 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -50,8 +50,8 @@ # Retrieves all of the org_admins for the specified org scope :org_admins, -> (org_id) { - joins(:perms).where("users.org_id = ? AND perms.name IN (?)", org_id, - ['grant_permissions', 'modify_templates', 'modify_guidance', 'change_org_details']) + joins(:perms).where("users.org_id = ? AND perms.name IN (?) AND users.active = ?", org_id, + ['grant_permissions', 'modify_templates', 'modify_guidance', 'change_org_details'], true) } scope :search, -> (term) { @@ -67,6 +67,12 @@ after_update :when_org_changes + ## + # This method uses Devise's built-in handling for inactive users + def active_for_authentication? + super && self.active? + end + # EVALUATE CLASS AND INSTANCE METHODS BELOW # # What do they do? do they do it efficiently, and do we need them? diff --git a/app/policies/plan_policy.rb b/app/policies/plan_policy.rb index 870cd41..136ae45 100644 --- a/app/policies/plan_policy.rb +++ b/app/policies/plan_policy.rb @@ -14,7 +14,7 @@ end def share? - @plan.editable_by?(@user.id) + @plan.editable_by?(@user.id) end def export? diff --git a/app/policies/user_policy.rb b/app/policies/user_policy.rb index 070e52f..f9f6f47 100644 --- a/app/policies/user_policy.rb +++ b/app/policies/user_policy.rb @@ -28,6 +28,18 @@ user.can_super_admin? end + def activate? + user.can_super_admin? + end + + def edit? + user.can_super_admin? + end + + def update? + user.can_super_admin? + end + class Scope < Scope def resolve scope.where(org_id: user.org_id) diff --git a/app/views/devise/registrations/_personal_details.html.erb b/app/views/devise/registrations/_personal_details.html.erb index 0bc6bda..7ab72c1 100644 --- a/app/views/devise/registrations/_personal_details.html.erb +++ b/app/views/devise/registrations/_personal_details.html.erb @@ -1,6 +1,6 @@ <%= form_for(resource, as: resource_name, url: registration_path(resource_name), html: {method: :put, id: 'personal_details_registration_form' }) do |f| %>

- <%= _("Please note that your email address is used as your username. + <%= raw _("Please note that your email address is used as your username. If you change this, remember to use your new email address on sign in.") %>

diff --git a/app/views/guidances/admin_index.html.erb b/app/views/guidances/admin_index.html.erb index 03bf76c..b2500e8 100644 --- a/app/views/guidances/admin_index.html.erb +++ b/app/views/guidances/admin_index.html.erb @@ -3,7 +3,7 @@

<%= _('Guidance') %>

- <%= _("First create a guidance group. This could be organisation wide or a subset e.g. a particular College / School, Institute or department. When you create guidance you'll be asked to assign it to a guidance group.") %> + <%= raw _("First create a guidance group. This could be organisation wide or a subset e.g. a particular College / School, Institute or department. When you create guidance you'll be asked to assign it to a guidance group.") %>

diff --git a/app/views/layouts/_paginable.html.erb b/app/views/layouts/_paginable.html.erb index d50573b..2bc62d2 100644 --- a/app/views/layouts/_paginable.html.erb +++ b/app/views/layouts/_paginable.html.erb @@ -1,13 +1,13 @@ <% # Custom layout to be included on any view that needs pagination - # locals: { controller, action, paginable, scope } + # locals: { scope, search_term } %> -<% total = paginable ? scope.total_count : scope.length %> +<% total = paginable? ? scope.total_count : scope.length %>
- <%= render(partial: '/shared/search', locals: { controller: controller, action: action }) if searchable? || total > Kaminari.config.default_per_page %> + <%= render(partial: '/shared/search', locals: { search_term: search_term }) if searchable? || total > Kaminari.config.default_per_page %>
@@ -29,29 +29,29 @@ <% if total > Kaminari.config.default_per_page %> <% if searchable? %> <% else %> - <% if paginable %> - <%= link_to(_('View all'), paginable_base_url('ALL'), { 'data-remote': true }) %> + <% if paginable? %> + <%= link_to(_('View all'), paginable_base_url_with_query_params(page: 'ALL'), { 'data-remote': true }) if @paginable_options[:view_all] %> <% else %> - <%= link_to(_('View less'), paginable_base_url(1), { 'data-remote': true }) %> + <%= link_to(_('View less'), paginable_base_url_with_query_params(page: 1), { 'data-remote': true }) %> <% end %> <% end %> <% else %> <% if searchable? %> - <%= link_to(_('Clear search results'), paginable_base_url(1), { 'data-remote': true, class: 'clear' }) %> + <%= link_to(_('Clear search results'), paginable_base_url_with_query_params(page: 1, search: nil), { 'data-remote': true }) %> <% end %> <% end %>
- <% if paginable %> - <%= paginate(scope, params: { controller: controller, action: action }, remote: true) %> + <% if paginable? %> + <%= paginate(scope, params: @paginable_params, remote: true) %> <% end %>
diff --git a/app/views/paginable/plans/_privately_visible.html.erb b/app/views/paginable/plans/_privately_visible.html.erb index 31ff50b..547e0f2 100644 --- a/app/views/paginable/plans/_privately_visible.html.erb +++ b/app/views/paginable/plans/_privately_visible.html.erb @@ -55,18 +55,19 @@