From 6e03939c10b3549a179f73e9cc2c748e063b7b6b Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Thu, 14 Feb 2019 18:09:21 +0100 Subject: [PATCH] Add StudyGroup to submissions and show it for submissions and RfCs Also take care of deleting a StudyGroup for existing submissions --- app/controllers/concerns/lti.rb | 1 + .../concerns/submission_parameters.rb | 3 ++- .../request_for_comments_controller.rb | 11 ++--------- app/models/study_group.rb | 4 ++-- app/policies/application_policy.rb | 16 ++++++++++++++++ app/policies/submission_policy.rb | 10 ++-------- app/views/request_for_comments/show.html.slim | 3 +++ app/views/submissions/show.html.slim | 1 + 8 files changed, 29 insertions(+), 20 deletions(-) diff --git a/app/controllers/concerns/lti.rb b/app/controllers/concerns/lti.rb index b378dcc3..3157ccf4 100644 --- a/app/controllers/concerns/lti.rb +++ b/app/controllers/concerns/lti.rb @@ -168,6 +168,7 @@ module Lti end group.users |= [@current_user] # add current user if not already member of the group group.save + session[:study_group_id] = group.id end def set_embedding_options diff --git a/app/controllers/concerns/submission_parameters.rb b/app/controllers/concerns/submission_parameters.rb index fa5c848b..effa8ddc 100644 --- a/app/controllers/concerns/submission_parameters.rb +++ b/app/controllers/concerns/submission_parameters.rb @@ -16,7 +16,8 @@ module SubmissionParameters current_user_id = current_user.id current_user_class_name = current_user.class.name end - submission_params = params[:submission].present? ? params[:submission].permit(:cause, :exercise_id, files_attributes: file_attributes).merge(user_id: current_user_id, user_type: current_user_class_name) : {} + # The study_group_id might not be present in the session (e.g. for internal users), resulting in session[:study_group_id] = nil which is intended. + submission_params = params[:submission].present? ? params[:submission].permit(:cause, :exercise_id, files_attributes: file_attributes).merge(user_id: current_user_id, user_type: current_user_class_name, study_group_id: session[:study_group_id]) : {} reject_illegal_file_attributes!(submission_params) submission_params end diff --git a/app/controllers/request_for_comments_controller.rb b/app/controllers/request_for_comments_controller.rb index 39f1ebb8..64301965 100644 --- a/app/controllers/request_for_comments_controller.rb +++ b/app/controllers/request_for_comments_controller.rb @@ -83,17 +83,10 @@ class RequestForCommentsController < ApplicationController authorize! end - # GET /request_for_comments/new - def new - @request_for_comment = RequestForComment.new - authorize! - end - # GET /request_for_comments/1/edit def edit end - # POST /request_for_comments # POST /request_for_comments.json def create # Consider all requests as JSON @@ -149,8 +142,8 @@ class RequestForCommentsController < ApplicationController # Never trust parameters from the scary internet, only allow the white list through. def request_for_comment_params - # we are using the current_user.id here, since internal users are not able to create comments. The external_user.id is a primary key and does not require the consumer_id to be unique. - params.require(:request_for_comment).permit(:exercise_id, :file_id, :question, :requested_at, :solved, :submission_id).merge(user_id: current_user.id, user_type: current_user.class.name) + # The study_group_id might not be present in the session (e.g. for internal users), resulting in session[:study_group_id] = nil which is intended. + params.require(:request_for_comment).permit(:exercise_id, :file_id, :question, :requested_at, :solved, :submission_id).merge(user_id: current_user.id, user_type: current_user.class.name, study_group_id: session[:study_group_id]) end def comment_params diff --git a/app/models/study_group.rb b/app/models/study_group.rb index 13167047..16633165 100644 --- a/app/models/study_group.rb +++ b/app/models/study_group.rb @@ -1,12 +1,12 @@ # frozen_string_literal: true class StudyGroup < ApplicationRecord - has_many :study_group_memberships + has_many :study_group_memberships, dependent: :destroy # Use `ExternalUser` as `source_type` for now. # Using `User` will lead ActiveRecord to access the inexistent table `users`. # Issue created: https://github.com/rails/rails/issues/34531 has_many :users, through: :study_group_memberships, source_type: 'ExternalUser' - has_many :submissions + has_many :submissions, dependent: :nullify belongs_to :consumer def to_s diff --git a/app/policies/application_policy.rb b/app/policies/application_policy.rb index 9d8b9cd1..4e8ae44b 100644 --- a/app/policies/application_policy.rb +++ b/app/policies/application_policy.rb @@ -25,6 +25,22 @@ class ApplicationPolicy end private :no_one + def everyone_in_study_group + study_group = @record.study_group + return false if study_group.blank? + + users_in_same_study_group = study_group.users + return false if users_in_same_study_group.blank? + + 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 + def initialize(user, record) @user = user @record = record diff --git a/app/policies/submission_policy.rb b/app/policies/submission_policy.rb index fe2137c3..0d891f3e 100644 --- a/app/policies/submission_policy.rb +++ b/app/policies/submission_policy.rb @@ -12,14 +12,8 @@ class SubmissionPolicy < ApplicationPolicy 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 + def show_study_group? + admin? || teacher_in_study_group end - private :teacher_in_study_group end diff --git a/app/views/request_for_comments/show.html.slim b/app/views/request_for_comments/show.html.slim index 479dd29d..6b49f216 100644 --- a/app/views/request_for_comments/show.html.slim +++ b/app/views/request_for_comments/show.html.slim @@ -9,6 +9,9 @@ - testruns = Testrun.where(:submission_id => @request_for_comment.submission) = link_to_if(policy(user).show?, user.displayname, user) | | #{@request_for_comment.created_at.localtime} + - if @request_for_comment.submission.study_group.present? && policy(@request_for_comment.submission).show_study_group? + = ' | ' + = link_to_if(policy(@request_for_comment.submission.study_group).show?, @request_for_comment.submission.study_group, @request_for_comment.submission.study_group) .rfc .description h5 diff --git a/app/views/submissions/show.html.slim b/app/views/submissions/show.html.slim index 9a7ab69b..92953f25 100644 --- a/app/views/submissions/show.html.slim +++ b/app/views/submissions/show.html.slim @@ -9,6 +9,7 @@ h1 = @submission = 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.study_group', value: link_to_if(policy(@submission.study_group).show?, @submission.study_group, @submission.study_group)) = row(label: 'submission.cause', value: t("submissions.causes.#{@submission.cause}")) = row(label: 'submission.score', value: @submission.score)