From 9c3392b3242f7ad3d0fe6d5f3842bd907ecb2184 Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Mon, 20 Feb 2023 09:35:21 +0100 Subject: [PATCH] Add consumer-based RfC Visibility settings This setting will be useful to increase data protection, where users might not be allowed to see RfCs from other contexts. --- app/controllers/consumers_controller.rb | 2 +- .../request_for_comments_controller.rb | 18 ++++-- app/models/consumer.rb | 6 ++ app/models/request_for_comment.rb | 15 ++++- app/policies/request_for_comment_policy.rb | 26 +++++++++ app/views/consumers/_form.html.slim | 3 + app/views/consumers/show.html.slim | 1 + .../request_for_comments/index.html.slim | 11 ++-- config/locales/de.yml | 5 ++ config/locales/en.yml | 5 ++ ...19113125_add_rfc_visibility_to_consumer.rb | 7 +++ db/schema.rb | 3 +- .../request_for_comments_controller_spec.rb | 57 +++++++++++++++++++ spec/factories/consumer.rb | 1 + 14 files changed, 146 insertions(+), 14 deletions(-) create mode 100644 db/migrate/20230219113125_add_rfc_visibility_to_consumer.rb diff --git a/app/controllers/consumers_controller.rb b/app/controllers/consumers_controller.rb index 7f5cc1e2..8d3701be 100644 --- a/app/controllers/consumers_controller.rb +++ b/app/controllers/consumers_controller.rb @@ -23,7 +23,7 @@ class ConsumersController < ApplicationController end def consumer_params - params[:consumer].permit(:name, :oauth_key, :oauth_secret) if params[:consumer].present? + params[:consumer].permit(:name, :oauth_key, :oauth_secret, :rfc_visibility) if params[:consumer].present? end private :consumer_params diff --git a/app/controllers/request_for_comments_controller.rb b/app/controllers/request_for_comments_controller.rb index 435ad2d6..3ac4acba 100644 --- a/app/controllers/request_for_comments_controller.rb +++ b/app/controllers/request_for_comments_controller.rb @@ -15,7 +15,7 @@ class RequestForCommentsController < ApplicationController # GET /request_for_comments # GET /request_for_comments.json def index - @search = RequestForComment + @search = policy_scope(RequestForComment) .last_per_user(2) .with_last_activity .ransack(params[:q]) @@ -31,7 +31,7 @@ class RequestForCommentsController < ApplicationController # GET /my_request_for_comments def my_comment_requests - @search = RequestForComment + @search = policy_scope(RequestForComment) .with_last_activity .where(user: current_user) .ransack(params[:q]) @@ -44,7 +44,7 @@ class RequestForCommentsController < ApplicationController # GET /my_rfc_activity def rfcs_with_my_comments - @search = RequestForComment + @search = policy_scope(RequestForComment) .with_last_activity .joins(:comments) # we don't need to outer join here, because we know the user has commented on these .where(comments: {user_id: current_user.id}) @@ -59,7 +59,7 @@ class RequestForCommentsController < ApplicationController # GET /exercises/:id/request_for_comments def rfcs_for_exercise exercise = Exercise.find(params[:exercise_id]) - @search = RequestForComment + @search = policy_scope(RequestForComment) .with_last_activity .where(exercise_id: exercise.id) .ransack(params[:q]) @@ -155,8 +155,14 @@ class RequestForCommentsController < ApplicationController # The study groups are grouped by the current study group and other study groups of the user def set_study_group_grouping current_study_group = StudyGroup.find_by(id: session[:study_group_id]) - my_study_groups = current_user.study_groups.reject {|group| group == current_study_group } + my_study_groups = case current_user.consumer.rfc_visibility + when 'all' then current_user.study_groups.order(name: :desc) + when 'consumer' then current_user.study_groups.where(consumer: current_user.consumer).order(name: :desc) + when 'study_group' then current_study_group.present? ? Array(current_study_group) : [] + else raise "Unknown RfC Visibility #{current_user.consumer.rfc_visibility}" + end + @study_groups_grouping = [[t('request_for_comments.index.study_groups.current'), Array(current_study_group)], - [t('request_for_comments.index.study_groups.my'), my_study_groups]] + [t('request_for_comments.index.study_groups.my'), my_study_groups.reject {|group| group == current_study_group }]] end end diff --git a/app/models/consumer.rb b/app/models/consumer.rb index 9f8e7670..1ddeda90 100644 --- a/app/models/consumer.rb +++ b/app/models/consumer.rb @@ -1,6 +1,12 @@ # frozen_string_literal: true class Consumer < ApplicationRecord + enum rfc_visibility: { + all: 0, + consumer: 1, + study_group: 2, + }, _default: :all, _prefix: true + has_many :users has_many :study_groups, dependent: :destroy diff --git a/app/models/request_for_comment.rb b/app/models/request_for_comment.rb index b3b73274..0d1b6288 100644 --- a/app/models/request_for_comment.rb +++ b/app/models/request_for_comment.rb @@ -88,7 +88,20 @@ class RequestForComment < ApplicationRecord private def row_number_user_sql - select('id, user_id, user_type, exercise_id, file_id, question, created_at, updated_at, solved, full_score_reached, submission_id, row_number() OVER (PARTITION BY user_id, user_type ORDER BY created_at DESC) as row_number') + select(' + request_for_comments.id, + request_for_comments.user_id, + request_for_comments.user_type, + request_for_comments.exercise_id, + request_for_comments.file_id, + request_for_comments.question, + request_for_comments.created_at, + request_for_comments.updated_at, + request_for_comments.solved, + request_for_comments.full_score_reached, + request_for_comments.submission_id, + row_number() OVER (PARTITION BY request_for_comments.user_id, request_for_comments.user_type ORDER BY request_for_comments.created_at DESC) as row_number + ') end end end diff --git a/app/policies/request_for_comment_policy.rb b/app/policies/request_for_comment_policy.rb index 55c01b4d..48e41feb 100644 --- a/app/policies/request_for_comment_policy.rb +++ b/app/policies/request_for_comment_policy.rb @@ -48,4 +48,30 @@ class RequestForCommentPolicy < ApplicationPolicy def rfcs_with_my_comments? everyone end + + class Scope < Scope + def resolve + if @user.admin? + @scope.all + else + case @user.consumer.rfc_visibility + when 'all' + @scope.all + when 'consumer' + rfcs_with_users = @scope.distinct + .joins('LEFT OUTER JOIN external_users ON request_for_comments.user_type = \'ExternalUser\' AND request_for_comments.user_id = external_users.id') + .joins('LEFT OUTER JOIN internal_users ON request_for_comments.user_type = \'InternalUser\' AND request_for_comments.user_id = internal_users.id') + + rfcs_with_users.where(external_users: {consumer_id: @user.consumer.id}) + .or(rfcs_with_users.where(internal_users: {consumer_id: @user.consumer.id})) + when 'study_group' + @scope.distinct + .joins(:submission) + .where(submission: {study_group: @user.current_study_group_id}) + else + @scope.none + end + end + end + end end diff --git a/app/views/consumers/_form.html.slim b/app/views/consumers/_form.html.slim index ad6ad4e4..65a11ae1 100644 --- a/app/views/consumers/_form.html.slim +++ b/app/views/consumers/_form.html.slim @@ -9,4 +9,7 @@ .mb-3 = f.label(:oauth_secret, class: 'form-label') = f.text_field(:oauth_secret, class: 'form-control', required: true) + .mb-3 + = f.label(:rfc_visibility, class: 'form-label') + = f.collection_select(:rfc_visibility, Consumer.rfc_visibilities.map { |rfc_visibility, _id| [t("activerecord.attributes.consumer.rfc_visibility_type.#{rfc_visibility}"), rfc_visibility] }, :second, :first, {}, class: 'form-control form-control-sm') .actions = render('shared/submit_button', f: f, object: @consumer) diff --git a/app/views/consumers/show.html.slim b/app/views/consumers/show.html.slim index 9bd5a093..c5acac9d 100644 --- a/app/views/consumers/show.html.slim +++ b/app/views/consumers/show.html.slim @@ -6,5 +6,6 @@ h1 - %w[oauth_key oauth_secret].each do |attribute| = row(label: "consumer.#{attribute}") do = content_tag(:input, nil, class: 'form-control bg-secondary', readonly: true, value: @consumer.send(attribute)) += row(label: 'consumer.rfc_visibility', value: t("activerecord.attributes.consumer.rfc_visibility_type.#{@consumer.rfc_visibility}")) = render('study_groups/table', study_groups: @consumer.study_groups.sort) diff --git a/app/views/request_for_comments/index.html.slim b/app/views/request_for_comments/index.html.slim index 8328308c..913a03df 100644 --- a/app/views/request_for_comments/index.html.slim +++ b/app/views/request_for_comments/index.html.slim @@ -9,11 +9,12 @@ h1 = RequestForComment.model_name.human(count: 2) .col-auto.mt-3.mt-md-0 = f.label(:title_cont, t('request_for_comments.solved'), class: 'visually-hidden form-label') = f.select(:solved_not_eq, [[t('request_for_comments.show_all'), 2], [t('request_for_comments.show_unsolved'), 1], [t('request_for_comments.show_solved'), 0]]) - .row - .col - = f.label(:submission_study_group_id_eq, t('request_for_comments.index.study_groups.placeholder'), class: 'visually-hidden form-label') - = f.grouped_collection_select(:submission_study_group_id_in, @study_groups_grouping, :second, :first, :id, :to_s, {}, - { class: 'form-control', multiple: true, "data-placeholder": t('request_for_comments.index.study_groups.placeholder') }) + - unless current_user.consumer.rfc_visibility_study_group? + .row + .col + = f.label(:submission_study_group_id_eq, t('request_for_comments.index.study_groups.placeholder'), class: 'visually-hidden form-label') + = f.grouped_collection_select(:submission_study_group_id_in, @study_groups_grouping, :second, :first, :id, :to_s, {}, + { class: 'form-control', multiple: true, "data-placeholder": t('request_for_comments.index.study_groups.placeholder') }) .table-responsive table.table.sortable.mt-4 diff --git a/config/locales/de.yml b/config/locales/de.yml index 80790d07..894b2008 100644 --- a/config/locales/de.yml +++ b/config/locales/de.yml @@ -7,6 +7,11 @@ de: name: Name oauth_key: OAuth Key oauth_secret: OAuth Secret + rfc_visibility: Sichtbarkeit von Kommentaranfragen + rfc_visibility_type: + all: Alle Kommentaranfrage sind sichtbar + consumer: Nur Kommentaranfragen des aktuellen Konsumenten sind sichtbar + study_group: Nur Kommentaranfragen der aktuellen Lerngruppe sind sichtbar execution_environment: docker_image: Docker-Image exposed_ports: Zugängliche Ports diff --git a/config/locales/en.yml b/config/locales/en.yml index 4aa44e7b..f1538534 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -7,6 +7,11 @@ en: name: Name oauth_key: OAuth Key oauth_secret: OAuth Secret + rfc_visibility: Visibility of Request for Comments + rfc_visibility_type: + all: All Requests for Comments are visible + consumer: Only Requests for Comments of the current consumer are visible + study_group: Only Requests for Comments of the current study group are visible execution_environment: docker_image: Docker Image exposed_ports: Exposed Ports diff --git a/db/migrate/20230219113125_add_rfc_visibility_to_consumer.rb b/db/migrate/20230219113125_add_rfc_visibility_to_consumer.rb new file mode 100644 index 00000000..b76e9fbc --- /dev/null +++ b/db/migrate/20230219113125_add_rfc_visibility_to_consumer.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AddRfcVisibilityToConsumer < ActiveRecord::Migration[7.0] + def change + add_column :consumers, :rfc_visibility, :integer, limit: 1, null: false, default: 0, comment: 'Used as enum in Rails' + end +end diff --git a/db/schema.rb b/db/schema.rb index 79780f02..90b99baf 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.0].define(version: 2023_02_06_203117) do +ActiveRecord::Schema[7.0].define(version: 2023_02_19_113125) do # These are extensions that must be enabled in order to support this database enable_extension "pg_trgm" enable_extension "pgcrypto" @@ -107,6 +107,7 @@ ActiveRecord::Schema[7.0].define(version: 2023_02_06_203117) do t.datetime "updated_at" t.string "oauth_key" t.string "oauth_secret" + t.integer "rfc_visibility", limit: 2, default: 0, null: false, comment: "Used as enum in Rails" end create_table "error_template_attributes", id: :serial, force: :cascade do |t| diff --git a/spec/controllers/request_for_comments_controller_spec.rb b/spec/controllers/request_for_comments_controller_spec.rb index 0a89a20b..556f448a 100644 --- a/spec/controllers/request_for_comments_controller_spec.rb +++ b/spec/controllers/request_for_comments_controller_spec.rb @@ -9,6 +9,57 @@ describe RequestForCommentsController do before { allow(controller).to receive(:current_user).and_return(user) } + shared_examples 'RfC visibility settings' do + let(:user) { create(:learner) } + + let!(:rfc_other_consumer) do + rfc = create(:rfc) + rfc.user.update(consumer: create(:consumer, name: 'Other Consumer')) + rfc + end + + let!(:rfc_other_study_group) do + rfc = create(:rfc) + rfc.user.update(consumer: user.consumer) + rfc.submission.update(study_group: create(:study_group)) + rfc + end + + let!(:rfc_peer) do + rfc = create(:rfc) + rfc.user.update(consumer: user.consumer) + rfc.submission.update(study_group: user.study_groups.first) + rfc + end + + context 'when rfc_visibility is set to all' do + before { user.consumer.update(rfc_visibility: 'all') } + + it 'shows all RfCs' do + get :index + expect(assigns(:request_for_comments)).to match_array([rfc_other_consumer, rfc_other_study_group, rfc_peer]) + end + end + + context 'when rfc_visibility is set to consumer' do + before { user.consumer.update(rfc_visibility: 'consumer') } + + it 'shows only RfCs of the same consumer' do + get :index + expect(assigns(:request_for_comments)).to match_array([rfc_other_study_group, rfc_peer]) + end + end + + context 'when rfc_visibility is set to study_group' do + before { user.consumer.update(rfc_visibility: 'study_group') } + + it 'shows only RfCs of the same study group' do + get :index + expect(assigns(:request_for_comments)).to match_array([rfc_peer]) + end + end + end + describe 'GET #index' do it 'renders the index template' do get :index @@ -32,6 +83,8 @@ describe RequestForCommentsController do expect(assigns(:request_for_comments)).to eq([rfc_within_my_study_group]) end + + include_examples 'RfC visibility settings' end describe 'GET #my_comment_requests' do @@ -39,6 +92,8 @@ describe RequestForCommentsController do expect_http_status(:ok) expect_template(:index) + + include_examples 'RfC visibility settings' end describe 'GET #rfcs_with_my_comments' do @@ -46,6 +101,8 @@ describe RequestForCommentsController do expect_http_status(:ok) expect_template(:index) + + include_examples 'RfC visibility settings' end describe 'GET #rfcs_for_exercise' do diff --git a/spec/factories/consumer.rb b/spec/factories/consumer.rb index 57c6a67f..9b742d1b 100644 --- a/spec/factories/consumer.rb +++ b/spec/factories/consumer.rb @@ -3,6 +3,7 @@ FactoryBot.define do factory :consumer do name { 'openHPI' } + rfc_visibility { 'all' } singleton_consumer end