diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 24de6da5..438ef0ff 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -6,6 +6,9 @@ on: - master pull_request: +permissions: + contents: read + jobs: test: strategy: diff --git a/Gemfile.lock b/Gemfile.lock index cb8982fd..01dbc4f1 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -92,7 +92,7 @@ GEM brakeman (6.1.1) racc builder (3.3.0) - bundler-audit (0.9.1) + bundler-audit (0.9.2) bundler (>= 1.2.0, < 3) thor (~> 1.0) byebug (11.1.3) @@ -250,9 +250,9 @@ GEM net-smtp (0.5.1) net-protocol nio4r (2.7.4) - nokogiri (1.18.8-arm64-darwin) + nokogiri (1.18.9-arm64-darwin) racc (~> 1.4) - nokogiri (1.18.8-x86_64-linux-gnu) + nokogiri (1.18.9-x86_64-linux-gnu) racc (~> 1.4) oauth2 (2.0.9) faraday (>= 0.17.3, < 3.0) @@ -301,7 +301,7 @@ GEM pundit (2.3.1) activesupport (>= 3.0.0) racc (1.8.1) - rack (3.1.14) + rack (3.1.16) rack-cors (2.0.2) rack (>= 2.0.0) rack-protection (4.1.1) @@ -429,7 +429,7 @@ GEM stringio (3.1.6) terminal-table (4.0.0) unicode-display_width (>= 1.1.1, < 4) - thor (1.3.2) + thor (1.4.0) timecop (0.9.8) timeout (0.4.3) tzinfo (2.0.6) diff --git a/app/controllers/measure_categories_controller.rb b/app/controllers/measure_categories_controller.rb index 71115fad..493f2bf1 100644 --- a/app/controllers/measure_categories_controller.rb +++ b/app/controllers/measure_categories_controller.rb @@ -1,5 +1,6 @@ class MeasureCategoriesController < ApplicationController - before_action :set_and_authorize_measure_category, only: [:show, :update, :destroy] + before_action :set_and_authorize_measure_category, only: [:show, :destroy] + skip_before_action :authenticate_user!, only: [:update] # GET /measure_categories def index @@ -27,19 +28,15 @@ def create end end - # PATCH/PUT /measure_categories/1 - def update - if @measure_category.update!(permitted_attributes(@measure_category)) - set_and_authorize_measure_category - render json: serialize(@measure_category) - end - end - # DELETE /measure_categories/1 def destroy @measure_category.destroy end + def update + head :not_implemented + end + private # Use callbacks to share common setup or constraints between actions. @@ -47,8 +44,19 @@ def set_and_authorize_measure_category @measure_category = policy_scope(base_object).find(params[:id]) authorize @measure_category rescue ActiveRecord::RecordNotFound - raise unless action_name == "destroy" - head :no_content + if action_name == "destroy" + record = base_object.find_by(id: params[:id]) + + if record.present? + # Record exists but is out of scope — test authorization anyway + authorize record + end + + # If we got here, it's okay to respond as deleted + head :no_content + else + raise + end end def base_object diff --git a/app/controllers/measure_indicators_controller.rb b/app/controllers/measure_indicators_controller.rb index dc6878bf..044ffaf1 100644 --- a/app/controllers/measure_indicators_controller.rb +++ b/app/controllers/measure_indicators_controller.rb @@ -1,5 +1,6 @@ class MeasureIndicatorsController < ApplicationController - before_action :set_and_authorize_measure_indicator, only: [:show, :update, :destroy] + before_action :set_and_authorize_measure_indicator, only: [:show, :destroy] + skip_before_action :authenticate_user!, only: [:update] # GET /measure_indicators def index @@ -27,25 +28,35 @@ def create end end - # PATCH/PUT /measure_indicators/1 - def update - if @measure_indicator.update!(permitted_attributes(@measure_indicator)) - set_and_authorize_measure_indicator - render json: serialize(@measure_indicator) - end - end - # DELETE /measure_indicators/1 def destroy @measure_indicator.destroy end + def update + head :not_implemented + end + private # Use callbacks to share common setup or constraints between actions. def set_and_authorize_measure_indicator @measure_indicator = policy_scope(base_object).find(params[:id]) authorize @measure_indicator + rescue ActiveRecord::RecordNotFound + if action_name == "destroy" + record = base_object.find_by(id: params[:id]) + + if record.present? + # Record exists but is out of scope — test authorization anyway + authorize record + end + + # If we got here, it's okay to respond as deleted + head :no_content + else + raise + end end def base_object diff --git a/app/controllers/recommendation_categories_controller.rb b/app/controllers/recommendation_categories_controller.rb index 25f913ea..9c994681 100644 --- a/app/controllers/recommendation_categories_controller.rb +++ b/app/controllers/recommendation_categories_controller.rb @@ -1,5 +1,6 @@ class RecommendationCategoriesController < ApplicationController - before_action :set_and_authorize_recommendation_category, only: [:show, :update, :destroy] + before_action :set_and_authorize_recommendation_category, only: [:show, :destroy] + skip_before_action :authenticate_user!, only: [:update] # GET /recommendation_categories def index @@ -27,18 +28,15 @@ def create end end - # PATCH/PUT /recommendation_categories/1 - def update - if @recommendation_category.update!(permitted_attributes(@recommendation_category)) - render json: serialize(@recommendation_category) - end - end - # DELETE /recommendation_categories/1 def destroy @recommendation_category.destroy end + def update + head :not_implemented + end + private # Use callbacks to share common setup or constraints between actions. @@ -46,8 +44,19 @@ def set_and_authorize_recommendation_category @recommendation_category = policy_scope(base_object).find(params[:id]) authorize @recommendation_category rescue ActiveRecord::RecordNotFound - raise unless action_name == "destroy" - head :no_content + if action_name == "destroy" + record = base_object.find_by(id: params[:id]) + + if record.present? + # Record exists but is out of scope — test authorization anyway + authorize record + end + + # If we got here, it's okay to respond as deleted + head :no_content + else + raise + end end def base_object diff --git a/app/controllers/recommendation_indicators_controller.rb b/app/controllers/recommendation_indicators_controller.rb index 4b2ef0fe..83747aa8 100644 --- a/app/controllers/recommendation_indicators_controller.rb +++ b/app/controllers/recommendation_indicators_controller.rb @@ -1,5 +1,6 @@ class RecommendationIndicatorsController < ApplicationController before_action :set_and_authorize_recommendation_indicator, only: [:show, :destroy] + skip_before_action :authenticate_user!, only: [:update] def index @recommendation_indicators = policy_scope(base_object).order(created_at: :desc).page(params[:page]) @@ -37,6 +38,20 @@ def update def set_and_authorize_recommendation_indicator @recommendation_indicator = policy_scope(base_object).find(params[:id]) authorize @recommendation_indicator + rescue ActiveRecord::RecordNotFound + if action_name == "destroy" + record = base_object.find_by(id: params[:id]) + + if record.present? + # Record exists but is out of scope — test authorization anyway + authorize record + end + + # If we got here, it's okay to respond as deleted + head :no_content + else + raise + end end def base_object diff --git a/app/controllers/recommendation_measures_controller.rb b/app/controllers/recommendation_measures_controller.rb index 8586d7d2..f2b28ced 100644 --- a/app/controllers/recommendation_measures_controller.rb +++ b/app/controllers/recommendation_measures_controller.rb @@ -1,5 +1,6 @@ class RecommendationMeasuresController < ApplicationController - before_action :set_and_authorize_recommendation_measure, only: [:show, :update, :destroy] + before_action :set_and_authorize_recommendation_measure, only: [:show, :destroy] + skip_before_action :authenticate_user!, only: [:update] # GET /recommendation_measures def index @@ -27,24 +28,35 @@ def create end end - # PATCH/PUT /recommendation_measures/1 - def update - if @recommendation_measure.update!(permitted_attributes(@recommendation_measure)) - render json: serialize(@recommendation_measure) - end - end - # DELETE /recommendation_measures/1 def destroy @recommendation_measure.destroy end + def update + head :not_implemented + end + private # Use callbacks to share common setup or constraints between actions. def set_and_authorize_recommendation_measure @recommendation_measure = policy_scope(base_object).find(params[:id]) authorize @recommendation_measure + rescue ActiveRecord::RecordNotFound + if action_name == "destroy" + record = base_object.find_by(id: params[:id]) + + if record.present? + # Record exists but is out of scope — test authorization anyway + authorize record + end + + # If we got here, it's okay to respond as deleted + head :no_content + else + raise + end end def base_object diff --git a/app/controllers/recommendation_recommendations_controller.rb b/app/controllers/recommendation_recommendations_controller.rb index 87a0da7d..6a5c9439 100644 --- a/app/controllers/recommendation_recommendations_controller.rb +++ b/app/controllers/recommendation_recommendations_controller.rb @@ -36,6 +36,20 @@ def update def set_and_authorize_recommendation_recommendation @recommendation_recommendation = policy_scope(base_object).find(params[:id]) authorize @recommendation_recommendation + rescue ActiveRecord::RecordNotFound + if action_name == "destroy" + record = base_object.find_by(id: params[:id]) + + if record.present? + # Record exists but is out of scope — test authorization anyway + authorize record + end + + # If we got here, it's okay to respond as deleted + head :no_content + else + raise + end end def base_object diff --git a/app/controllers/s3_controller.rb b/app/controllers/s3_controller.rb index 7cbb840b..3b7dbe3a 100644 --- a/app/controllers/s3_controller.rb +++ b/app/controllers/s3_controller.rb @@ -9,7 +9,10 @@ def sign object_path = "#{ENV["S3_ASSET_FOLDER"]}/#{params[:objectName]}" s3_url = ::FogStorage.put_object_url(ENV["S3_BUCKET_NAME"], object_path, 15.minutes.from_now.to_time.to_i, headers, options) - url = "#{ENV["CLIENT_URL"]}/#{object_path}?#{URI(s3_url).query}" + url = s3_url + if ENV["CLIENT_URL"].present? + url = "#{ENV["CLIENT_URL"]}/#{object_path}?#{URI(s3_url).query}" + end render json: {signedUrl: url} end diff --git a/app/controllers/user_categories_controller.rb b/app/controllers/user_categories_controller.rb index 1b55d794..b882b5e9 100644 --- a/app/controllers/user_categories_controller.rb +++ b/app/controllers/user_categories_controller.rb @@ -1,5 +1,6 @@ class UserCategoriesController < ApplicationController - before_action :set_and_authorize_user_category, only: [:show, :update, :destroy] + before_action :set_and_authorize_user_category, only: [:show, :destroy] + skip_before_action :authenticate_user!, only: [:update] # GET /user_categories def index @@ -27,18 +28,15 @@ def create end end - # PATCH/PUT /user_categories/1 - def update - if @user_category.update!(permitted_attributes(@user_category)) - render json: serialize(@user_category) - end - end - # DELETE /user_categories/1 def destroy @user_category.destroy end + def update + head :not_implemented + end + private # Use callbacks to share common setup or constraints between actions. @@ -46,8 +44,19 @@ def set_and_authorize_user_category @user_category = policy_scope(base_object).find(params[:id]) authorize @user_category rescue ActiveRecord::RecordNotFound - raise unless action_name == "destroy" - head :no_content + if action_name == "destroy" + record = base_object.find_by(id: params[:id]) + + if record.present? + # Record exists but is out of scope — test authorization anyway + authorize record + end + + # If we got here, it's okay to respond as deleted + head :no_content + else + raise + end end def base_object diff --git a/app/controllers/user_roles_controller.rb b/app/controllers/user_roles_controller.rb index beea4f8c..3d1434f9 100644 --- a/app/controllers/user_roles_controller.rb +++ b/app/controllers/user_roles_controller.rb @@ -1,5 +1,6 @@ class UserRolesController < ApplicationController - before_action :set_and_authorize_user_role, only: [:show, :update, :destroy] + before_action :set_and_authorize_user_role, only: [:show, :destroy] + skip_before_action :authenticate_user!, only: [:update] # GET /user_roles def index @@ -27,24 +28,35 @@ def create end end - # PATCH/PUT /user_roles/1 - def update - if @user_role.update!(permitted_attributes(@user_role)) - render json: serialize(@user_role) - end - end - # DELETE /user_roles/1 def destroy @user_role.destroy end + def update + head :not_implemented + end + private # Use callbacks to share common setup or constraints between actions. def set_and_authorize_user_role @user_role = policy_scope(base_object).find(params[:id]) authorize @user_role + rescue ActiveRecord::RecordNotFound + if action_name == "destroy" + record = base_object.find_by(id: params[:id]) + + if record.present? + # Record exists but is out of scope — test authorization anyway + authorize record + end + + # If we got here, it's okay to respond as deleted + head :no_content + else + raise + end end def base_object diff --git a/app/models/indicator.rb b/app/models/indicator.rb index 4e6821ac..9a2c7d86 100644 --- a/app/models/indicator.rb +++ b/app/models/indicator.rb @@ -23,8 +23,6 @@ class Indicator < VersionedRecord belongs_to :manager, class_name: "User", foreign_key: :manager_id, required: false belongs_to :relationship_updated_by, class_name: "User", required: false - accepts_nested_attributes_for :measure_indicators - def is_current measures.empty? || measures.any?(&:is_current) end diff --git a/app/models/measure.rb b/app/models/measure.rb index 2ed37dc0..89bb32bc 100644 --- a/app/models/measure.rb +++ b/app/models/measure.rb @@ -15,9 +15,6 @@ class Measure < VersionedRecord belongs_to :relationship_updated_by, class_name: "User", required: false - accepts_nested_attributes_for :recommendation_measures - accepts_nested_attributes_for :measure_categories - validates :title, presence: true validates :reference, presence: true, uniqueness: true diff --git a/app/models/measure_category.rb b/app/models/measure_category.rb index b4c8a158..5b4f62e1 100644 --- a/app/models/measure_category.rb +++ b/app/models/measure_category.rb @@ -1,12 +1,12 @@ class MeasureCategory < VersionedRecord - belongs_to :measure - belongs_to :category - accepts_nested_attributes_for :measure - accepts_nested_attributes_for :category + belongs_to :measure, optional: true + belongs_to :category, optional: true validates :category_id, uniqueness: {scope: :measure_id} validates :measure_id, presence: true validates :category_id, presence: true + validate :measure_must_exist, on: :create + validate :category_must_exist, on: :create validate :single_category_per_taxonomy, on: :create after_commit :set_relationship_updated, on: [:create, :update, :destroy] @@ -38,6 +38,22 @@ def save_with_cleanup private + def measure_must_exist + return if measure_id.blank? + + unless Measure.exists?(measure_id) + errors.add(:measure, "must exist (id: #{measure_id})") + end + end + + def category_must_exist + return if category_id.blank? + + unless Category.exists?(category_id) + errors.add(:category, "must exist (id: #{category_id})") + end + end + def lock_name "MeasureCategory-measure_id_#{measure_id}-taxonomy_id_#{category.taxonomy_id}" end diff --git a/app/models/measure_indicator.rb b/app/models/measure_indicator.rb index 68ddc351..275be2f1 100644 --- a/app/models/measure_indicator.rb +++ b/app/models/measure_indicator.rb @@ -1,17 +1,33 @@ class MeasureIndicator < VersionedRecord - belongs_to :measure - belongs_to :indicator - accepts_nested_attributes_for :measure - accepts_nested_attributes_for :indicator + belongs_to :measure, optional: true + belongs_to :indicator, optional: true validates :measure_id, uniqueness: {scope: :indicator_id} validates :measure_id, presence: true validates :indicator_id, presence: true + validate :measure_must_exist, on: :create + validate :indicator_must_exist, on: :create after_commit :set_relationship_updated, on: [:create, :update, :destroy] private + def measure_must_exist + return if measure_id.blank? + + unless Measure.exists?(measure_id) + errors.add(:measure, "must exist (id: #{measure_id})") + end + end + + def indicator_must_exist + return if indicator_id.blank? + + unless Indicator.exists?(indicator_id) + errors.add(:indicator, "must exist (id: #{indicator_id})") + end + end + def set_relationship_updated if measure && !measure.destroyed? measure.update_column(:relationship_updated_at, Time.zone.now) diff --git a/app/models/recommendation.rb b/app/models/recommendation.rb index a4c93c12..3250cf69 100644 --- a/app/models/recommendation.rb +++ b/app/models/recommendation.rb @@ -34,8 +34,6 @@ def progress_reports belongs_to :relationship_updated_by, class_name: "User", required: false - accepts_nested_attributes_for :recommendation_categories - validates :title, presence: true validates :reference, presence: true, uniqueness: true diff --git a/app/models/recommendation_category.rb b/app/models/recommendation_category.rb index 171a7e99..660107fa 100644 --- a/app/models/recommendation_category.rb +++ b/app/models/recommendation_category.rb @@ -1,14 +1,15 @@ class RecommendationCategory < VersionedRecord - belongs_to :recommendation - belongs_to :category - accepts_nested_attributes_for :recommendation - accepts_nested_attributes_for :category + belongs_to :recommendation, optional: true + belongs_to :category, optional: true validates :category_id, uniqueness: {scope: :recommendation_id} validates :recommendation_id, presence: true validates :category_id, presence: true validate :single_category_per_taxonomy, on: :create + validate :recommendation_must_exist, on: :create + validate :category_must_exist, on: :create + after_commit :set_relationship_updated, on: [:create, :update, :destroy] # replacing "save": before creating the record we need to remove all @@ -38,6 +39,22 @@ def save_with_cleanup private + def recommendation_must_exist + return if recommendation_id.blank? + + unless Recommendation.exists?(recommendation_id) + errors.add(:recommendation, "must exist (id: #{recommendation_id})") + end + end + + def category_must_exist + return if category_id.blank? + + unless Category.exists?(category_id) + errors.add(:category, "must exist (id: #{category_id})") + end + end + def lock_name "RecommendationCategory-recommendation_id_#{recommendation_id}-taxonomy_id_#{category.taxonomy_id}" end diff --git a/app/models/recommendation_indicator.rb b/app/models/recommendation_indicator.rb index 961aab03..d82e4252 100644 --- a/app/models/recommendation_indicator.rb +++ b/app/models/recommendation_indicator.rb @@ -1,14 +1,32 @@ class RecommendationIndicator < VersionedRecord - belongs_to :recommendation - belongs_to :indicator + belongs_to :recommendation, inverse_of: :recommendation_indicators, optional: true + belongs_to :indicator, inverse_of: :recommendation_indicators, optional: true validates :recommendation_id, presence: true validates :indicator_id, presence: true + validate :recommendation_must_exist, on: :create + validate :indicator_must_exist, on: :create after_commit :set_relationship_updated, on: [:create, :update, :destroy] private + def recommendation_must_exist + return if recommendation_id.blank? + + unless Recommendation.exists?(recommendation_id) + errors.add(:recommendation, "must exist (id: #{recommendation_id})") + end + end + + def indicator_must_exist + return if indicator_id.blank? + + unless Indicator.exists?(indicator_id) + errors.add(:indicator, "must exist (id: #{indicator_id})") + end + end + def set_relationship_updated if recommendation && !recommendation.destroyed? recommendation.update_column(:relationship_updated_at, Time.zone.now) diff --git a/app/models/recommendation_measure.rb b/app/models/recommendation_measure.rb index daef2f6d..8021dd97 100644 --- a/app/models/recommendation_measure.rb +++ b/app/models/recommendation_measure.rb @@ -1,17 +1,33 @@ class RecommendationMeasure < VersionedRecord - belongs_to :recommendation, inverse_of: :recommendation_measures - belongs_to :measure, inverse_of: :recommendation_measures - accepts_nested_attributes_for :recommendation - accepts_nested_attributes_for :measure + belongs_to :recommendation, inverse_of: :recommendation_measures, optional: true + belongs_to :measure, inverse_of: :recommendation_measures, optional: true validates :measure_id, uniqueness: {scope: :recommendation_id} validates :recommendation_id, presence: true validates :measure_id, presence: true + validate :recommendation_must_exist, on: :create + validate :measure_must_exist, on: :create after_commit :set_relationship_updated, on: [:create, :update, :destroy] private + def recommendation_must_exist + return if recommendation_id.blank? + + unless Recommendation.exists?(recommendation_id) + errors.add(:recommendation, "must exist (id: #{recommendation_id})") + end + end + + def measure_must_exist + return if measure_id.blank? + + unless Measure.exists?(measure_id) + errors.add(:measure, "must exist (id: #{measure_id})") + end + end + def set_relationship_updated if recommendation && !recommendation.destroyed? recommendation.update_column(:relationship_updated_at, Time.zone.now) diff --git a/app/policies/indicator_policy.rb b/app/policies/indicator_policy.rb index cd7a2522..4bbb662d 100644 --- a/app/policies/indicator_policy.rb +++ b/app/policies/indicator_policy.rb @@ -12,11 +12,7 @@ def permitted_attributes :repeat, :start_date, :title, - (:is_archive if @user.role?("admin")), - measure_indicators_attributes: [ - :measure_id, - measure_attributes: [:id, :title, :description, :target_date, :draft] - ] + (:is_archive if @user.role?("admin")) ].compact end diff --git a/app/policies/measure_category_policy.rb b/app/policies/measure_category_policy.rb index df45ae99..88b01748 100644 --- a/app/policies/measure_category_policy.rb +++ b/app/policies/measure_category_policy.rb @@ -2,13 +2,7 @@ class MeasureCategoryPolicy < ApplicationPolicy def permitted_attributes - [:measure_id, - :category_id, - measure_attributes: [:title, :description, :target_date, :draft], - category_attributes: [:id, :title, :short_title, :description, :url, - :taxonomy_id, - :draft, - :manager_id]] + [:measure_id, :category_id] end def update? diff --git a/app/policies/measure_indicator_policy.rb b/app/policies/measure_indicator_policy.rb index b717faee..ba80fd74 100644 --- a/app/policies/measure_indicator_policy.rb +++ b/app/policies/measure_indicator_policy.rb @@ -2,10 +2,7 @@ class MeasureIndicatorPolicy < ApplicationPolicy def permitted_attributes - [:measure_id, - :indicator_id, - measure_attributes: [:id, :title, :description, :target_date, :draft], - indicator_attributes: [:id, :title, :description, :draft]] + [:measure_id, :indicator_id] end def update? diff --git a/app/policies/measure_policy.rb b/app/policies/measure_policy.rb index 200daf57..a28af687 100644 --- a/app/policies/measure_policy.rb +++ b/app/policies/measure_policy.rb @@ -11,24 +11,7 @@ def permitted_attributes :target_date_comment, :target_date, :title, - (:is_archive if @user.role?("admin")), - recommendation_measures_attributes: [ - :recommendation_id, - recommendation_attributes: [:id, :title, :number, :draft] - ], - measure_categories_attributes: [ - :category_id, - category_attributes: [ - :description, - :draft, - :id, - :manager_id, - :short_title, - :taxonomy_id, - :title, - :url - ] - ] + (:is_archive if @user.role?("admin")) ].compact end diff --git a/app/policies/recommendation_category_policy.rb b/app/policies/recommendation_category_policy.rb index 8b4cd742..f2e84f60 100644 --- a/app/policies/recommendation_category_policy.rb +++ b/app/policies/recommendation_category_policy.rb @@ -2,13 +2,7 @@ class RecommendationCategoryPolicy < ApplicationPolicy def permitted_attributes - [:recommendation_id, - :category_id, - recommendation_attributes: [:id, :title, :number, :draft], - category_attributes: [:id, :title, :short_title, :description, :url, - :taxonomy_id, - :draft, - :manager_id]] + [:recommendation_id, :category_id] end def update? diff --git a/app/policies/recommendation_indicator_policy.rb b/app/policies/recommendation_indicator_policy.rb index 1d30658d..59d6c4f1 100644 --- a/app/policies/recommendation_indicator_policy.rb +++ b/app/policies/recommendation_indicator_policy.rb @@ -5,8 +5,6 @@ def permitted_attributes [:recommendation_id, :indicator_id] end - # TODO pretty sure we don't actually need this now as I've excluded the route. But, consistency? Could remove from other policies and add `except: [:update]`` on their routes instead? - # it's really weird how some controllers implement an update but the policy precludes the use of it... possibly I am missing something? def update? false end diff --git a/app/policies/recommendation_measure_policy.rb b/app/policies/recommendation_measure_policy.rb index 0970998b..c63325cf 100644 --- a/app/policies/recommendation_measure_policy.rb +++ b/app/policies/recommendation_measure_policy.rb @@ -2,10 +2,11 @@ class RecommendationMeasurePolicy < ApplicationPolicy def permitted_attributes - [:recommendation_id, - :measure_id, - recommendation_attributes: [:id, :title, :number, :draft], - measure_attributes: [:id, :title, :description, :target_date, :draft]] + [:recommendation_id, :measure_id] + end + + def update? + false end class Scope < Scope diff --git a/app/policies/recommendation_policy.rb b/app/policies/recommendation_policy.rb index 385e5dc3..25eeeb1c 100644 --- a/app/policies/recommendation_policy.rb +++ b/app/policies/recommendation_policy.rb @@ -11,8 +11,7 @@ def permitted_attributes :description, :framework_id, :support_level, - (:is_archive if @user.role?("admin")), - recommendation_categories_attributes: [:category_id] + (:is_archive if @user.role?("admin")) ] end diff --git a/app/policies/user_category_policy.rb b/app/policies/user_category_policy.rb index 248d9063..a6db9795 100644 --- a/app/policies/user_category_policy.rb +++ b/app/policies/user_category_policy.rb @@ -2,12 +2,7 @@ class UserCategoryPolicy < ApplicationPolicy def permitted_attributes - [:user_id, - :category_id, - category_attributes: [:id, :title, :short_title, :description, :url, - :taxonomy_id, - :draft, - :manager_id]] + [:user_id, :category_id] end def update? diff --git a/app/policies/user_role_policy.rb b/app/policies/user_role_policy.rb index 2b6c6bf3..360d370e 100644 --- a/app/policies/user_role_policy.rb +++ b/app/policies/user_role_policy.rb @@ -24,10 +24,7 @@ def destroy? end def permitted_attributes - [:user_id, - :role_id, - user_attributes: [:id], - role_attributes: [:id]] + [:user_id, :role_id] end class Scope < Scope diff --git a/config/routes.rb b/config/routes.rb index e0c3a0e5..59d42c6f 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -14,29 +14,19 @@ get "static_pages/home" get "s3/sign" - resources :measure_categories - resources :measure_indicators - resources :recommendation_categories - resources :user_categories - resources :recommendation_measures - resources :categories do - resources :recommendations, only: [:index, :show] - resources :measures, only: [:index, :show] - end - resources :recommendations do - resources :measures, only: [:index, :show] - end - resources :measures do - resources :recommendations, only: [:index, :show] - end - resources :indicators do - resources :measures, only: [:index, :show] - resources :progress_reports, only: [:index, :show] - end + resources :measure_categories, only: [:index, :show, :create, :destroy] + resources :measure_indicators, only: [:index, :show, :create, :destroy] + resources :recommendation_categories, only: [:index, :show, :create, :destroy] + resources :user_categories, only: [:index, :show, :create, :destroy] + resources :recommendation_measures, only: [:index, :show, :create, :destroy] + resources :categories + resources :recommendations + resources :measures + resources :indicators resources :progress_reports resources :due_dates resources :users - resources :user_roles + resources :user_roles, only: [:index, :show, :create, :destroy] resources :roles resources :pages resources :bookmarks @@ -45,8 +35,8 @@ resources :framework_frameworks, only: [:index, :show] resources :framework_taxonomies, only: [:index, :show] - resources :recommendation_recommendations, except: [:update] - resources :recommendation_indicators, except: [:update] + resources :recommendation_recommendations, only: [:index, :show, :create, :destroy] + resources :recommendation_indicators, only: [:index, :show, :create, :destroy] mount LetterOpenerWeb::Engine, at: "/letter_opener" if Rails.env.development? diff --git a/db/schema.rb b/db/schema.rb index da128ef0..942565b6 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,9 +10,9 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.2].define(version: 2025_03_24_162158) do +ActiveRecord::Schema[8.0].define(version: 2025_03_24_162158) do # These are extensions that must be enabled in order to support this database - enable_extension "plpgsql" + enable_extension "pg_catalog.plpgsql" create_table "bookmarks", id: :serial, force: :cascade do |t| t.integer "user_id", null: false diff --git a/db/seeds.rb b/db/seeds.rb index f808ebae..44c8f3b9 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -29,17 +29,34 @@ def base_seeds! Page.new(title: "Copyright", menu_title: "Copyright").save! Page.new(title: "Disclaimer", menu_title: "Disclaimer").save! Page.new(title: "Privacy", menu_title: "Privacy").save! - Page.new(title: "About the Human Rights Monitor", menu_title: "About").save! + Page.new(title: "About IMPACT OSS", menu_title: "About").save! # set up frameworks hr = Framework.create!( - title: "International Human Rights Obligations", + title: "Human Rights Obligations", short_title: "HR", has_indicators: false, has_measures: true, has_response: true ) + sdgfw = Framework.new( + title: "Sustainable Development Goals", + short_title: "SDGs", + has_indicators: true, + has_measures: true, + has_response: false + ) + sdgfw.save! + + ndp = Framework.create!( + title: "National Development Plan", + short_title: "NDP", + has_indicators: true, + has_measures: true, + has_response: false + ) + # Set up taxonomies # 1. Global taxonomy body = Taxonomy.create!( @@ -152,6 +169,46 @@ def base_seeds! priority: 7 ) + # 9. Country specific taxonomy + cluster = Taxonomy.create!( + title: "Thematic cluster", + tags_measures: true, + tags_users: false, + allow_multiple: true, + priority: 100, + groups_measures_default: 1 + ) + FrameworkTaxonomy.create!( + framework: hr, + taxonomy: cluster + ) + FrameworkTaxonomy.create!( + framework: sdgfw, + taxonomy: cluster + ) + FrameworkTaxonomy.create!( + framework: ndp, + taxonomy: cluster + ) + # 10. Global taxonomy + sdg = Taxonomy.create!( + framework: sdgfw, + title: "SDGs", + has_manager: true, + allow_multiple: true, + priority: 31, + tags_measures: false, + tags_users: false + ) + FrameworkTaxonomy.create!( + framework: hr, + taxonomy: sdg + ) + FrameworkTaxonomy.create!( + framework: sdgfw, + taxonomy: sdg + ) + # Set up categories # SMART categories smart.categories.create!( diff --git a/spec/controllers/recommendation_measures_controller_spec.rb b/spec/controllers/recommendation_measures_controller_spec.rb index b9bcf17f..0d4f324e 100644 --- a/spec/controllers/recommendation_measures_controller_spec.rb +++ b/spec/controllers/recommendation_measures_controller_spec.rb @@ -87,72 +87,6 @@ end end - describe "PUT update" do - let(:recommendation_measure) { FactoryBot.create(:recommendation_measure, created_by: FactoryBot.create(:user, :admin)) } - subject do - put :update, - format: :json, - params: { - id: recommendation_measure, - recommendation_measure: { - title: "test update", - description: "test update", - target_date: "today update" - } - } - end - - context "when not signed in" do - it "not allow updating a recommendation_measure" do - expect(subject).to be_unauthorized - end - end - - context "when user signed in" do - let(:guest) { FactoryBot.create(:user) } - let(:manager) { FactoryBot.create(:user, :manager) } - let(:admin) { FactoryBot.create(:user, :admin) } - - it "will not allow a guest to update a recommendation_measure" do - sign_in guest - expect(subject).to be_forbidden - end - - it "will allow a manager to update a recommendation_measure" do - sign_in manager - expect(subject).to be_ok - end - - it "will record what manager updated the recommendation_measure", versioning: true do - expect(PaperTrail).to be_enabled - sign_in manager - json = JSON.parse(subject.body) - expect(json.dig("data", "attributes", "updated_by_id").to_i).to eq manager.id - end - - it "will allow an admin to update a recommendation_measure" do - sign_in admin - expect(subject).to be_ok - end - - it "will record what admin updated the recommendation_measure", versioning: true do - expect(PaperTrail).to be_enabled - sign_in admin - json = JSON.parse(subject.body) - expect(json.dig("data", "attributes", "updated_by_id").to_i).to eq admin.id - end - - it "will return an error if params are incorrect" do - sign_in manager - put :update, format: :json, params: { - id: recommendation_measure, - recommendation_measure: {recommendation_id: 999} - } - expect(response).to have_http_status(422) - end - end - end - describe "Delete destroy" do let(:recommendation_measure) { FactoryBot.create(:recommendation_measure) } subject { diff --git a/spec/controllers/user_roles_controller_spec.rb b/spec/controllers/user_roles_controller_spec.rb index 4bf75b6d..a04a08c8 100644 --- a/spec/controllers/user_roles_controller_spec.rb +++ b/spec/controllers/user_roles_controller_spec.rb @@ -316,7 +316,7 @@ context "when user signed in" do it "will not allow a guest to delete a user_role" do sign_in guest - expect(subject).to be_not_found + expect(subject).to be_forbidden end it "will not allow a contributor to delete a user_role" do diff --git a/spec/models/measure_category_spec.rb b/spec/models/measure_category_spec.rb index 3a16820a..42268aa3 100644 --- a/spec/models/measure_category_spec.rb +++ b/spec/models/measure_category_spec.rb @@ -2,8 +2,8 @@ require_relative "../shared_examples/enforce_allow_multiple" RSpec.describe MeasureCategory, type: :model do - it { is_expected.to belong_to :measure } - it { is_expected.to belong_to :category } + it { is_expected.to belong_to(:measure).optional } + it { is_expected.to belong_to(:category).optional } it { is_expected.to validate_uniqueness_of(:category_id).scoped_to(:measure_id) } it { is_expected.to validate_presence_of :category_id } it { is_expected.to validate_presence_of :measure_id } diff --git a/spec/models/measure_indicator_spec.rb b/spec/models/measure_indicator_spec.rb index eafb7ffc..611e1fd6 100644 --- a/spec/models/measure_indicator_spec.rb +++ b/spec/models/measure_indicator_spec.rb @@ -1,8 +1,8 @@ require "rails_helper" RSpec.describe MeasureIndicator, type: :model do - it { is_expected.to belong_to :measure } - it { is_expected.to belong_to :indicator } + it { is_expected.to belong_to(:measure).optional } + it { is_expected.to belong_to(:indicator).optional } it { is_expected.to validate_uniqueness_of(:measure_id).scoped_to(:indicator_id) } it { is_expected.to validate_presence_of(:measure_id) } it { is_expected.to validate_presence_of(:indicator_id) } diff --git a/spec/models/recommendation_category_spec.rb b/spec/models/recommendation_category_spec.rb index 5d9f7ef1..295ddf91 100644 --- a/spec/models/recommendation_category_spec.rb +++ b/spec/models/recommendation_category_spec.rb @@ -2,8 +2,8 @@ require_relative "../shared_examples/enforce_allow_multiple" RSpec.describe RecommendationCategory, type: :model do - it { is_expected.to belong_to :recommendation } - it { is_expected.to belong_to :category } + it { is_expected.to belong_to(:recommendation).optional } + it { is_expected.to belong_to(:category).optional } it { is_expected.to validate_uniqueness_of(:category_id).scoped_to(:recommendation_id) } it { is_expected.to validate_presence_of(:recommendation_id) } it { is_expected.to validate_presence_of(:category_id) } diff --git a/spec/models/recommendation_indicator_spec.rb b/spec/models/recommendation_indicator_spec.rb index 155d9acb..89540e7f 100644 --- a/spec/models/recommendation_indicator_spec.rb +++ b/spec/models/recommendation_indicator_spec.rb @@ -1,8 +1,8 @@ require "rails_helper" RSpec.describe RecommendationIndicator, type: :model do - it { is_expected.to belong_to :recommendation } - it { is_expected.to belong_to :indicator } + it { is_expected.to belong_to(:recommendation).optional } + it { is_expected.to belong_to(:indicator).optional } it { is_expected.to validate_presence_of(:recommendation_id) } it { is_expected.to validate_presence_of(:indicator_id) } diff --git a/spec/models/recommendation_measure_spec.rb b/spec/models/recommendation_measure_spec.rb index 7f9c6a66..bd897bd3 100644 --- a/spec/models/recommendation_measure_spec.rb +++ b/spec/models/recommendation_measure_spec.rb @@ -1,8 +1,8 @@ require "rails_helper" RSpec.describe RecommendationMeasure, type: :model do - it { is_expected.to belong_to :recommendation } - it { is_expected.to belong_to :measure } + it { is_expected.to belong_to(:recommendation).optional } + it { is_expected.to belong_to(:measure).optional } it { is_expected.to validate_uniqueness_of(:measure_id).scoped_to(:recommendation_id) } it { is_expected.to validate_presence_of(:recommendation_id) } it { is_expected.to validate_presence_of(:measure_id) } diff --git a/spec/models/recommendation_spec.rb b/spec/models/recommendation_spec.rb index f3267ffd..5cddac34 100644 --- a/spec/models/recommendation_spec.rb +++ b/spec/models/recommendation_spec.rb @@ -23,8 +23,6 @@ it { is_expected.to belong_to(:framework).optional } - it { is_expected.to accept_nested_attributes_for :recommendation_categories } - context "is_current" do let(:category) { FactoryBot.create(:category) } let(:recommendation) { FactoryBot.create(:recommendation) } diff --git a/spec/shared_examples/enforce_allow_multiple.rb b/spec/shared_examples/enforce_allow_multiple.rb index d4724201..5541b712 100644 --- a/spec/shared_examples/enforce_allow_multiple.rb +++ b/spec/shared_examples/enforce_allow_multiple.rb @@ -109,8 +109,8 @@ FactoryBot.create( :category, taxonomy: taxonomy, - short_title: "Taxonomy Allow #{taxonomy_index + 1} - Category #{category_index + 1}", - title: "Category #{category_index + 1} for Taxonomy Allow #{taxonomy_index + 1}" + short_title: "Taxonomy Disallow #{taxonomy_index + 1} - Category #{category_index + 1}", + title: "Category #{category_index + 1} for Taxonomy Disallow #{taxonomy_index + 1}" ) end end @@ -252,7 +252,7 @@ end end - context "one from the same taxonomy with allow_multiple: false\n" do + context "one from a taxonomy with allow_multiple: false\n" do before do described_class.create(:category => categories_disallow.first.first, associated_model[:association] => subject_association) end @@ -275,12 +275,12 @@ new_category # replaced categories_disallow.first.first ] end - let(:potential_race_conditions) do - [ - categories_disallow.first.first, - categories_disallow.first.third - ] - end + # let(:potential_race_conditions) do + # [ + # categories_disallow.first.first, + # categories_disallow.first.third + # ] + # end it "replaces the existing category from its taxonomy" do run_test @@ -305,12 +305,12 @@ categories_disallow.second.first ] end - let(:potential_race_conditions) do - [ - categories_disallow.first.first, - categories_disallow.first.third - ] - end + # let(:potential_race_conditions) do + # [ + # categories_disallow.first.first, + # categories_disallow.first.third + # ] + # end it "replaces the existing category from its taxonomy" do run_test @@ -380,12 +380,12 @@ categories_disallow.third.first ] end - let(:potential_race_conditions) do - [ - categories_disallow.first.first, - categories_disallow.first.third - ] - end + # let(:potential_race_conditions) do + # [ + # categories_disallow.first.first, + # categories_disallow.first.third + # ] + # end it "replaces the existing category from its taxonomy" do run_test @@ -442,12 +442,12 @@ categories_disallow.second.first ] end - let(:potential_race_conditions) do - [ - categories_disallow.first.first, - categories_disallow.first.third - ] - end + # let(:potential_race_conditions) do + # [ + # categories_disallow.first.first, + # categories_disallow.first.third + # ] + # end it "replaces the existing category from its taxonomy" do run_test