From 71fd3b9b07426cfa186f87ac30d547f4b8772252 Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Thu, 22 Nov 2018 18:45:31 +0100 Subject: [PATCH 01/12] Add role to ExternalUser and update it via LTI --- app/controllers/concerns/lti.rb | 17 ++++++++++++++++- app/views/external_users/show.html.slim | 1 + ...20181116143743_add_role_to_external_users.rb | 5 +++++ 3 files changed, 22 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20181116143743_add_role_to_external_users.rb diff --git a/app/controllers/concerns/lti.rb b/app/controllers/concerns/lti.rb index e8241af3..124b32cb 100644 --- a/app/controllers/concerns/lti.rb +++ b/app/controllers/concerns/lti.rb @@ -52,6 +52,21 @@ module Lti end private :external_user_name + def external_user_role(provider) + result = 'learner' + provider.roles.each do |role| + case role.downcase! + when 'administrator' + result = 'admin' + when 'instructor' + result = 'teacher' if result == 'learner' + else # 'learner' + next + end + end + result + end + def refuse_lti_launch(options = {}) return_to_consumer(lti_errorlog: options[:message], lti_errormsg: t('sessions.oauth.failure')) end @@ -129,7 +144,7 @@ module Lti def set_current_user @current_user = ExternalUser.find_or_create_by(consumer_id: @consumer.id, external_id: @provider.user_id) - @current_user.update(email: external_user_email(@provider), name: external_user_name(@provider)) + @current_user.update(email: external_user_email(@provider), name: external_user_name(@provider), role: external_user_role(@provider)) end private :set_current_user diff --git a/app/views/external_users/show.html.slim b/app/views/external_users/show.html.slim index 33e2dd6b..9d18e942 100644 --- a/app/views/external_users/show.html.slim +++ b/app/views/external_users/show.html.slim @@ -3,6 +3,7 @@ h1 = @user.name = row(label: 'external_user.name', value: @user.name) //= row(label: 'external_user.email', value: @user.email) = row(label: 'external_user.consumer', value: link_to(@user.consumer, @user.consumer)) += row(label: 'external_user.role', value: t("users.roles.#{@user.role}")) h4.mt-4 = link_to(t('.exercise_statistics'), statistics_external_user_path(@user)) diff --git a/db/migrate/20181116143743_add_role_to_external_users.rb b/db/migrate/20181116143743_add_role_to_external_users.rb new file mode 100644 index 00000000..5b4d7066 --- /dev/null +++ b/db/migrate/20181116143743_add_role_to_external_users.rb @@ -0,0 +1,5 @@ +class AddRoleToExternalUsers < ActiveRecord::Migration[5.2] + def change + add_column :external_users, :role, :string, default: 'learner', null: false + end +end From 4b251599ff29d576baf61f6d95cd0f47b170acb7 Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Thu, 22 Nov 2018 18:55:54 +0100 Subject: [PATCH 02/12] Use admin? or teacher? for permission check instead of internal_user? --- app/policies/admin_or_author_policy.rb | 2 +- app/policies/exercise_policy.rb | 4 ++-- app/policies/intervention_policy.rb | 4 ++-- app/policies/proxy_exercise_policy.rb | 6 +++--- app/policies/search_policy.rb | 4 ++-- app/policies/tag_policy.rb | 4 ++-- app/views/application/_breadcrumbs.html.slim | 2 +- app/views/application/_navigation.html.slim | 2 +- app/views/application/_session.html.slim | 8 ++++---- app/views/application/welcome.html.slim | 6 +++--- app/views/layouts/application.html.slim | 2 +- 11 files changed, 22 insertions(+), 22 deletions(-) diff --git a/app/policies/admin_or_author_policy.rb b/app/policies/admin_or_author_policy.rb index 195d8438..bf640bad 100644 --- a/app/policies/admin_or_author_policy.rb +++ b/app/policies/admin_or_author_policy.rb @@ -1,6 +1,6 @@ class AdminOrAuthorPolicy < ApplicationPolicy [:create?, :index?, :new?].each do |action| - define_method(action) { @user.internal_user? } + define_method(action) { admin? || teacher? } end [:destroy?, :edit?, :show?, :update?].each do |action| diff --git a/app/policies/exercise_policy.rb b/app/policies/exercise_policy.rb index e4563832..adc246d8 100644 --- a/app/policies/exercise_policy.rb +++ b/app/policies/exercise_policy.rb @@ -9,7 +9,7 @@ class ExercisePolicy < AdminOrAuthorPolicy end def show? - @user.internal_user? + admin? || teacher? end [:clone?, :destroy?, :edit?, :statistics?, :update?, :feedback?].each do |action| @@ -24,7 +24,7 @@ class ExercisePolicy < AdminOrAuthorPolicy def resolve if @user.admin? @scope.all - elsif @user.internal_user? + elsif @user.teacher? @scope.where('user_id = ? OR public = TRUE', @user.id) else @scope.none diff --git a/app/policies/intervention_policy.rb b/app/policies/intervention_policy.rb index b3a25667..7716e667 100644 --- a/app/policies/intervention_policy.rb +++ b/app/policies/intervention_policy.rb @@ -9,7 +9,7 @@ class InterventionPolicy < AdminOrAuthorPolicy end def show? - @user.internal_user? + admin? || teacher? end [:clone?, :destroy?, :edit?, :update?].each do |action| @@ -24,7 +24,7 @@ class InterventionPolicy < AdminOrAuthorPolicy def resolve if @user.admin? @scope.all - elsif @user.internal_user? + elsif @user.teacher? @scope.where('user_id = ? OR public = TRUE', @user.id) else @scope.none diff --git a/app/policies/proxy_exercise_policy.rb b/app/policies/proxy_exercise_policy.rb index 28de525e..c65f1f38 100644 --- a/app/policies/proxy_exercise_policy.rb +++ b/app/policies/proxy_exercise_policy.rb @@ -9,7 +9,7 @@ class ProxyExercisePolicy < AdminOrAuthorPolicy end def show? - @user.internal_user? + admin? || teacher? end [:clone?, :destroy?, :edit?, :update?].each do |action| @@ -24,8 +24,8 @@ class ProxyExercisePolicy < AdminOrAuthorPolicy def resolve if @user.admin? @scope.all - elsif @user.internal_user? - @scope.where('user_id = ? OR public = TRUE', @user.id) + elsif @user.teacher? + @scope.where('user_id = ?', @user.id) else @scope.none end diff --git a/app/policies/search_policy.rb b/app/policies/search_policy.rb index 9da9a641..b45ae00d 100644 --- a/app/policies/search_policy.rb +++ b/app/policies/search_policy.rb @@ -9,7 +9,7 @@ class SearchPolicy < AdminOrAuthorPolicy end def show? - @user.internal_user? + admin? || teacher? end [:clone?, :destroy?, :edit?, :update?].each do |action| @@ -24,7 +24,7 @@ class SearchPolicy < AdminOrAuthorPolicy def resolve if @user.admin? @scope.all - elsif @user.internal_user? + elsif @user.teacher? @scope.where('user_id = ? OR public = TRUE', @user.id) else @scope.none diff --git a/app/policies/tag_policy.rb b/app/policies/tag_policy.rb index 8325b9fa..51665053 100644 --- a/app/policies/tag_policy.rb +++ b/app/policies/tag_policy.rb @@ -9,7 +9,7 @@ class TagPolicy < AdminOrAuthorPolicy end def show? - @user.internal_user? + admin? || teacher? end [:clone?, :destroy?, :edit?, :update?].each do |action| @@ -24,7 +24,7 @@ class TagPolicy < AdminOrAuthorPolicy def resolve if @user.admin? @scope.all - elsif @user.internal_user? + elsif @user.teacher? @scope.where('user_id = ? OR public = TRUE', @user.id) else @scope.none diff --git a/app/views/application/_breadcrumbs.html.slim b/app/views/application/_breadcrumbs.html.slim index 7899ae0e..18045440 100644 --- a/app/views/application/_breadcrumbs.html.slim +++ b/app/views/application/_breadcrumbs.html.slim @@ -1,4 +1,4 @@ -- if current_user.try(:internal_user?) +- if current_user.try(:admin?) or current_user.try(:teacher?) ul.breadcrumb - if model = Kernel.const_get(controller_path.classify) rescue nil - object = model.find_by(id: params[:id]) diff --git a/app/views/application/_navigation.html.slim b/app/views/application/_navigation.html.slim index 006de33b..a23e59da 100644 --- a/app/views/application/_navigation.html.slim +++ b/app/views/application/_navigation.html.slim @@ -1,4 +1,4 @@ -- if current_user.try(:internal_user?) +- if current_user.try(:admin?) or current_user.try(:teacher?) ul.nav.navbar-nav li.nav-item.dropdown a.nav-link.dropdown-toggle.mx-3 data-toggle='dropdown' href='#' diff --git a/app/views/application/_session.html.slim b/app/views/application/_session.html.slim index db401908..abe53b21 100644 --- a/app/views/application/_session.html.slim +++ b/app/views/application/_session.html.slim @@ -5,10 +5,10 @@ = current_user span.caret ul.dropdown-menu.p-0.mt-1 role='menu' - - if current_user.internal_user? - li = link_to(t('consumers.show.link'), current_user.consumer, class: 'dropdown-item') if current_user.consumer - li = link_to(t('internal_users.show.link'), current_user, class: 'dropdown-item') - li = link_to(t('request_for_comments.index.all'), request_for_comments_path, class: 'dropdown-item') + - if current_user.try(:admin?) or current_user.try(:teacher?) + li = link_to(t('consumers.show.link'), current_user.consumer, class: 'dropdown-item') if current_user.consumer and policy(current_user.consumer).show? + li = link_to(t('internal_users.show.link'), current_user, class: 'dropdown-item') if policy(current_user).show? + li = link_to(t('request_for_comments.index.all'), request_for_comments_path, class: 'dropdown-item') if policy(RequestForComment).index? li = link_to(t('request_for_comments.index.get_my_rfc_activity'), my_rfc_activity_path, class: 'dropdown-item') li = link_to(t('request_for_comments.index.get_my_comment_requests'), my_request_for_comments_path, class: 'dropdown-item') - if current_user.internal_user? diff --git a/app/views/application/welcome.html.slim b/app/views/application/welcome.html.slim index 8f9448c7..eb8ebac3 100644 --- a/app/views/application/welcome.html.slim +++ b/app/views/application/welcome.html.slim @@ -1,8 +1,8 @@ h1 = t('.title', application_name: application_name) -- if current_user.try(:external_user?) - p = t('.text_signed_in_as_external_user', application_name: application_name) -- elsif current_user.try(:internal_user?) +- if current_user.try(:admin?) or current_user.try(:teacher?) p = t('.text_signed_in_as_internal_user', user_name: current_user.name) +- elsif current_user.try(:external_user?) + p = t('.text_signed_in_as_external_user', application_name: application_name) - else p == t('.text_signed_out', application_name: application_name, sign_in_path: sign_in_path) diff --git a/app/views/layouts/application.html.slim b/app/views/layouts/application.html.slim index 64b5549f..7ba2cdd3 100644 --- a/app/views/layouts/application.html.slim +++ b/app/views/layouts/application.html.slim @@ -28,7 +28,7 @@ html lang='en' = render('session') .container data-controller=controller_name = render('flash') - = render('breadcrumbs') if current_user.try(:internal_user?) + = render('breadcrumbs') if current_user.try(:admin?) or current_user.try(:teacher?) - if (controller_name == "exercises" && action_name == "implement") .container-fluid = yield From 7a63a9c1c11da6dfb7ea4da7c173ab70224cec55 Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Thu, 22 Nov 2018 19:11:40 +0100 Subject: [PATCH 03/12] Hide navigation elements in the UI based on policies --- app/policies/code_ocean/file_policy.rb | 8 ++++++++ .../application/_navigation_submenu.html.slim | 13 +++++++------ app/views/execution_environments/index.html.slim | 14 +++++++------- 3 files changed, 22 insertions(+), 13 deletions(-) diff --git a/app/policies/code_ocean/file_policy.rb b/app/policies/code_ocean/file_policy.rb index 8cf6d890..e8d7b5a8 100644 --- a/app/policies/code_ocean/file_policy.rb +++ b/app/policies/code_ocean/file_policy.rb @@ -4,6 +4,14 @@ module CodeOcean @user == @record.context.author end + def show? + if @record.context.is_a?(Exercise) + admin? || author? || !@record.hidden + else + admin? || author? + end + end + def create? if @record.context.is_a?(Exercise) admin? || author? diff --git a/app/views/application/_navigation_submenu.html.slim b/app/views/application/_navigation_submenu.html.slim index 047955d7..fc4c9dec 100644 --- a/app/views/application/_navigation_submenu.html.slim +++ b/app/views/application/_navigation_submenu.html.slim @@ -1,6 +1,7 @@ -li.dropdown-submenu - - link = link.nil? ? "#" : link - a.dropdown-item.dropdown-toggle href=link data-toggle="dropdown" = title - ul.dropdown-menu.p-0 - - models.each do |model| - = render('navigation_collection_link', model: model, cached: true) +- if models.any? { |model| policy(model).index? } + li.dropdown-submenu + - link = link.nil? ? "#" : link + a.dropdown-item.dropdown-toggle href=link data-toggle="dropdown" = title + ul.dropdown-menu.p-0 + - models.each do |model| + = render('navigation_collection_link', model: model, cached: true) diff --git a/app/views/execution_environments/index.html.slim b/app/views/execution_environments/index.html.slim index a749df7c..10501911 100644 --- a/app/views/execution_environments/index.html.slim +++ b/app/views/execution_environments/index.html.slim @@ -14,17 +14,17 @@ h1 = ExecutionEnvironment.model_name.human(count: 2) tbody - @execution_environments.each do |execution_environment| tr - td = link_to(execution_environment.name, execution_environment) - td = link_to(execution_environment.author, execution_environment.author) + td = link_to_if(policy(execution_environment).show?, execution_environment.name, execution_environment) + td = link_to_if(policy(execution_environment.author).show?, execution_environment.author, execution_environment.author) td = execution_environment.pool_size td = execution_environment.memory_limit td = symbol_for(execution_environment.network_enabled) td = execution_environment.permitted_execution_time - td = link_to(t('shared.show'), execution_environment) - td = link_to(t('shared.edit'), edit_execution_environment_path(execution_environment)) - td = link_to(t('shared.destroy'), execution_environment, data: {confirm: t('shared.confirm_destroy')}, method: :delete) - td = link_to(t('.shell'), shell_execution_environment_path(execution_environment)) - td = link_to(t('shared.statistics'), statistics_execution_environment_path(execution_environment)) + td = link_to(t('shared.show'), execution_environment) if policy(execution_environment).show? + td = link_to(t('shared.edit'), edit_execution_environment_path(execution_environment)) if policy(execution_environment).edit? + td = link_to(t('shared.destroy'), execution_environment, data: {confirm: t('shared.confirm_destroy')}, method: :delete) if policy(execution_environment).destroy? + td = link_to(t('.shell'), shell_execution_environment_path(execution_environment)) if policy(execution_environment).shell? + td = link_to(t('shared.statistics'), statistics_execution_environment_path(execution_environment)) if policy(execution_environment).statistics? = render('shared/pagination', collection: @execution_environments) p = render('shared/new_button', model: ExecutionEnvironment) From d3f67ab4c7a430dc782182dcae8644d4c19ac389 Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Tue, 27 Nov 2018 17:05:38 +0100 Subject: [PATCH 04/12] Rethink permissions in CodeOcean for usage in schools and adopt views --- app/controllers/application_controller.rb | 1 + app/controllers/exercises_controller.rb | 15 ++++++++------- app/controllers/proxy_exercises_controller.rb | 6 +++--- app/policies/application_policy.rb | 16 +++++++++++----- app/policies/comment_policy.rb | 5 ----- app/policies/consumer_policy.rb | 4 +--- app/policies/execution_environment_policy.rb | 7 +------ app/policies/exercise_policy.rb | 5 ----- app/policies/file_template_policy.rb | 6 +++++- app/policies/file_type_policy.rb | 7 +------ app/policies/intervention_policy.rb | 5 ----- app/policies/proxy_exercise_policy.rb | 5 ----- app/policies/request_for_comment_policy.rb | 5 ----- app/policies/search_policy.rb | 5 ----- app/policies/submission_policy.rb | 16 +++++++++++----- app/policies/tag_policy.rb | 5 ----- app/views/file_types/index.html.slim | 10 +++++----- 17 files changed, 47 insertions(+), 76 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index c58303ba..0988d8f0 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -33,6 +33,7 @@ class ApplicationController < ActionController::Base private :set_locale def welcome + # Show root page end def allow_iframe_requests diff --git a/app/controllers/exercises_controller.rb b/app/controllers/exercises_controller.rb index 839aef4c..e60797d5 100644 --- a/app/controllers/exercises_controller.rb +++ b/app/controllers/exercises_controller.rb @@ -7,8 +7,8 @@ class ExercisesController < ApplicationController before_action :handle_file_uploads, only: [:create, :update] before_action :set_execution_environments, only: [:create, :edit, :new, :update] - before_action :set_exercise, only: MEMBER_ACTIONS + [:clone, :implement, :working_times, :intervention, :search, :run, :statistics, :submit, :reload, :feedback] - before_action :set_external_user, only: [:statistics] + before_action :set_exercise_and_authorize, only: MEMBER_ACTIONS + [:clone, :implement, :working_times, :intervention, :search, :run, :statistics, :submit, :reload, :feedback] + before_action :set_external_user_and_authorize, only: [:statistics] before_action :set_file_types, only: [:create, :edit, :new, :update] before_action :set_course_token, only: [:implement] @@ -291,19 +291,19 @@ class ExercisesController < ApplicationController end private :set_execution_environments - def set_exercise + def set_exercise_and_authorize @exercise = Exercise.find(params[:id]) authorize! end - private :set_exercise + private :set_exercise_and_authorize - def set_external_user + def set_external_user_and_authorize if params[:external_user_id] @external_user = ExternalUser.find(params[:external_user_id]) authorize! end end - private :set_exercise + private :set_external_user_and_authorize def set_file_types @file_types = FileType.all.order(:name) @@ -321,10 +321,11 @@ class ExercisesController < ApplicationController private :collect_set_and_unset_exercise_tags def show + # Show exercise details for teachers and admins end - #we might want to think about auth here def reload + # Returns JSON with original file content end def statistics diff --git a/app/controllers/proxy_exercises_controller.rb b/app/controllers/proxy_exercises_controller.rb index 40bd20ca..171fe061 100644 --- a/app/controllers/proxy_exercises_controller.rb +++ b/app/controllers/proxy_exercises_controller.rb @@ -1,7 +1,7 @@ class ProxyExercisesController < ApplicationController include CommonBehavior - before_action :set_exercise, only: MEMBER_ACTIONS + [:clone, :reload] + before_action :set_exercise_and_authorize, only: MEMBER_ACTIONS + [:clone, :reload] def authorize! authorize(@proxy_exercise || @proxy_exercises) @@ -56,11 +56,11 @@ class ProxyExercisesController < ApplicationController authorize! end - def set_exercise + def set_exercise_and_authorize @proxy_exercise = ProxyExercise.find(params[:id]) authorize! end - private :set_exercise + private :set_exercise_and_authorize def show @search = @proxy_exercise.exercises.search diff --git a/app/policies/application_policy.rb b/app/policies/application_policy.rb index 6ce391a2..9d8b9cd1 100644 --- a/app/policies/application_policy.rb +++ b/app/policies/application_policy.rb @@ -9,22 +9,28 @@ class ApplicationPolicy end private :teacher? + def author? + @user == @record.author + end + private :author? + def everyone + # As the ApplicationController forces to have any authorization, `everyone` here means `every user logged in` true end private :everyone + def no_one + false + end + private :no_one + def initialize(user, record) @user = user @record = record require_user! end - def no_one - false - end - private :no_one - def require_user! fail Pundit::NotAuthorizedError unless @user end diff --git a/app/policies/comment_policy.rb b/app/policies/comment_policy.rb index 3bdcdda0..41caae9b 100644 --- a/app/policies/comment_policy.rb +++ b/app/policies/comment_policy.rb @@ -1,9 +1,4 @@ class CommentPolicy < ApplicationPolicy - def author? - @user == @record.author - end - private :author? - def create? everyone end diff --git a/app/policies/consumer_policy.rb b/app/policies/consumer_policy.rb index 5cdad33f..8566473d 100644 --- a/app/policies/consumer_policy.rb +++ b/app/policies/consumer_policy.rb @@ -1,5 +1,3 @@ class ConsumerPolicy < AdminOnlyPolicy - def show? - super || @user.consumer == @record - end + end diff --git a/app/policies/execution_environment_policy.rb b/app/policies/execution_environment_policy.rb index 3ee5a1c0..9e36b558 100644 --- a/app/policies/execution_environment_policy.rb +++ b/app/policies/execution_environment_policy.rb @@ -1,14 +1,9 @@ class ExecutionEnvironmentPolicy < AdminOnlyPolicy - def author? - @user == @record.author - end - private :author? - [:execute_command?, :shell?, :statistics?].each do |action| define_method(action) { admin? || author? } end - [:create?, :index?, :new?].each do |action| + [:show?, :index?, :new?].each do |action| define_method(action) { admin? || teacher? } end end diff --git a/app/policies/exercise_policy.rb b/app/policies/exercise_policy.rb index adc246d8..cb9a32cf 100644 --- a/app/policies/exercise_policy.rb +++ b/app/policies/exercise_policy.rb @@ -1,9 +1,4 @@ class ExercisePolicy < AdminOrAuthorPolicy - def author? - @user == @record.author - end - private :author? - def batch_update? admin? end diff --git a/app/policies/file_template_policy.rb b/app/policies/file_template_policy.rb index 92ced442..a50302d9 100644 --- a/app/policies/file_template_policy.rb +++ b/app/policies/file_template_policy.rb @@ -1,7 +1,11 @@ class FileTemplatePolicy < AdminOnlyPolicy + def index? + admin? || teacher? + end + def show? - everyone + admin? || teacher? end def by_file_type? diff --git a/app/policies/file_type_policy.rb b/app/policies/file_type_policy.rb index afa6aa7a..ad6dddd0 100644 --- a/app/policies/file_type_policy.rb +++ b/app/policies/file_type_policy.rb @@ -1,10 +1,5 @@ class FileTypePolicy < AdminOnlyPolicy - def author? - @user == @record.author - end - private :author? - - [:create?, :index?, :new?].each do |action| + [:index?, :show?].each do |action| define_method(action) { admin? || teacher? } end diff --git a/app/policies/intervention_policy.rb b/app/policies/intervention_policy.rb index 7716e667..373adcb2 100644 --- a/app/policies/intervention_policy.rb +++ b/app/policies/intervention_policy.rb @@ -1,9 +1,4 @@ class InterventionPolicy < AdminOrAuthorPolicy - def author? - @user == @record.author - end - private :author? - def batch_update? admin? end diff --git a/app/policies/proxy_exercise_policy.rb b/app/policies/proxy_exercise_policy.rb index c65f1f38..3a7bedf7 100644 --- a/app/policies/proxy_exercise_policy.rb +++ b/app/policies/proxy_exercise_policy.rb @@ -1,9 +1,4 @@ class ProxyExercisePolicy < AdminOrAuthorPolicy - def author? - @user == @record.author - end - private :author? - def batch_update? admin? end diff --git a/app/policies/request_for_comment_policy.rb b/app/policies/request_for_comment_policy.rb index d02c5568..80b95f5f 100644 --- a/app/policies/request_for_comment_policy.rb +++ b/app/policies/request_for_comment_policy.rb @@ -1,9 +1,4 @@ class RequestForCommentPolicy < ApplicationPolicy - def author? - @user == @record.author - end - private :author? - def create? everyone end diff --git a/app/policies/search_policy.rb b/app/policies/search_policy.rb index b45ae00d..7f861c30 100644 --- a/app/policies/search_policy.rb +++ b/app/policies/search_policy.rb @@ -1,9 +1,4 @@ class SearchPolicy < AdminOrAuthorPolicy - def author? - @user == @record.author - end - private :author? - def batch_update? admin? end diff --git a/app/policies/submission_policy.rb b/app/policies/submission_policy.rb index 32916450..fe2137c3 100644 --- a/app/policies/submission_policy.rb +++ b/app/policies/submission_policy.rb @@ -1,9 +1,4 @@ class SubmissionPolicy < ApplicationPolicy - def author? - @user == @record.author - end - private :author? - def create? everyone end @@ -16,4 +11,15 @@ class SubmissionPolicy < ApplicationPolicy def index? admin? end + + def everyone_in_study_group + users_in_same_study_group = @record.study_groups.users + users_in_same_study_group.include? @user + end + private :everyone_in_study_group + + def teacher_in_study_group + teacher? && everyone_in_study_group + end + private :teacher_in_study_group end diff --git a/app/policies/tag_policy.rb b/app/policies/tag_policy.rb index 51665053..7691134a 100644 --- a/app/policies/tag_policy.rb +++ b/app/policies/tag_policy.rb @@ -1,9 +1,4 @@ class TagPolicy < AdminOrAuthorPolicy - def author? - @user == @record.author - end - private :author? - def batch_update? admin? end diff --git a/app/views/file_types/index.html.slim b/app/views/file_types/index.html.slim index 95f1394f..08f850e7 100644 --- a/app/views/file_types/index.html.slim +++ b/app/views/file_types/index.html.slim @@ -11,12 +11,12 @@ h1 = FileType.model_name.human(count: 2) tbody - @file_types.each do |file_type| tr - td = link_to(file_type.name, file_type) - td = link_to(file_type.author, file_type.author) + td = link_to_if(policy(file_type).show?, file_type.name, file_type) + td = link_to_if(policy(file_type.author).show?, file_type.author, file_type.author) td = file_type.file_extension - td = link_to(t('shared.show'), file_type) - td = link_to(t('shared.edit'), edit_file_type_path(file_type)) - td = link_to(t('shared.destroy'), file_type, data: {confirm: t('shared.confirm_destroy')}, method: :delete) + td = link_to(t('shared.show'), file_type) if policy(file_type).show? + td = link_to(t('shared.edit'), edit_file_type_path(file_type)) if policy(file_type).edit? + td = link_to(t('shared.destroy'), file_type, data: {confirm: t('shared.confirm_destroy')}, method: :delete) if policy(file_type).destroy? = render('shared/pagination', collection: @file_types) p = render('shared/new_button', model: FileType) From 2125fb1c1d530c708e443a743c5ed9deb471ca94 Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Fri, 30 Nov 2018 13:29:04 +0100 Subject: [PATCH 05/12] Ensure views only link to those actions allowed for current user --- app/controllers/comments_controller.rb | 33 +++---------------- app/mailers/user_mailer.rb | 1 + app/policies/exercise_policy.rb | 2 +- app/views/admin/dashboard/show.html.slim | 2 +- app/views/application/_breadcrumbs.html.slim | 4 +-- app/views/application/_navigation.html.slim | 4 +-- app/views/application/_session.html.slim | 2 +- app/views/code_harbor_links/index.html.slim | 8 ++--- app/views/code_harbor_links/show.html.slim | 2 +- app/views/comments/_form.html.slim | 33 ------------------- app/views/comments/edit.html.slim | 7 ---- app/views/comments/index.html.slim | 24 -------------- app/views/comments/new.html.slim | 5 --- app/views/comments/show.html.slim | 25 -------------- app/views/consumers/index.html.slim | 8 ++--- app/views/consumers/show.html.slim | 2 +- .../error_template_attributes/index.html.slim | 10 +++--- app/views/error_templates/index.html.slim | 8 ++--- app/views/error_templates/show.html.slim | 9 ++--- .../execution_environments/show.html.slim | 2 +- .../statistics.html.slim | 2 +- .../exercise_collections/index.html.slim | 10 +++--- app/views/exercise_collections/show.html.slim | 6 ++-- app/views/exercises/feedback.html.slim | 2 +- app/views/exercises/index.html.slim | 2 +- app/views/exercises/show.html.slim | 3 +- app/views/exercises/statistics.html.slim | 2 +- app/views/external_users/index.html.slim | 6 ++-- app/views/external_users/show.html.slim | 2 +- app/views/external_users/statistics.html.slim | 2 +- app/views/file_templates/index.html.slim | 10 +++--- app/views/file_templates/show.html.slim | 2 +- app/views/file_types/show.html.slim | 2 +- app/views/internal_users/index.html.slim | 12 +++---- app/views/internal_users/show.html.slim | 5 ++- app/views/interventions/index.html.slim | 5 +-- app/views/proxy_exercises/_form.html.slim | 2 +- app/views/proxy_exercises/index.html.slim | 4 +-- app/views/proxy_exercises/show.html.slim | 5 ++- .../_admin_menu.html.slim | 8 ++--- .../request_for_comments/_form.html.slim | 28 ---------------- .../request_for_comments/index.html.slim | 4 +-- app/views/request_for_comments/show.html.slim | 6 ++-- app/views/shared/_edit_button.html.slim | 7 ++-- app/views/shared/_file.html.slim | 2 +- app/views/submissions/index.html.slim | 8 ++--- app/views/submissions/show.html.slim | 4 +-- app/views/submissions/statistics.html.slim | 2 +- app/views/tags/index.html.slim | 8 ++--- .../user_exercise_feedbacks/index.html.slim | 11 +++---- .../user_exercise_feedbacks/show.html.slim | 4 +-- .../exercise_anomaly_detected.html.slim | 8 ++--- app/views/user_mailer/got_new_comment.slim | 8 ++++- .../user_mailer/send_thank_you_note.slim | 6 +++- config/routes.rb | 2 +- db/schema.rb | 1 + 56 files changed, 128 insertions(+), 264 deletions(-) delete mode 100644 app/views/comments/_form.html.slim delete mode 100644 app/views/comments/edit.html.slim delete mode 100644 app/views/comments/index.html.slim delete mode 100644 app/views/comments/new.html.slim delete mode 100644 app/views/comments/show.html.slim delete mode 100644 app/views/request_for_comments/_form.html.slim diff --git a/app/controllers/comments_controller.rb b/app/controllers/comments_controller.rb index d292804e..ef2ee4db 100644 --- a/app/controllers/comments_controller.rb +++ b/app/controllers/comments_controller.rb @@ -9,7 +9,6 @@ class CommentsController < ApplicationController end private :authorize! - # GET /comments # GET /comments.json def index file = CodeOcean::File.find(params[:file_id]) @@ -29,24 +28,11 @@ class CommentsController < ApplicationController authorize! end - # GET /comments/1 # GET /comments/1.json def show authorize! end - # GET /comments/new - def new - @comment = Comment.new - authorize! - end - - # GET /comments/1/edit - def edit - authorize! - end - - # POST /comments # POST /comments.json def create @comment = Comment.new(comment_params_without_request_id) @@ -59,40 +45,31 @@ class CommentsController < ApplicationController send_mail_to_subscribers @comment, request_for_comment end - format.html { redirect_to @comment, notice: 'Comment was successfully created.' } - format.json { render :show, status: :created, location: @comment } + render :show, status: :created, location: @comment else - format.html { render :new } - format.json { render json: @comment.errors, status: :unprocessable_entity } + render json: @comment.errors, status: :unprocessable_entity end end authorize! end - # PATCH/PUT /comments/1 # PATCH/PUT /comments/1.json def update respond_to do |format| if @comment.update(comment_params_without_request_id) - format.html { head :no_content, notice: 'Comment was successfully updated.' } - format.json { render :show, status: :ok, location: @comment } + render :show, status: :ok, location: @comment else - format.html { render :edit } - format.json { render json: @comment.errors, status: :unprocessable_entity } + render json: @comment.errors, status: :unprocessable_entity end end authorize! end - # DELETE /comments/1 # DELETE /comments/1.json def destroy authorize! @comment.destroy - respond_to do |format| - format.html { head :no_content, notice: 'Comment was successfully destroyed.' } - format.json { head :no_content } - end + head :no_content end private diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb index 497c2a31..708baf03 100644 --- a/app/mailers/user_mailer.rb +++ b/app/mailers/user_mailer.rb @@ -39,6 +39,7 @@ class UserMailer < ActionMailer::Base end def exercise_anomaly_detected(exercise_collection, anomalies) + @user = exercise_collection.user @receiver_displayname = exercise_collection.user.displayname @collection = exercise_collection @anomalies = anomalies diff --git a/app/policies/exercise_policy.rb b/app/policies/exercise_policy.rb index cb9a32cf..63249b1f 100644 --- a/app/policies/exercise_policy.rb +++ b/app/policies/exercise_policy.rb @@ -8,7 +8,7 @@ class ExercisePolicy < AdminOrAuthorPolicy end [:clone?, :destroy?, :edit?, :statistics?, :update?, :feedback?].each do |action| - define_method(action) { admin? || author?} + define_method(action) { admin? || author? } end [:implement?, :working_times?, :intervention?, :search?, :submit?, :reload?].each do |action| diff --git a/app/views/admin/dashboard/show.html.slim b/app/views/admin/dashboard/show.html.slim index 65da67f4..823fea30 100644 --- a/app/views/admin/dashboard/show.html.slim +++ b/app/views/admin/dashboard/show.html.slim @@ -30,7 +30,7 @@ h2 Docker tbody - ExecutionEnvironment.order(:name).each do |execution_environment| tr data-id=execution_environment.id - td.name = link_to(execution_environment, execution_environment) + td.name = link_to_if(policy(execution_environment).show?, execution_environment, execution_environment) td.pool-size td.quantity = progress_bar(0) h3 = t('.history') diff --git a/app/views/application/_breadcrumbs.html.slim b/app/views/application/_breadcrumbs.html.slim index 18045440..fb3a656e 100644 --- a/app/views/application/_breadcrumbs.html.slim +++ b/app/views/application/_breadcrumbs.html.slim @@ -7,9 +7,9 @@ - if object li.breadcrumb-item = object - else - li.breadcrumb-item = link_to(model.model_name.human(count: 2), send(:"#{model.model_name.collection}_path")) + li.breadcrumb-item = link_to_if(policy(model).show?, model.model_name.human(count: 2), send(:"#{model.model_name.collection}_path")) - if object - li.breadcrumb-item = link_to(object, send(:"#{model.model_name.singular}_path", object)) + li.breadcrumb-item = link_to_if(policy(object).show?, object, send(:"#{model.model_name.singular}_path", object)) li.breadcrumb-item.active - if I18n.translation_present?("shared.#{params[:action]}") = t("shared.#{params[:action]}") diff --git a/app/views/application/_navigation.html.slim b/app/views/application/_navigation.html.slim index a23e59da..5e853f0d 100644 --- a/app/views/application/_navigation.html.slim +++ b/app/views/application/_navigation.html.slim @@ -6,8 +6,8 @@ span.caret ul.dropdown-menu.p-0.mt-1 role='menu' - if current_user.admin? - li = link_to(t('breadcrumbs.dashboard.show'), admin_dashboard_path, class: 'dropdown-item', 'data-turbolinks' => "false") - li = link_to(t('breadcrumbs.statistics.show'), statistics_path, class: 'dropdown-item') + li = link_to(t('breadcrumbs.dashboard.show'), admin_dashboard_path, class: 'dropdown-item', 'data-turbolinks' => "false") if policy([:admin, :dashboard]).show? + li = link_to(t('breadcrumbs.statistics.show'), statistics_path, class: 'dropdown-item') if policy(:statistics).show? li.dropdown-divider role='separator' = render('navigation_submenu', title: t('activerecord.models.exercise.other'), models: [Exercise, ExerciseCollection, ProxyExercise, Tag, Submission], link: exercises_path, cached: true) diff --git a/app/views/application/_session.html.slim b/app/views/application/_session.html.slim index abe53b21..9f79ef80 100644 --- a/app/views/application/_session.html.slim +++ b/app/views/application/_session.html.slim @@ -8,7 +8,7 @@ - if current_user.try(:admin?) or current_user.try(:teacher?) li = link_to(t('consumers.show.link'), current_user.consumer, class: 'dropdown-item') if current_user.consumer and policy(current_user.consumer).show? li = link_to(t('internal_users.show.link'), current_user, class: 'dropdown-item') if policy(current_user).show? - li = link_to(t('request_for_comments.index.all'), request_for_comments_path, class: 'dropdown-item') if policy(RequestForComment).index? + li = link_to(t('request_for_comments.index.all'), request_for_comments_path, class: 'dropdown-item') if policy(:request_for_comment).index? li = link_to(t('request_for_comments.index.get_my_rfc_activity'), my_rfc_activity_path, class: 'dropdown-item') li = link_to(t('request_for_comments.index.get_my_comment_requests'), my_request_for_comments_path, class: 'dropdown-item') - if current_user.internal_user? diff --git a/app/views/code_harbor_links/index.html.slim b/app/views/code_harbor_links/index.html.slim index ca4a81d4..c07e43da 100644 --- a/app/views/code_harbor_links/index.html.slim +++ b/app/views/code_harbor_links/index.html.slim @@ -9,10 +9,10 @@ h1 = CodeHarborLink.model_name.human(count: 2) tbody - @code_harbor_links.each do |code_harbor_link| tr - td = link_to(code_harbor_link.oauth2token, code_harbor_link) - td = link_to(t('shared.show'), code_harbor_link) - td = link_to(t('shared.edit'), edit_code_harbor_link_path(code_harbor_link)) - td = link_to(t('shared.destroy'), code_harbor_link, data: {confirm: t('shared.confirm_destroy')}, method: :delete) + td = link_to_if(policy(code_harbor_link).show?, code_harbor_link.oauth2token, code_harbor_link) + td = link_to(t('shared.show'), code_harbor_link) if policy(code_harbor_link).show? + td = link_to(t('shared.edit'), edit_code_harbor_link_path(code_harbor_link)) if policy(code_harbor_link).edit? + td = link_to(t('shared.destroy'), code_harbor_link, data: {confirm: t('shared.confirm_destroy')}, method: :delete) if policy(code_harbor_link).destroy? = render('shared/pagination', collection: @code_harbor_links) p = render('shared/new_button', model: CodeHarborLink) diff --git a/app/views/code_harbor_links/show.html.slim b/app/views/code_harbor_links/show.html.slim index b2d95342..e4a29f75 100644 --- a/app/views/code_harbor_links/show.html.slim +++ b/app/views/code_harbor_links/show.html.slim @@ -1,6 +1,6 @@ h1 = @code_harbor_link - = render('shared/edit_button', object: @code_harbor_link) if policy(@code_harbor_link).edit? + = render('shared/edit_button', object: @code_harbor_link) - %w[oauth2token].each do |attribute| = row(label: "code_harbor_link.#{attribute}") do diff --git a/app/views/comments/_form.html.slim b/app/views/comments/_form.html.slim deleted file mode 100644 index 96149d69..00000000 --- a/app/views/comments/_form.html.slim +++ /dev/null @@ -1,33 +0,0 @@ -= form_for(@comment) do |f| - - if @comment.errors.any? - #error_explanation - h2 - = pluralize(@comment.errors.count, "error") - | prohibited this comment from being saved: - - ul - - @comment.errors.full_messages.each do |message| - li= message - - .field - = f.label :user_id - br/ - = f.text_field :user_id - .field - = f.label :file_id - br/ - = f.text_field :file_id - .field - = f.label :row - br/ - = f.number_field :row - .field - = f.label :column - br/ - = f.number_field :column - .field - = f.label :text - br/ - = f.text_field :text - .actions - = f.submit diff --git a/app/views/comments/edit.html.slim b/app/views/comments/edit.html.slim deleted file mode 100644 index dd0f49e3..00000000 --- a/app/views/comments/edit.html.slim +++ /dev/null @@ -1,7 +0,0 @@ -h1 Editing comment - -= render 'form' - -= link_to 'Show', @comment -| | -= link_to 'Back', comments_path diff --git a/app/views/comments/index.html.slim b/app/views/comments/index.html.slim deleted file mode 100644 index 275a5042..00000000 --- a/app/views/comments/index.html.slim +++ /dev/null @@ -1,24 +0,0 @@ -h1 Listing comments - -table - thead - tr - th User - th File - th Row - th Column - th Text - th colspan="3" - tbody - - @comments.each do |comment| - tr - td= comment.user - td= comment.file - td= comment.row - td= comment.column - td= comment.text - td= link_to 'Show', comment - td= link_to 'Edit', edit_comment_path(comment) - td= link_to 'Destroy', comment, method: :delete, data: confirm: 'Are you sure?' -br/ -= link_to 'New Comment', new_comment_path diff --git a/app/views/comments/new.html.slim b/app/views/comments/new.html.slim deleted file mode 100644 index 1b985e65..00000000 --- a/app/views/comments/new.html.slim +++ /dev/null @@ -1,5 +0,0 @@ -h1 New comment - -= render 'form' - -= link_to 'Back', comments_path diff --git a/app/views/comments/show.html.slim b/app/views/comments/show.html.slim deleted file mode 100644 index 24233d06..00000000 --- a/app/views/comments/show.html.slim +++ /dev/null @@ -1,25 +0,0 @@ -p#notice= notice - -p - strong User: - = @comment.user - -p - strong File: - = @comment.file - -p - strong Row: - = @comment.row - -p - strong Column: - = @comment.column - -p - strong Text: - = @comment.text - -= link_to 'Edit', edit_comment_path(@comment) -| | -= link_to 'Back', comments_path diff --git a/app/views/consumers/index.html.slim b/app/views/consumers/index.html.slim index b707c1f0..9771cac7 100644 --- a/app/views/consumers/index.html.slim +++ b/app/views/consumers/index.html.slim @@ -9,10 +9,10 @@ h1 = Consumer.model_name.human(count: 2) tbody - @consumers.each do |consumer| tr - td = link_to(consumer.name, consumer) - td = link_to(t('shared.show'), consumer) - td = link_to(t('shared.edit'), edit_consumer_path(consumer)) - td = link_to(t('shared.destroy'), consumer, data: {confirm: t('shared.confirm_destroy')}, method: :delete) + td = link_to_if(policy(consumer).show?, consumer.name, consumer) + td = link_to(t('shared.show'), consumer) if policy(consumer).show? + td = link_to(t('shared.edit'), edit_consumer_path(consumer)) if policy(consumer).edit? + td = link_to(t('shared.destroy'), consumer, data: {confirm: t('shared.confirm_destroy')}, method: :delete) if policy(consumer).destroy? = render('shared/pagination', collection: @consumers) p = render('shared/new_button', model: Consumer) diff --git a/app/views/consumers/show.html.slim b/app/views/consumers/show.html.slim index 876be032..c355d819 100644 --- a/app/views/consumers/show.html.slim +++ b/app/views/consumers/show.html.slim @@ -1,6 +1,6 @@ h1 = @consumer - = render('shared/edit_button', object: @consumer) if policy(@consumer).edit? + = render('shared/edit_button', object: @consumer) = row(label: 'consumer.name', value: @consumer.name) - %w[oauth_key oauth_secret].each do |attribute| diff --git a/app/views/error_template_attributes/index.html.slim b/app/views/error_template_attributes/index.html.slim index 268b1547..3bb6ee0a 100644 --- a/app/views/error_template_attributes/index.html.slim +++ b/app/views/error_template_attributes/index.html.slim @@ -8,7 +8,7 @@ h1 = ErrorTemplateAttribute.model_name.human(count: 2) th = t('activerecord.attributes.error_template_attribute.key') th = t('activerecord.attributes.error_template_attribute.description') th = t('activerecord.attributes.error_template_attribute.regex') - th colspan=5 = t('shared.actions') + th colspan=3 = t('shared.actions') tbody - @error_template_attributes.each do |error_template_attribute| tr @@ -17,13 +17,13 @@ h1 = ErrorTemplateAttribute.model_name.human(count: 2) span class="fa fa-star" aria-hidden="true" - else span class="fa fa-star-o" aria-hidden="true" - td = link_to(error_template_attribute.key, error_template_attribute) + td = link_to_if(policy(error_template_attribute).show?, error_template_attribute.key, error_template_attribute) td = error_template_attribute.description td code = error_template_attribute.regex - td = link_to(t('shared.show'), error_template_attribute) - td = link_to(t('shared.edit'), edit_error_template_attribute_path(error_template_attribute)) - td = link_to(t('shared.destroy'), error_template_attribute, data: {confirm: t('shared.confirm_destroy')}, method: :delete) + td = link_to(t('shared.show'), error_template_attribute) if policy(error_template_attribute).show? + td = link_to(t('shared.edit'), edit_error_template_attribute_path(error_template_attribute)) if policy(error_template_attribute).edit? + td = link_to(t('shared.destroy'), error_template_attribute, data: {confirm: t('shared.confirm_destroy')}, method: :delete) if policy(error_template_attribute).destroy? = render('shared/pagination', collection: @error_template_attributes) p = render('shared/new_button', model: ErrorTemplateAttribute) diff --git a/app/views/error_templates/index.html.slim b/app/views/error_templates/index.html.slim index 1f532a86..fb2b9c3d 100644 --- a/app/views/error_templates/index.html.slim +++ b/app/views/error_templates/index.html.slim @@ -11,12 +11,12 @@ h1 = ErrorTemplate.model_name.human(count: 2) tbody - @error_templates.each do |error_template| tr - td = link_to(error_template.name, error_template) + td = link_to_if(policy(error_template).show?, error_template.name, error_template) td = error_template.description td = link_to(error_template.execution_environment) - td = link_to(t('shared.show'), error_template) - td = link_to(t('shared.edit'), edit_error_template_path(error_template)) - td = link_to(t('shared.destroy'), error_template, data: {confirm: t('shared.confirm_destroy')}, method: :delete) + td = link_to(t('shared.show'), error_template) if policy(error_template).show? + td = link_to(t('shared.edit'), edit_error_template_path(error_template)) if policy(error_template).edit? + td = link_to(t('shared.destroy'), error_template, data: {confirm: t('shared.confirm_destroy')}, method: :delete) if policy(error_template).destroy? = render('shared/pagination', collection: @error_templates) p = render('shared/new_button', model: ErrorTemplate) diff --git a/app/views/error_templates/show.html.slim b/app/views/error_templates/show.html.slim index ae6ea2a4..19903fd8 100644 --- a/app/views/error_templates/show.html.slim +++ b/app/views/error_templates/show.html.slim @@ -3,7 +3,7 @@ h1 = render('shared/edit_button', object: @error_template) = row(label: 'error_template.name', value: @error_template.name) -= row(label: 'exercise.execution_environment', value: link_to(@error_template.execution_environment)) += row(label: 'exercise.execution_environment', value: link_to_if(policy(@error_template.execution_environment).show?, @error_template.execution_environment)) = row(label: "error_template.signature") do code = @error_template.signature - [:description, :hint].each do |attribute| @@ -29,12 +29,13 @@ h2.mt-4 span class="fa fa-star" aria-hidden="true" - else span class="fa fa-star-o" aria-hidden="true" - td = link_to(attribute.key, attribute) + td = link_to_if(policy(attribute).show?, attribute.key, attribute) td = attribute.description td code = attribute.regex - td = link_to(t('shared.show'), attribute) - td = link_to(t('shared.destroy'), attribute_error_template_url(:error_template_attribute_id => attribute.id), :method => :delete) + td = link_to(t('shared.show'), attribute) if policy(attribute).show? + td = link_to(t('shared.edit'), edit_error_template_attribute_path(attribute)) if policy(attribute).edit? + td = link_to(t('shared.destroy'), attribute_error_template_url(:error_template_attribute_id => attribute.id), :method => :delete) if policy(attribute).destroy? #add-attribute = collection_select({}, :error_template_attribute_id, diff --git a/app/views/execution_environments/show.html.slim b/app/views/execution_environments/show.html.slim index 6d917a61..21133a71 100644 --- a/app/views/execution_environments/show.html.slim +++ b/app/views/execution_environments/show.html.slim @@ -3,7 +3,7 @@ h1 = render('shared/edit_button', object: @execution_environment) = row(label: 'execution_environment.name', value: @execution_environment.name) -= row(label: 'execution_environment.user', value: link_to(@execution_environment.author, @execution_environment.author)) += row(label: 'execution_environment.user', value: link_to_if(policy(@execution_environment.author).show?, @execution_environment.author, @execution_environment.author)) = row(label: 'execution_environment.file_type', value: @execution_environment.file_type.present? ? link_to(@execution_environment.file_type, @execution_environment.file_type) : nil) - [:docker_image, :exposed_ports, :memory_limit, :network_enabled, :permitted_execution_time, :pool_size].each do |attribute| = row(label: "execution_environment.#{attribute}", value: @execution_environment.send(attribute)) diff --git a/app/views/execution_environments/statistics.html.slim b/app/views/execution_environments/statistics.html.slim index 64d046b0..c90b42b6 100644 --- a/app/views/execution_environments/statistics.html.slim +++ b/app/views/execution_environments/statistics.html.slim @@ -14,7 +14,7 @@ h1 = @execution_environment - if wts then average_time = wts["average_time"] else 0 - if wts then stddev_time = wts["stddev_time"] else 0 tr - td = link_to exercise.title, controller: "exercises", action: "statistics", id: exercise.id, 'data-turbolinks' => "false" + td = link_to_if policy(exercise).show?, exercise.title, controller: "exercises", action: "statistics", id: exercise.id, 'data-turbolinks' => "false" td = us["users"] td = us["average_score"].to_f.round(4) td = us["maximum_score"].to_f.round(2) diff --git a/app/views/exercise_collections/index.html.slim b/app/views/exercise_collections/index.html.slim index e0e8ebbc..8e5dff54 100644 --- a/app/views/exercise_collections/index.html.slim +++ b/app/views/exercise_collections/index.html.slim @@ -13,13 +13,13 @@ h1 = ExerciseCollection.model_name.human(count: 2) - @exercise_collections.each do |collection| tr td = collection.id - td = link_to(collection.name, collection) + td = link_to_if(policy(collection).show?, collection.name, collection) td = collection.updated_at td = collection.exercises.size - td = link_to(t('shared.show'), collection) - td = link_to(t('shared.edit'), edit_exercise_collection_path(collection)) - td = link_to(t('shared.statistics'), statistics_exercise_collection_path(collection)) - td = link_to(t('shared.destroy'), collection, data: {confirm: t('shared.confirm_destroy')}, method: :delete) + td = link_to(t('shared.show'), collection) if policy(collection).show? + td = link_to(t('shared.edit'), edit_exercise_collection_path(collection)) if policy(collection).edit? + td = link_to(t('shared.statistics'), statistics_exercise_collection_path(collection)) if policy(collection).statistics? + td = link_to(t('shared.destroy'), collection, data: {confirm: t('shared.confirm_destroy')}, method: :delete) if policy(collection).destroy? = render('shared/pagination', collection: @exercise_collections) p = render('shared/new_button', model: ExerciseCollection) diff --git a/app/views/exercise_collections/show.html.slim b/app/views/exercise_collections/show.html.slim index 4fa6a8ac..fb1eded4 100644 --- a/app/views/exercise_collections/show.html.slim +++ b/app/views/exercise_collections/show.html.slim @@ -3,7 +3,7 @@ h1 = render('shared/edit_button', object: @exercise_collection) = row(label: 'exercise_collections.name', value: @exercise_collection.name) -= row(label: 'exercise_collections.user', value: link_to(@exercise_collection.user.name, @exercise_collection.user)) unless @exercise_collection.user.nil? += row(label: 'exercise_collections.user', value: link_to_if(policy(@exercise_collection.user).show?, @exercise_collection.user.name, @exercise_collection.user)) unless @exercise_collection.user.nil? = row(label: 'exercise_collections.use_anomaly_detection', value: @exercise_collection.use_anomaly_detection) = row(label: 'exercise_collections.updated_at', value: @exercise_collection.updated_at) @@ -22,7 +22,7 @@ h4.mt-4 = t('activerecord.attributes.exercise_collections.exercises') - exercise = exercise_collection_item.exercise tr td = exercise_collection_item.position - td = link_to(exercise.title, exercise) + td = link_to_if(policy(exercise).show?, exercise.title, exercise) td = link_to_if(exercise.execution_environment && policy(exercise.execution_environment).show?, exercise.execution_environment, exercise.execution_environment) td = link_to_if(exercise.user && policy(exercise.user).show?, exercise.user.name, exercise.user) - td = link_to(t('shared.statistics'), statistics_exercise_path(exercise), 'data-turbolinks' => "false") + td = link_to(t('shared.statistics'), statistics_exercise_path(exercise), 'data-turbolinks' => "false") if policy(exercise).statistics? diff --git a/app/views/exercises/feedback.html.slim b/app/views/exercises/feedback.html.slim index 5090d0e2..a00e1be6 100644 --- a/app/views/exercises/feedback.html.slim +++ b/app/views/exercises/feedback.html.slim @@ -1,4 +1,4 @@ -h1 = link_to(@exercise, exercise_path(@exercise)) +h1 = link_to_if(policy(@exercise).show?, @exercise, exercise_path(@exercise)) .feedback-page .header = t('activerecord.attributes.exercise.description') diff --git a/app/views/exercises/index.html.slim b/app/views/exercises/index.html.slim index 35da19cb..9c26ab6f 100644 --- a/app/views/exercises/index.html.slim +++ b/app/views/exercises/index.html.slim @@ -27,7 +27,7 @@ h1 = Exercise.model_name.human(count: 2) tbody - @exercises.each do |exercise| tr data-id=exercise.id - td.p-1.pt-2 = link_to(exercise.title, exercise, 'data-turbolinks' => "false") if policy(exercise).show? + td.p-1.pt-2 = link_to_if(policy(exercise).show?, exercise.title, exercise, 'data-turbolinks' => "false") td.p-1.pt-2 = link_to_if(exercise.execution_environment && policy(exercise.execution_environment).show?, exercise.execution_environment, exercise.execution_environment) td.p-1.pt-2 = exercise.files.teacher_defined_tests.count td.p-1.pt-2 = exercise.maximum_score diff --git a/app/views/exercises/show.html.slim b/app/views/exercises/show.html.slim index ee503c99..e2f7ada9 100644 --- a/app/views/exercises/show.html.slim +++ b/app/views/exercises/show.html.slim @@ -7,8 +7,7 @@ h1 = @exercise - - if policy(@exercise).edit? - = render('shared/edit_button', object: @exercise) + = render('shared/edit_button', object: @exercise) = row(label: 'exercise.title', value: @exercise.title) = row(label: 'exercise.user', value: link_to_if(policy(@exercise.author).show?, @exercise.author, @exercise.author)) diff --git a/app/views/exercises/statistics.html.slim b/app/views/exercises/statistics.html.slim index 5455162b..c982adff 100644 --- a/app/views/exercises/statistics.html.slim +++ b/app/views/exercises/statistics.html.slim @@ -49,7 +49,7 @@ h1 = @exercise - if user_statistics[user.id] then us = user_statistics[user.id] else us = {"maximum_score" => nil, "runs" => nil} - label = "#{user.displayname}" tr - td = link_to_if symbol==:external_users, label, {controller: "exercises", action: "statistics", external_user_id: user.id, id: @exercise.id} + td = link_to_if symbol==:external_users && policy(user).statistics?, label, {controller: "exercises", action: "statistics", external_user_id: user.id, id: @exercise.id} td = us['maximum_score'] or 0 td = us['runs'] td = @exercise.average_working_time_for(user.id) or 0 diff --git a/app/views/external_users/index.html.slim b/app/views/external_users/index.html.slim index 34359b2a..1b24fa4d 100644 --- a/app/views/external_users/index.html.slim +++ b/app/views/external_users/index.html.slim @@ -10,8 +10,8 @@ h1 = ExternalUser.model_name.human(count: 2) tbody - @users.each do |user| tr - td = user.name - td = link_to(user.consumer, user.consumer) - td = link_to(t('shared.show'), user) + td = link_to_if(policy(user).show?, user.name) + td = link_to_if(policy(user.consumer).show?, user.consumer, user.consumer) + td = link_to(t('shared.show'), user) if policy(user).show? = render('shared/pagination', collection: @users) diff --git a/app/views/external_users/show.html.slim b/app/views/external_users/show.html.slim index 9d18e942..ea26d4d2 100644 --- a/app/views/external_users/show.html.slim +++ b/app/views/external_users/show.html.slim @@ -5,7 +5,7 @@ h1 = @user.name = row(label: 'external_user.consumer', value: link_to(@user.consumer, @user.consumer)) = row(label: 'external_user.role', value: t("users.roles.#{@user.role}")) -h4.mt-4 = link_to(t('.exercise_statistics'), statistics_external_user_path(@user)) +h4.mt-4 = link_to(t('.exercise_statistics'), statistics_external_user_path(@user)) if policy(@user).statistics? h4.mt-4 = t('.tag_statistics') #loading diff --git a/app/views/external_users/statistics.html.slim b/app/views/external_users/statistics.html.slim index b252c3a2..368846a8 100644 --- a/app/views/external_users/statistics.html.slim +++ b/app/views/external_users/statistics.html.slim @@ -13,7 +13,7 @@ h1 = t('.title') - if statistics[exercise.id] - stats = statistics[exercise.id] tr - td = link_to exercise, controller: "exercises", action: "statistics", external_user_id: @user.id, id: exercise.id + td = link_to_if policy(exercise).show?, exercise, controller: "exercises", action: "statistics", external_user_id: @user.id, id: exercise.id td = stats["maximum_score"] or 0 td = stats["runs"] or 0 td = stats["working_time"] or 0 diff --git a/app/views/file_templates/index.html.slim b/app/views/file_templates/index.html.slim index ba7c3eb7..dbe2db36 100644 --- a/app/views/file_templates/index.html.slim +++ b/app/views/file_templates/index.html.slim @@ -10,11 +10,11 @@ h1 = FileTemplate.model_name.human(count: 2) tbody - @file_templates.each do |file_template| tr - td = link_to(file_template.name, file_template) - td = link_to(file_template.file_type, file_type_path(file_template.file_type)) - td = link_to(t('shared.show'), file_template) - td = link_to(t('shared.edit'), edit_file_template_path(file_template)) - td = link_to(t('shared.destroy'), file_template, data: {confirm: t('shared.confirm_destroy')}, method: :delete) + td = link_to_if(policy(file_template).show?, file_template.name, file_template) + td = link_to_if(policy(file_template.file_type).show?, file_template.file_type, file_type_path(file_template.file_type)) + td = link_to(t('shared.show'), file_template) if policy(file_template).show? + td = link_to(t('shared.edit'), edit_file_template_path(file_template)) if policy(file_template).edit? + td = link_to(t('shared.destroy'), file_template, data: {confirm: t('shared.confirm_destroy')}, method: :delete) if policy(file_template).destroy? = render('shared/pagination', collection: @file_templates) p = render('shared/new_button', model: FileTemplate) diff --git a/app/views/file_templates/show.html.slim b/app/views/file_templates/show.html.slim index 19f0d28f..b691aad9 100644 --- a/app/views/file_templates/show.html.slim +++ b/app/views/file_templates/show.html.slim @@ -3,5 +3,5 @@ h1 = render('shared/edit_button', object: @file_template) = row(label: 'file_template.name', value: @file_template.name) -= row(label: 'file_template.file_type', value: link_to(@file_template.file_type, file_type_path(@file_template.file_type))) += row(label: 'file_template.file_type', value: link_to_if(policy(@file_template.file_type).show?, @file_template.file_type, file_type_path(@file_template.file_type))) = row(label: 'file_template.content', value: @file_template.content) diff --git a/app/views/file_types/show.html.slim b/app/views/file_types/show.html.slim index 10a02cc3..d6e39500 100644 --- a/app/views/file_types/show.html.slim +++ b/app/views/file_types/show.html.slim @@ -3,6 +3,6 @@ h1 = render('shared/edit_button', object: @file_type) = row(label: 'file_type.name', value: @file_type.name) -= row(label: 'file_type.user', value: link_to(@file_type.author, @file_type.author)) += row(label: 'file_type.user', value: link_to_if(policy(@file_type.author).show?, @file_type.author, @file_type.author)) - [:editor_mode, :file_extension, :indent_size, :binary, :executable, :renderable].each do |attribute| = row(label: "file_type.#{attribute}", value: @file_type.send(attribute)) diff --git a/app/views/internal_users/index.html.slim b/app/views/internal_users/index.html.slim index 29f07649..0f1493e4 100644 --- a/app/views/internal_users/index.html.slim +++ b/app/views/internal_users/index.html.slim @@ -22,14 +22,12 @@ h1 = InternalUser.model_name.human(count: 2) tbody - @users.each do |user| tr - td = user.name - td = user.consumer ? link_to(user.consumer, user.consumer) : empty + td = link_to_if(policy(user).show?, user.name) + td = user.consumer ? link_to_if(policy(user.consumer).show?, user.consumer, user.consumer) : empty td = t("users.roles.#{user.role}") - td = link_to(t('shared.show'), user) - td = link_to(t('shared.edit'), edit_internal_user_path(user)) - td - - if policy(user).destroy? - = link_to(t('shared.destroy'), user, data: {confirm: t('shared.confirm_destroy')}, method: :delete) + td = link_to(t('shared.show'), user) if policy(user).show? + td = link_to(t('shared.edit'), edit_internal_user_path(user)) if policy(user).edit? + td = link_to(t('shared.destroy'), user, data: {confirm: t('shared.confirm_destroy')}, method: :delete) if policy(user).destroy? = render('shared/pagination', collection: @users) p = render('shared/new_button', model: InternalUser) diff --git a/app/views/internal_users/show.html.slim b/app/views/internal_users/show.html.slim index 158351b2..167443f6 100644 --- a/app/views/internal_users/show.html.slim +++ b/app/views/internal_users/show.html.slim @@ -1,10 +1,9 @@ h1 = @user - - if policy(@user).edit? - = render('shared/edit_button', object: @user) + = render('shared/edit_button', object: @user) = row(label: 'internal_user.email', value: @user.email) = row(label: 'internal_user.name', value: @user.name) -= row(label: 'internal_user.consumer', value: @user.consumer ? link_to(@user.consumer, @user.consumer) : nil) += row(label: 'internal_user.consumer', value: @user.consumer ? link_to_if(policy(@user.consumer).show?, @user.consumer, @user.consumer) : nil) = row(label: 'internal_user.role', value: t("users.roles.#{@user.role}")) = row(label: 'internal_user.activated', value: @user.activated?) diff --git a/app/views/interventions/index.html.slim b/app/views/interventions/index.html.slim index fc7afe05..92763567 100644 --- a/app/views/interventions/index.html.slim +++ b/app/views/interventions/index.html.slim @@ -5,10 +5,11 @@ h1 = Intervention.model_name.human(count: 2) thead tr th = t('activerecord.attributes.intervention.name') + th = t('shared.actions') tbody - @interventions.each do |intervention| tr - td = intervention.name - td = link_to(t('shared.show'), intervention) + td = link_to_if policy(intervention).show?, intervention.name + td = link_to(t('shared.show'), intervention) if policy(intervention).show? = render('shared/pagination', collection: @interventions) diff --git a/app/views/proxy_exercises/_form.html.slim b/app/views/proxy_exercises/_form.html.slim index 601ef950..fce092ba 100644 --- a/app/views/proxy_exercises/_form.html.slim +++ b/app/views/proxy_exercises/_form.html.slim @@ -18,7 +18,7 @@ = collection_check_boxes :proxy_exercise, :exercise_ids, @exercises, :id, :title do |b| tr td = b.check_box - td = link_to(b.object, b.object) + td = link_to_if(policy(b.object).show?, b.object, b.object) td = l(b.object.created_at, format: :short) .actions = render('shared/submit_button', f: f, object: @proxy_exercise) \ No newline at end of file diff --git a/app/views/proxy_exercises/index.html.slim b/app/views/proxy_exercises/index.html.slim index a2e7e460..2cc84454 100644 --- a/app/views/proxy_exercises/index.html.slim +++ b/app/views/proxy_exercises/index.html.slim @@ -12,11 +12,11 @@ h1 = ProxyExercise.model_name.human(count: 2) th.p-1 = sort_link(@search, :title, t('activerecord.attributes.proxy_exercise.title')) th.p-1 = t('activerecord.attributes.exercise.token') th.p-1 = t('activerecord.attributes.proxy_exercise.files_count') - th.p-1 colspan=6 = t('shared.actions') + th.p-1 colspan=2 = t('shared.actions') tbody - @proxy_exercises.each do |proxy_exercise| tr data-id=proxy_exercise.id - td.p-1.pt-2 = link_to(proxy_exercise.title,proxy_exercise) + td.p-1.pt-2 = link_to_if(policy(proxy_exercise).show?, proxy_exercise.title, proxy_exercise) td.p-1.pt-2 = proxy_exercise.token td.p-1.pt-2 = proxy_exercise.count_files td.p-1.pt-2 = link_to(t('shared.edit'), edit_proxy_exercise_path(proxy_exercise)) if policy(proxy_exercise).edit? diff --git a/app/views/proxy_exercises/show.html.slim b/app/views/proxy_exercises/show.html.slim index 07f5061a..ec8f3212 100644 --- a/app/views/proxy_exercises/show.html.slim +++ b/app/views/proxy_exercises/show.html.slim @@ -7,8 +7,7 @@ h1 = @proxy_exercise.title - - if policy(@proxy_exercise).edit? - = render('shared/edit_button', object: @proxy_exercise) + = render('shared/edit_button', object: @proxy_exercise) = row(label: 'exercise.title', value: @proxy_exercise.title) = row(label: 'proxy_exercise.files_count', value: @exercises.count) @@ -24,5 +23,5 @@ h2.mt-4 Exercises th = sort_link(@search, :created_at, t('shared.created_at')) - @proxy_exercise.exercises.each do |exercise| tr - td = link_to(exercise.title, exercise) + td = link_to_if(policy(exercise).show?, exercise.title, exercise) td = l(exercise.created_at, format: :short) diff --git a/app/views/request_for_comments/_admin_menu.html.slim b/app/views/request_for_comments/_admin_menu.html.slim index cfdfccf1..64580229 100644 --- a/app/views/request_for_comments/_admin_menu.html.slim +++ b/app/views/request_for_comments/_admin_menu.html.slim @@ -1,8 +1,8 @@ hr h5.mt-4 Admin Menu ul.text - li = link_to "User's current status of this exercise", statistics_external_user_exercise_path(id: @request_for_comment.exercise_id, external_user_id: @request_for_comment.user_id) - li = link_to "All exercises of this user", statistics_external_user_path(id: @request_for_comment.user_id) + li = link_to "User's current status of this exercise", statistics_external_user_exercise_path(id: @request_for_comment.exercise_id, external_user_id: @request_for_comment.user_id) if policy(@request_for_comment.exercise).statistics? + li = link_to "All exercises of this user", statistics_external_user_path(id: @request_for_comment.user_id) if policy(@request_for_comment.user).statistics? ul.text - li = link_to "Implement the exercise yourself", implement_exercise_path(id: @request_for_comment.exercise_id) - li = link_to "Show the exercise", exercise_path(id: @request_for_comment.exercise_id) + li = link_to "Implement the exercise yourself", implement_exercise_path(id: @request_for_comment.exercise_id) if policy(@request_for_comment.exercise).implement? + li = link_to "Show the exercise", exercise_path(id: @request_for_comment.exercise_id) if policy(@request_for_comment.exercise).show? diff --git a/app/views/request_for_comments/_form.html.slim b/app/views/request_for_comments/_form.html.slim deleted file mode 100644 index f24ddd22..00000000 --- a/app/views/request_for_comments/_form.html.slim +++ /dev/null @@ -1,28 +0,0 @@ -= form_for(@request_for_comment) do |f| - - if @request_for_comment.errors.any? - #error_explanation - h2 - = pluralize(@request_for_comment.errors.count, "error") - | prohibited this request_for_comment from being saved: - ul - - @request_for_comment.errors.full_messages.each do |message| - li= message - - .field - = f.label :user_id - br/ - = f.number_field :user_id - .field - = f.label :exercise_id - br/ - = f.number_field :exercise_id - .field - = f.label :file_id - br/ - = f.number_field :file_id - .field - = f.label :user_type - br/ - = f.text_field :user_type - .actions - = f.submit diff --git a/app/views/request_for_comments/index.html.slim b/app/views/request_for_comments/index.html.slim index 94262267..eb266d64 100644 --- a/app/views/request_for_comments/index.html.slim +++ b/app/views/request_for_comments/index.html.slim @@ -32,8 +32,8 @@ h1 = RequestForComment.model_name.human(count: 2) span class="fa fa-check" style="color:darkgrey" aria-hidden="true" - else td = '' - td = link_to(request_for_comment.exercise.title, request_for_comment) - - if request_for_comment.has_attribute?(:question) && request_for_comment.question + td = link_to_if(policy(request_for_comment.exercise).show?, request_for_comment.exercise.title, request_for_comment) + - if request_for_comment.has_attribute?(:question) && request_for_comment.question.present? td = truncate(request_for_comment.question, length: 200) - else td = '-' diff --git a/app/views/request_for_comments/show.html.slim b/app/views/request_for_comments/show.html.slim index a55764c4..479dd29d 100644 --- a/app/views/request_for_comments/show.html.slim +++ b/app/views/request_for_comments/show.html.slim @@ -2,12 +2,12 @@ h4#exercise_caption.list-group-item-heading data-comment-exercise-url=create_comment_exercise_request_for_comment_path data-exercise-id="#{@request_for_comment.exercise.id}" data-rfc-id="#{@request_for_comment.id}" - if @request_for_comment.solved? span.fa.fa-check aria-hidden="true" - = link_to(@request_for_comment.exercise.title, [:implement, @request_for_comment.exercise]) + = link_to_if(policy(@request_for_comment.exercise).show?, @request_for_comment.exercise.title, [:implement, @request_for_comment.exercise]) p.list-group-item-text - user = @request_for_comment.user - submission = @request_for_comment.submission - testruns = Testrun.where(:submission_id => @request_for_comment.submission) - = user.displayname + = link_to_if(policy(user).show?, user.displayname, user) | | #{@request_for_comment.created_at.localtime} .rfc .description @@ -22,7 +22,7 @@ = t('activerecord.attributes.request_for_comments.question') .text - question = @request_for_comment.question - = question.nil? or question.empty? ? t('request_for_comments.no_question') : question + = question.blank? ? t('request_for_comments.no_question') : question - if policy(@request_for_comment).mark_as_solved? and not @request_for_comment.solved? = render('mark_as_solved') diff --git a/app/views/shared/_edit_button.html.slim b/app/views/shared/_edit_button.html.slim index 36f35639..a0c4bd21 100644 --- a/app/views/shared/_edit_button.html.slim +++ b/app/views/shared/_edit_button.html.slim @@ -1,3 +1,4 @@ -// default value for fetch will always be evaluated even if it is not returned -- link_target = local_assigns.fetch(:path, false) || send(:"edit_#{object.class.name.underscore}_path", object) -= link_to(t('shared.edit'), link_target, class: 'btn btn-secondary float-right') +- if policy(object).edit? + // default value for fetch will always be evaluated even if it is not returned + - link_target = local_assigns.fetch(:path, false) || send(:"edit_#{object.class.name.underscore}_path", object) + = link_to(t('shared.edit'), link_target, class: 'btn btn-secondary float-right') diff --git a/app/views/shared/_file.html.slim b/app/views/shared/_file.html.slim index cc47be10..01fa812b 100644 --- a/app/views/shared/_file.html.slim +++ b/app/views/shared/_file.html.slim @@ -7,4 +7,4 @@ - if file.teacher_defined_test? = row(label: 'file.feedback_message', value: render_markdown(file.feedback_message), class: 'm-0') = row(label: 'file.weight', value: file.weight) -= row(label: 'file.content', value: file.native_file? ? link_to(file.native_file.file.filename, file.native_file.url) : code_tag(file.content)) += row(label: 'file.content', value: file.native_file? ? link_to_if(policy(file.native_file.file.filename).show?, file.native_file.file.filename, file.native_file.url) : code_tag(file.content)) diff --git a/app/views/submissions/index.html.slim b/app/views/submissions/index.html.slim index 45ff355f..b37e07eb 100644 --- a/app/views/submissions/index.html.slim +++ b/app/views/submissions/index.html.slim @@ -21,12 +21,12 @@ h1 = Submission.model_name.human(count: 2) tbody - @submissions.each do |submission| tr - td = link_to(submission.exercise, submission.exercise) - td = link_to(submission.user, submission.user) + td = link_to_if(policy(submission.exercise).show?, submission.exercise, submission.exercise) + td = link_to_if(policy(submission.user).show?, submission.user, submission.user) td = t("submissions.causes.#{submission.cause}") td = submission.score td = l(submission.created_at, format: :short) - td = link_to(t('shared.show'), submission) - td = link_to(t('shared.statistics'), statistics_submission_path(submission)) + td = link_to(t('shared.show'), submission) if policy(submission).show? + td = link_to(t('shared.statistics'), statistics_submission_path(submission)) if policy(submission).statistics? = render('shared/pagination', collection: @submissions) diff --git a/app/views/submissions/show.html.slim b/app/views/submissions/show.html.slim index ebf1ba72..9a7ab69b 100644 --- a/app/views/submissions/show.html.slim +++ b/app/views/submissions/show.html.slim @@ -7,8 +7,8 @@ h1 = @submission -= row(label: 'submission.exercise', value: link_to(@submission.exercise, @submission.exercise)) -= row(label: 'submission.user', value: link_to(@submission.user, @submission.user)) += row(label: 'submission.exercise', value: link_to_if(policy(@submission.exercise).show?, @submission.exercise, @submission.exercise)) += row(label: 'submission.user', value: link_to_if(policy(@submission.user).show?, @submission.user, @submission.user)) = row(label: 'submission.cause', value: t("submissions.causes.#{@submission.cause}")) = row(label: 'submission.score', value: @submission.score) diff --git a/app/views/submissions/statistics.html.slim b/app/views/submissions/statistics.html.slim index 1b07a984..4d3732bd 100644 --- a/app/views/submissions/statistics.html.slim +++ b/app/views/submissions/statistics.html.slim @@ -23,4 +23,4 @@ h2.mt-4 = t('.history') td = l(submission.created_at, format: :short) td = submission.score td = progress_bar(submission.percentage) - td = link_to(t('shared.show'), submission) + td = link_to(t('shared.show'), submission) if policy(submission).show? diff --git a/app/views/tags/index.html.slim b/app/views/tags/index.html.slim index 449a1165..84e57dbb 100644 --- a/app/views/tags/index.html.slim +++ b/app/views/tags/index.html.slim @@ -9,10 +9,10 @@ h1 = Tag.model_name.human(count: 2) tbody - @tags.each do |tag| tr - td = tag.name - td = link_to(t('shared.show'), tag) - td = link_to(t('shared.edit'), edit_tag_path(tag)) - td = link_to(t('shared.destroy'), tag, data: {confirm: t('shared.confirm_destroy')}, method: :delete) if tag.can_be_destroyed? + td = link_to_if(policy(tag).show?, tag.name, tag) + td = link_to(t('shared.show'), tag) if policy(tag).show? + td = link_to(t('shared.edit'), edit_tag_path(tag)) if policy(tag).edit? + td = link_to(t('shared.destroy'), tag, data: {confirm: t('shared.confirm_destroy')}, method: :delete) if tag.can_be_destroyed? && policy(tag).destroy? = render('shared/pagination', collection: @tags) p = render('shared/new_button', model: Tag, path: new_tag_path) diff --git a/app/views/user_exercise_feedbacks/index.html.slim b/app/views/user_exercise_feedbacks/index.html.slim index 52249229..ba69183f 100644 --- a/app/views/user_exercise_feedbacks/index.html.slim +++ b/app/views/user_exercise_feedbacks/index.html.slim @@ -12,16 +12,15 @@ h1 = UserExerciseFeedback.model_name.human(count: 2) table.table thead tr - th colspan=2 = t('activerecord.attributes.user_exercise_feedback.user') + th = t('activerecord.attributes.user_exercise_feedback.user') th = t('activerecord.attributes.user_exercise_feedback.exercise') th colspan=2 = t('shared.actions') tbody - @uefs.each do |uef| tr - td = uef.user.id - td = uef.user.name - td = link_to(uef.exercise.title, uef.exercise) - td = link_to(t('shared.show'), uef) - td = link_to(t('shared.destroy'), uef, data: {confirm: t('shared.confirm_destroy')}, method: :delete) + td = link_to_if(policy(uef.user).show?, uef.user.name) + td = link_to_if(policy(uef.exercise).show?, uef.exercise.title, uef.exercise) + td = link_to(t('shared.show'), uef) if policy(uef).show? + td = link_to(t('shared.destroy'), uef, data: {confirm: t('shared.confirm_destroy')}, method: :delete) if policy(uef).destroy? = render('shared/pagination', collection: @uefs) diff --git a/app/views/user_exercise_feedbacks/show.html.slim b/app/views/user_exercise_feedbacks/show.html.slim index 2c53e47f..69b945f6 100644 --- a/app/views/user_exercise_feedbacks/show.html.slim +++ b/app/views/user_exercise_feedbacks/show.html.slim @@ -1,7 +1,7 @@ h2 = @uef -= row(label: 'activerecord.attributes.user_exercise_feedback.exercise', value: link_to(@uef.exercise.title, @uef.exercise)) -= row(label: 'user_exercise_feedback.user', value: @uef.user) += row(label: 'activerecord.attributes.user_exercise_feedback.exercise', value: link_to_if(policy(@uef.exercise).show?, @uef.exercise.title, @uef.exercise)) += row(label: 'user_exercise_feedback.user', value: link_to_if(policy(@uef.user).show?, @uef.user)) = row(label: 'activerecord.attributes.user_exercise_feedback.feedback_text', value: @uef.feedback_text) = row(label: 'user_exercise_feedback.difficulty', value: @uef.difficulty) = row(label: 'user_exercise_feedback.working_time', value: @uef.user_estimated_worktime) diff --git a/app/views/user_mailer/exercise_anomaly_detected.html.slim b/app/views/user_mailer/exercise_anomaly_detected.html.slim index 56233ce0..4384a30b 100644 --- a/app/views/user_mailer/exercise_anomaly_detected.html.slim +++ b/app/views/user_mailer/exercise_anomaly_detected.html.slim @@ -12,9 +12,9 @@ table(border=1) - @anomalies.keys.each do | id | - exercise = Exercise.find(id) tr - td = link_to(exercise.title, exercise_path(exercise)) + td = link_to_if(policy(@user, exercise).show?, exercise.title, exercise_path(exercise)) td = @anomalies[id] - td = link_to(t('shared.statistics', locale: :de), statistics_exercise_path(exercise)) + td = link_to_if(policy(@user, exercise).statistics?, t('shared.statistics', locale: :de), statistics_exercise_path(exercise)) == t('mailers.user_mailer.exercise_anomaly_detected.body2', @@ -31,8 +31,8 @@ table(border=1) - @anomalies.keys.each do | id | - exercise = Exercise.find(id) tr - td = link_to(exercise.title, exercise_path(exercise)) + td = link_to_if(policy(@user, exercise).show?, exercise.title, exercise_path(exercise)) td = @anomalies[id] - td = link_to(t('shared.statistics', locale: :en), statistics_exercise_path(exercise)) + td = link_to_if(policy(@user, exercise).statistics?, t('shared.statistics', locale: :en), statistics_exercise_path(exercise)) == t('mailers.user_mailer.exercise_anomaly_detected.body3') diff --git a/app/views/user_mailer/got_new_comment.slim b/app/views/user_mailer/got_new_comment.slim index 1f06ae49..af647e5f 100644 --- a/app/views/user_mailer/got_new_comment.slim +++ b/app/views/user_mailer/got_new_comment.slim @@ -1 +1,7 @@ -== t('mailers.user_mailer.got_new_comment.body', receiver_displayname: @receiver_displayname, link_to_comment: link_to(@rfc_link, @rfc_link), commenting_user_displayname: @commenting_user_displayname, comment_text: @comment_text, link_my_comments: link_to(t('request_for_comments.index.get_my_comment_requests'), my_request_for_comments_url), link_all_comments: link_to(t('request_for_comments.index.all'), request_for_comments_url) ) +== t('mailers.user_mailer.got_new_comment.body', + receiver_displayname: @receiver_displayname, + link_to_comment: link_to(@rfc_link, @rfc_link), + commenting_user_displayname: @commenting_user_displayname, + comment_text: @comment_text, + link_my_comments: link_to(t('request_for_comments.index.get_my_comment_requests'), my_request_for_comments_url), + link_all_comments: link_to(t('request_for_comments.index.all'), request_for_comments_url) ) diff --git a/app/views/user_mailer/send_thank_you_note.slim b/app/views/user_mailer/send_thank_you_note.slim index fcdcdf8d..6215ed54 100644 --- a/app/views/user_mailer/send_thank_you_note.slim +++ b/app/views/user_mailer/send_thank_you_note.slim @@ -1 +1,5 @@ -== t('mailers.user_mailer.send_thank_you_note.body', receiver_displayname: @receiver_displayname, link_to_comment: link_to(@rfc_link, @rfc_link), author: @author, thank_you_note: @thank_you_note ) +== t('mailers.user_mailer.send_thank_you_note.body', + receiver_displayname: @receiver_displayname, + link_to_comment: link_to(@rfc_link, @rfc_link), + author: @author, + thank_you_note: @thank_you_note ) diff --git a/config/routes.rb b/config/routes.rb index 4b1a4719..21972b23 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -21,7 +21,7 @@ Rails.application.routes.draw do post :set_thank_you_note end end - resources :comments + resources :comments, defaults: { format: :json } get '/my_request_for_comments', as: 'my_request_for_comments', to: 'request_for_comments#get_my_comment_requests' get '/my_rfc_activity', as: 'my_rfc_activity', to: 'request_for_comments#get_rfcs_with_my_comments' diff --git a/db/schema.rb b/db/schema.rb index 82ec19bf..b2fe9d07 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -169,6 +169,7 @@ ActiveRecord::Schema.define(version: 2018_11_29_093207) do t.string "name", limit: 255 t.datetime "created_at" t.datetime "updated_at" + t.string "role", default: "learner", null: false end create_table "file_templates", force: :cascade do |t| From 36650584355de0eef5a9fecb110547c4235b570d Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Fri, 30 Nov 2018 15:44:45 +0100 Subject: [PATCH 06/12] Fix tests and slightly refactor some policies --- app/controllers/concerns/lti.rb | 2 +- app/policies/execution_environment_policy.rb | 4 ++-- app/policies/file_template_policy.rb | 8 ------- app/policies/file_type_policy.rb | 3 --- app/policies/intervention_policy.rb | 4 ++-- app/policies/proxy_exercise_policy.rb | 2 +- app/policies/request_for_comment_policy.rb | 4 ++-- app/policies/subscription_policy.rb | 4 ---- app/policies/tag_policy.rb | 22 +++---------------- config/locales/de.yml | 1 + config/locales/en.yml | 1 + spec/features/authorization_spec.rb | 4 ++-- .../execution_environment_policy_spec.rb | 6 ++--- spec/policies/file_type_policy_spec.rb | 18 +-------------- 14 files changed, 19 insertions(+), 64 deletions(-) diff --git a/app/controllers/concerns/lti.rb b/app/controllers/concerns/lti.rb index 124b32cb..ff5e6ccf 100644 --- a/app/controllers/concerns/lti.rb +++ b/app/controllers/concerns/lti.rb @@ -63,7 +63,7 @@ module Lti else # 'learner' next end - end + end unless provider.roles.blank? result end diff --git a/app/policies/execution_environment_policy.rb b/app/policies/execution_environment_policy.rb index 9e36b558..19da368e 100644 --- a/app/policies/execution_environment_policy.rb +++ b/app/policies/execution_environment_policy.rb @@ -1,9 +1,9 @@ class ExecutionEnvironmentPolicy < AdminOnlyPolicy - [:execute_command?, :shell?, :statistics?].each do |action| + [:execute_command?, :shell?, :statistics?, :show?].each do |action| define_method(action) { admin? || author? } end - [:show?, :index?, :new?].each do |action| + [:index?].each do |action| define_method(action) { admin? || teacher? } end end diff --git a/app/policies/file_template_policy.rb b/app/policies/file_template_policy.rb index a50302d9..67aad7c0 100644 --- a/app/policies/file_template_policy.rb +++ b/app/policies/file_template_policy.rb @@ -1,13 +1,5 @@ class FileTemplatePolicy < AdminOnlyPolicy - def index? - admin? || teacher? - end - - def show? - admin? || teacher? - end - def by_file_type? everyone end diff --git a/app/policies/file_type_policy.rb b/app/policies/file_type_policy.rb index ad6dddd0..57cd4940 100644 --- a/app/policies/file_type_policy.rb +++ b/app/policies/file_type_policy.rb @@ -1,6 +1,3 @@ class FileTypePolicy < AdminOnlyPolicy - [:index?, :show?].each do |action| - define_method(action) { admin? || teacher? } - end end diff --git a/app/policies/intervention_policy.rb b/app/policies/intervention_policy.rb index 373adcb2..b8f482d5 100644 --- a/app/policies/intervention_policy.rb +++ b/app/policies/intervention_policy.rb @@ -11,8 +11,8 @@ class InterventionPolicy < AdminOrAuthorPolicy define_method(action) { admin? || author?} end - [:reload?].each do |action| - define_method(action) { everyone } + def reload? + everyone end class Scope < Scope diff --git a/app/policies/proxy_exercise_policy.rb b/app/policies/proxy_exercise_policy.rb index 3a7bedf7..14981c8e 100644 --- a/app/policies/proxy_exercise_policy.rb +++ b/app/policies/proxy_exercise_policy.rb @@ -20,7 +20,7 @@ class ProxyExercisePolicy < AdminOrAuthorPolicy if @user.admin? @scope.all elsif @user.teacher? - @scope.where('user_id = ?', @user.id) + @scope.where('user_id = ? OR public = TRUE', @user.id) else @scope.none end diff --git a/app/policies/request_for_comment_policy.rb b/app/policies/request_for_comment_policy.rb index 80b95f5f..5ecf488f 100644 --- a/app/policies/request_for_comment_policy.rb +++ b/app/policies/request_for_comment_policy.rb @@ -11,8 +11,8 @@ class RequestForCommentPolicy < ApplicationPolicy everyone end - [:destroy?].each do |action| - define_method(action) { admin? } + def destroy? + admin? end def mark_as_solved? diff --git a/app/policies/subscription_policy.rb b/app/policies/subscription_policy.rb index 289bd5ac..6c759837 100644 --- a/app/policies/subscription_policy.rb +++ b/app/policies/subscription_policy.rb @@ -7,10 +7,6 @@ class SubscriptionPolicy < ApplicationPolicy author? || admin? end - def show_error? - everyone - end - def author? @user == @record.user end diff --git a/app/policies/tag_policy.rb b/app/policies/tag_policy.rb index 7691134a..88bb28c9 100644 --- a/app/policies/tag_policy.rb +++ b/app/policies/tag_policy.rb @@ -1,29 +1,13 @@ -class TagPolicy < AdminOrAuthorPolicy - def batch_update? - admin? - end - - def show? - admin? || teacher? - end - - [:clone?, :destroy?, :edit?, :update?].each do |action| - define_method(action) { admin? || author?} - end - - [:reload?].each do |action| - define_method(action) { everyone } - end +class TagPolicy < AdminOnlyPolicy class Scope < Scope def resolve - if @user.admin? + if @user.admin? || @user.teacher? @scope.all - elsif @user.teacher? - @scope.where('user_id = ? OR public = TRUE', @user.id) else @scope.none end end end + end diff --git a/config/locales/de.yml b/config/locales/de.yml index 4cbd120c..1c001661 100644 --- a/config/locales/de.yml +++ b/config/locales/de.yml @@ -47,6 +47,7 @@ de: consumer: Konsument email: E-Mail name: Name + role: Rolle file: content: Inhalt feedback_message: Feedback-Nachricht diff --git a/config/locales/en.yml b/config/locales/en.yml index 40922a02..c82839d4 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -47,6 +47,7 @@ en: consumer: Consumer email: Email name: Name + role: Role file: content: Content feedback_message: Feedback Message diff --git a/spec/features/authorization_spec.rb b/spec/features/authorization_spec.rb index 7535634f..4ce5432f 100644 --- a/spec/features/authorization_spec.rb +++ b/spec/features/authorization_spec.rb @@ -23,11 +23,11 @@ describe 'Authorization' do let(:user) { FactoryBot.create(:teacher) } before(:each) { allow_any_instance_of(ApplicationController).to receive(:current_user).and_return(user) } - [Consumer, InternalUser].each do |model| + [Consumer, InternalUser, ExecutionEnvironment, FileType].each do |model| expect_forbidden_path(:"new_#{model.model_name.singular}_path") end - [ExecutionEnvironment, Exercise, FileType].each do |model| + [Exercise].each do |model| expect_permitted_path(:"new_#{model.model_name.singular}_path") end end diff --git a/spec/policies/execution_environment_policy_spec.rb b/spec/policies/execution_environment_policy_spec.rb index ef5787b9..50bd57ab 100644 --- a/spec/policies/execution_environment_policy_spec.rb +++ b/spec/policies/execution_environment_policy_spec.rb @@ -5,7 +5,7 @@ describe ExecutionEnvironmentPolicy do let(:execution_environment) { FactoryBot.build(:ruby) } - [:create?, :index?, :new?].each do |action| + [:index?].each do |action| permissions(action) do it 'grants access to admins' do expect(subject).to permit(FactoryBot.build(:admin), execution_environment) @@ -21,7 +21,7 @@ describe ExecutionEnvironmentPolicy do end end - [:execute_command?, :shell?, :statistics?].each do |action| + [:execute_command?, :shell?, :statistics?, :show?].each do |action| permissions(action) do it 'grants access to admins' do expect(subject).to permit(FactoryBot.build(:admin), execution_environment) @@ -39,7 +39,7 @@ describe ExecutionEnvironmentPolicy do end end - [:destroy?, :edit?, :show?, :update?].each do |action| + [:destroy?, :edit?, :update?, :new?, :create?].each do |action| permissions(action) do it 'grants access to admins' do expect(subject).to permit(FactoryBot.build(:admin), execution_environment) diff --git a/spec/policies/file_type_policy_spec.rb b/spec/policies/file_type_policy_spec.rb index 55ec4120..951512d5 100644 --- a/spec/policies/file_type_policy_spec.rb +++ b/spec/policies/file_type_policy_spec.rb @@ -5,23 +5,7 @@ describe FileTypePolicy do let(:file_type) { FactoryBot.build(:dot_rb) } - [:create?, :index?, :new?].each do |action| - permissions(action) do - it 'grants access to admins' do - expect(subject).to permit(FactoryBot.build(:admin), file_type) - end - - it 'grants access to teachers' do - expect(subject).to permit(FactoryBot.build(:teacher), file_type) - end - - it 'does not grant access to external users' do - expect(subject).not_to permit(FactoryBot.build(:external_user), file_type) - end - end - end - - [:destroy?, :edit?, :show?, :update?].each do |action| + [:destroy?, :edit?, :update?, :new?, :create?, :index?, :show?].each do |action| permissions(action) do it 'grants access to admins' do expect(subject).to permit(FactoryBot.build(:admin), file_type) From 6e6e7f47657c8934c2473782c11a0b86ed5fbf0e Mon Sep 17 00:00:00 2001 From: MrSerth Date: Tue, 18 Dec 2018 16:29:30 +0100 Subject: [PATCH 07/12] Update app/views/execution_environments/statistics.html.slim --- app/views/execution_environments/statistics.html.slim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/execution_environments/statistics.html.slim b/app/views/execution_environments/statistics.html.slim index c90b42b6..2b12137d 100644 --- a/app/views/execution_environments/statistics.html.slim +++ b/app/views/execution_environments/statistics.html.slim @@ -14,7 +14,7 @@ h1 = @execution_environment - if wts then average_time = wts["average_time"] else 0 - if wts then stddev_time = wts["stddev_time"] else 0 tr - td = link_to_if policy(exercise).show?, exercise.title, controller: "exercises", action: "statistics", id: exercise.id, 'data-turbolinks' => "false" + td = link_to_if policy(exercise).statistics?, exercise.title, controller: "exercises", action: "statistics", id: exercise.id, 'data-turbolinks' => "false" td = us["users"] td = us["average_score"].to_f.round(4) td = us["maximum_score"].to_f.round(2) From f74c2411413d9c0a5f2102d29657007385c52737 Mon Sep 17 00:00:00 2001 From: MrSerth Date: Tue, 18 Dec 2018 17:05:18 +0100 Subject: [PATCH 08/12] Update app/views/shared/_file.html.slim --- app/views/shared/_file.html.slim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/shared/_file.html.slim b/app/views/shared/_file.html.slim index 01fa812b..a98c92da 100644 --- a/app/views/shared/_file.html.slim +++ b/app/views/shared/_file.html.slim @@ -7,4 +7,4 @@ - if file.teacher_defined_test? = row(label: 'file.feedback_message', value: render_markdown(file.feedback_message), class: 'm-0') = row(label: 'file.weight', value: file.weight) -= row(label: 'file.content', value: file.native_file? ? link_to_if(policy(file.native_file.file.filename).show?, file.native_file.file.filename, file.native_file.url) : code_tag(file.content)) += row(label: 'file.content', value: file.native_file? ? link_to_if(policy(file).show?, file.native_file.file.filename, file.native_file.url) : code_tag(file.content)) From c0608b6f5073d87e6d3be00577e7e324e0847027 Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Wed, 19 Dec 2018 00:58:04 +0100 Subject: [PATCH 09/12] Don't set admin privileges through LTI --- app/controllers/concerns/lti.rb | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/app/controllers/concerns/lti.rb b/app/controllers/concerns/lti.rb index 2e193167..a2374c70 100644 --- a/app/controllers/concerns/lti.rb +++ b/app/controllers/concerns/lti.rb @@ -58,7 +58,8 @@ module Lti provider.roles.each do |role| case role.downcase! when 'administrator' - result = 'admin' + # We don't want anyone to get admin privileges through LTI + result = 'teacher' if result == 'learner' when 'instructor' result = 'teacher' if result == 'learner' else # 'learner' @@ -145,7 +146,11 @@ module Lti def set_current_user @current_user = ExternalUser.find_or_create_by(consumer_id: @consumer.id, external_id: @provider.user_id) - @current_user.update(email: external_user_email(@provider), name: external_user_name(@provider), role: external_user_role(@provider)) + external_role = external_user_role(@provider) + internal_role = @current_user.role + internal_role != 'admin' ? desired_role = external_role : desired_role = internal_role + # Update user with new information but change the role only if he is no admin user + @current_user.update(email: external_user_email(@provider), name: external_user_name(@provider), role: desired_role) end private :set_current_user From 0e8c663039dc2f643fa5cfcc669e1ecd87b61292 Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Wed, 19 Dec 2018 01:29:42 +0100 Subject: [PATCH 10/12] Remove intervention controller, route and policy --- app/controllers/interventions_controller.rb | 55 --------------------- app/policies/intervention_policy.rb | 29 ----------- app/views/interventions/_form.html.slim | 6 --- app/views/interventions/index.html.slim | 15 ------ app/views/interventions/show.html.slim | 4 -- config/routes.rb | 8 --- 6 files changed, 117 deletions(-) delete mode 100644 app/controllers/interventions_controller.rb delete mode 100644 app/policies/intervention_policy.rb delete mode 100644 app/views/interventions/_form.html.slim delete mode 100644 app/views/interventions/index.html.slim delete mode 100644 app/views/interventions/show.html.slim diff --git a/app/controllers/interventions_controller.rb b/app/controllers/interventions_controller.rb deleted file mode 100644 index 27bfe4e0..00000000 --- a/app/controllers/interventions_controller.rb +++ /dev/null @@ -1,55 +0,0 @@ -class InterventionsController < ApplicationController - include CommonBehavior - - before_action :set_intervention, only: MEMBER_ACTIONS - - def authorize! - authorize(@intervention || @interventions) - end - private :authorize! - - def create - #@intervention = Intervention.new(intervention_params) - #authorize! - #create_and_respond(object: @intervention) - end - - def destroy - destroy_and_respond(object: @intervention) - end - - def edit - end - - def intervention_params - params[:intervention].permit(:name) if params[:intervention].present? - end - private :intervention_params - - def index - @interventions = Intervention.all.paginate(page: params[:page]) - authorize! - end - - def new - #@intervention = Intervention.new - #authorize! - end - - def set_intervention - @intervention = Intervention.find(params[:id]) - authorize! - end - private :set_intervention - - def show - end - - def update - update_and_respond(object: @intervention, params: intervention_params) - end - - def to_s - name - end -end diff --git a/app/policies/intervention_policy.rb b/app/policies/intervention_policy.rb deleted file mode 100644 index b8f482d5..00000000 --- a/app/policies/intervention_policy.rb +++ /dev/null @@ -1,29 +0,0 @@ -class InterventionPolicy < AdminOrAuthorPolicy - def batch_update? - admin? - end - - def show? - admin? || teacher? - end - - [:clone?, :destroy?, :edit?, :update?].each do |action| - define_method(action) { admin? || author?} - end - - def reload? - everyone - end - - class Scope < Scope - def resolve - if @user.admin? - @scope.all - elsif @user.teacher? - @scope.where('user_id = ? OR public = TRUE', @user.id) - else - @scope.none - end - end - end -end diff --git a/app/views/interventions/_form.html.slim b/app/views/interventions/_form.html.slim deleted file mode 100644 index 6ffe7397..00000000 --- a/app/views/interventions/_form.html.slim +++ /dev/null @@ -1,6 +0,0 @@ -= form_for(@intervention) do |f| - = render('shared/form_errors', object: @intervention) - .form-group - = f.label(:name) - = f.text_field(:name, class: 'form-control', required: true) - .actions = render('shared/submit_button', f: f, object: @intervention) diff --git a/app/views/interventions/index.html.slim b/app/views/interventions/index.html.slim deleted file mode 100644 index 92763567..00000000 --- a/app/views/interventions/index.html.slim +++ /dev/null @@ -1,15 +0,0 @@ -h1 = Intervention.model_name.human(count: 2) - -.table-responsive - table.table - thead - tr - th = t('activerecord.attributes.intervention.name') - th = t('shared.actions') - tbody - - @interventions.each do |intervention| - tr - td = link_to_if policy(intervention).show?, intervention.name - td = link_to(t('shared.show'), intervention) if policy(intervention).show? - -= render('shared/pagination', collection: @interventions) diff --git a/app/views/interventions/show.html.slim b/app/views/interventions/show.html.slim deleted file mode 100644 index f9202240..00000000 --- a/app/views/interventions/show.html.slim +++ /dev/null @@ -1,4 +0,0 @@ -h1 - = @intervention.name - -= row(label: 'intervention.name', value: @intervention.name) diff --git a/config/routes.rb b/config/routes.rb index d8a62acc..ed453762 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -115,14 +115,6 @@ Rails.application.routes.draw do end end - resources :interventions do - member do - post :clone - get :reload - post :submit - end - end - resources :external_users, only: [:index, :show], concerns: :statistics do resources :exercises, concerns: :statistics member do From 71cd9e34886c66d257e95d398e27ba2139ead810 Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Wed, 19 Dec 2018 01:35:00 +0100 Subject: [PATCH 11/12] Remove user_exercise_feedback index and show routes / views --- .../user_exercise_feedbacks_controller.rb | 10 ------- .../user_exercise_feedbacks/index.html.slim | 26 ------------------- .../user_exercise_feedbacks/show.html.slim | 7 ----- config/routes.rb | 7 +---- 4 files changed, 1 insertion(+), 49 deletions(-) delete mode 100644 app/views/user_exercise_feedbacks/index.html.slim delete mode 100644 app/views/user_exercise_feedbacks/show.html.slim diff --git a/app/controllers/user_exercise_feedbacks_controller.rb b/app/controllers/user_exercise_feedbacks_controller.rb index a3c71abc..ed0e77fd 100644 --- a/app/controllers/user_exercise_feedbacks_controller.rb +++ b/app/controllers/user_exercise_feedbacks_controller.rb @@ -19,16 +19,6 @@ class UserExerciseFeedbacksController < ApplicationController [4,t('user_exercise_feedback.estimated_time_more_30')]] end - def index - @search = UserExerciseFeedback.all.search params[:q] - @uefs = @search.result.includes(:execution_environment).order(:id).paginate(page: params[:page]) - authorize! - end - - def show - authorize! - end - def create @exercise = Exercise.find(uef_params[:exercise_id]) rfc = RequestForComment.unsolved.where(exercise_id: @exercise.id, user_id: current_user.id).first diff --git a/app/views/user_exercise_feedbacks/index.html.slim b/app/views/user_exercise_feedbacks/index.html.slim deleted file mode 100644 index 85c7f6dc..00000000 --- a/app/views/user_exercise_feedbacks/index.html.slim +++ /dev/null @@ -1,26 +0,0 @@ -h1 = UserExerciseFeedback.model_name.human(count: 2) - -= render(layout: 'shared/form_filters') do |f| - .form-group - = f.label(:execution_environment_id_eq, t('activerecord.attributes.exercise.execution_environment'), class: 'sr-only') - = f.collection_select(:execution_environment_id_eq, ExecutionEnvironment.with_exercises, :id, :name, class: 'form-control', prompt: t('activerecord.attributes.exercise.execution_environment')) - .form-group - = f.label(:exercise_title_cont, t('activerecord.attributes.request_for_comments.exercise'), class: 'sr-only') - = f.search_field(:exercise_title_cont, class: 'form-control', placeholder: t('activerecord.attributes.request_for_comments.exercise')) - -.table-responsive - table.table - thead - tr - th = t('activerecord.attributes.user_exercise_feedback.user') - th = t('activerecord.attributes.user_exercise_feedback.exercise') - th colspan=2 = t('shared.actions') - tbody - - @uefs.each do |uef| - tr - td = link_to_if(policy(uef.user).show?, uef.user.displayname) - td = link_to_if(policy(uef.exercise).show?, uef.exercise.title, uef.exercise) - td = link_to(t('shared.show'), uef) if policy(uef).show? - td = link_to(t('shared.destroy'), uef, data: {confirm: t('shared.confirm_destroy')}, method: :delete) if policy(uef).destroy? - -= render('shared/pagination', collection: @uefs) diff --git a/app/views/user_exercise_feedbacks/show.html.slim b/app/views/user_exercise_feedbacks/show.html.slim deleted file mode 100644 index 69b945f6..00000000 --- a/app/views/user_exercise_feedbacks/show.html.slim +++ /dev/null @@ -1,7 +0,0 @@ -h2 = @uef - -= row(label: 'activerecord.attributes.user_exercise_feedback.exercise', value: link_to_if(policy(@uef.exercise).show?, @uef.exercise.title, @uef.exercise)) -= row(label: 'user_exercise_feedback.user', value: link_to_if(policy(@uef.user).show?, @uef.user)) -= row(label: 'activerecord.attributes.user_exercise_feedback.feedback_text', value: @uef.feedback_text) -= row(label: 'user_exercise_feedback.difficulty', value: @uef.difficulty) -= row(label: 'user_exercise_feedback.working_time', value: @uef.user_estimated_worktime) diff --git a/config/routes.rb b/config/routes.rb index ed453762..4c12d4cb 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -108,12 +108,7 @@ Rails.application.routes.draw do end end - resources :user_exercise_feedbacks do - member do - get :reload - post :submit - end - end + resources :user_exercise_feedbacks, except: [:show, :index] resources :external_users, only: [:index, :show], concerns: :statistics do resources :exercises, concerns: :statistics From 169382469038f88275634303d63cc2006f811c31 Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Wed, 19 Dec 2018 01:42:53 +0100 Subject: [PATCH 12/12] Clean routes and remove invalid ones --- config/routes.rb | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/config/routes.rb b/config/routes.rb index 4c12d4cb..ed0ea08f 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -16,9 +16,9 @@ Rails.application.routes.draw do resources :code_harbor_links resources :request_for_comments do member do - get :mark_as_solved - post :create_comment_exercise - post :set_thank_you_note + get :mark_as_solved, defaults: { format: :json } + post :create_comment_exercise, defaults: { format: :json } + post :set_thank_you_note, defaults: { format: :json } end end resources :comments, defaults: { format: :json } @@ -96,17 +96,10 @@ Rails.application.routes.draw do member do post :clone get :reload - post :submit end end - resources :tags do - member do - post :clone - get :reload - post :submit - end - end + resources :tags resources :user_exercise_feedbacks, except: [:show, :index]