Refactor various ruby files

* Insights based on brakeman report
This commit is contained in:
Sebastian Serth
2022-08-18 15:06:36 +02:00
parent 1560f6b316
commit 145c4aa8d5
35 changed files with 113 additions and 107 deletions

View File

@ -41,7 +41,7 @@ module CodeOcean
end
def destroy
@file = CodeOcean::File.find(params[:id])
@file = CodeOcean::File.find_by(id: params[:id])
authorize!
destroy_and_respond(object: @file, path: @file.context)
end

View File

@ -43,8 +43,7 @@ class CodeharborLinksController < ApplicationController
end
def set_codeharbor_link
@codeharbor_link = CodeharborLink.find(params[:id])
@codeharbor_link.user = current_user
@codeharbor_link = CodeharborLink.find_by(id: params[:id])
authorize!
end

View File

@ -10,7 +10,7 @@ class CommentsController < ApplicationController
# GET /comments.json
def index
file = CodeOcean::File.find(params[:file_id])
file = CodeOcean::File.find_by(id: params[:file_id])
# there might be no submission yet, so dont use find
submission = Submission.find_by(id: file.context_id)
if submission
@ -38,7 +38,7 @@ class CommentsController < ApplicationController
if @comment.save
if comment_params[:request_id]
request_for_comment = RequestForComment.find(comment_params[:request_id])
request_for_comment = RequestForComment.find_by(id: comment_params[:request_id])
send_mail_to_author @comment, request_for_comment
send_mail_to_subscribers @comment, request_for_comment
end
@ -71,7 +71,7 @@ class CommentsController < ApplicationController
# Use callbacks to share common setup or constraints between actions.
def set_comment
@comment = Comment.find(params[:id])
@comment = Comment.find_by(id: params[:id])
end
def comment_params_for_update

View File

@ -90,11 +90,11 @@ class CommunitySolutionsController < ApplicationController
# Use callbacks to share common setup or constraints between actions.
def set_community_solution
@community_solution = CommunitySolution.find(params[:id])
@community_solution = CommunitySolution.find_by(id: params[:id])
end
def set_community_solution_lock
@community_solution_lock = CommunitySolutionLock.find(params[:lock_id])
@community_solution_lock = CommunitySolutionLock.find_by(id: params[:lock_id])
end
def set_exercise_and_submission

View File

@ -129,12 +129,7 @@ module RedirectBehavior
lti_parameters_id: session[:lti_parameters_id]
)
lti_parameter = LtiParameter.where(external_users_id: @submission.user_id,
exercises_id: @submission.exercise_id).last
path = lti_return_path(submission_id: @submission.id,
url: consumer_return_url(build_tool_provider(consumer: @submission.user.consumer,
parameters: lti_parameter&.lti_parameters)))
path = lti_return_path(submission_id: @submission.id)
clear_lti_session_data(@submission.exercise_id, @submission.user_id)
respond_to do |format|
format.html { redirect_to(path) }

View File

@ -38,7 +38,7 @@ class ConsumersController < ApplicationController
end
def set_consumer
@consumer = Consumer.find(params[:id])
@consumer = Consumer.find_by(id: params[:id])
authorize!
end
private :set_consumer

View File

@ -86,7 +86,7 @@ class ErrorTemplateAttributesController < ApplicationController
# Use callbacks to share common setup or constraints between actions.
def set_error_template_attribute
@error_template_attribute = ErrorTemplateAttribute.find(params[:id])
@error_template_attribute = ErrorTemplateAttribute.find_by(id: params[:id])
end
def error_template_attribute_params

View File

@ -77,7 +77,7 @@ class ErrorTemplatesController < ApplicationController
def add_attribute
authorize!
@error_template.error_template_attributes << ErrorTemplateAttribute.find(params['error_template_attribute_id'])
@error_template.error_template_attributes << ErrorTemplateAttribute.find_by(id: params[:error_template_attribute_id])
respond_to do |format|
format.html { redirect_to @error_template }
format.json { head :no_content }
@ -86,7 +86,7 @@ class ErrorTemplatesController < ApplicationController
def remove_attribute
authorize!
@error_template.error_template_attributes.delete(ErrorTemplateAttribute.find(params['error_template_attribute_id']))
@error_template.error_template_attributes.delete(ErrorTemplateAttribute.find_by(id: params[:error_template_attribute_id]))
respond_to do |format|
format.html { redirect_to @error_template }
format.json { head :no_content }
@ -97,7 +97,7 @@ class ErrorTemplatesController < ApplicationController
# Use callbacks to share common setup or constraints between actions.
def set_error_template
@error_template = ErrorTemplate.find(params[:id])
@error_template = ErrorTemplate.find_by(id: params[:id])
end
def error_template_params

View File

@ -141,7 +141,7 @@ class ExecutionEnvironmentsController < ApplicationController
private :set_docker_images
def set_execution_environment
@execution_environment = ExecutionEnvironment.find(params[:id])
@execution_environment = ExecutionEnvironment.find_by(id: params[:id])
authorize!
end
private :set_execution_environment
@ -158,7 +158,7 @@ class ExecutionEnvironmentsController < ApplicationController
def show
if @execution_environment.testing_framework?
@testing_framework_adapter = Kernel.const_get(@execution_environment.testing_framework)
@testing_framework_adapter = TestingFrameworkAdapter.descendants.find {|klass| klass.name == @execution_environment.testing_framework }
end
end

View File

@ -41,7 +41,7 @@ class ExerciseCollectionsController < ApplicationController
private
def set_exercise_collection
@exercise_collection = ExerciseCollection.find(params[:id])
@exercise_collection = ExerciseCollection.find_by(id: params[:id])
authorize!
end

View File

@ -25,6 +25,7 @@ class ExercisesController < ApplicationController
def authorize!
authorize(@exercise || @exercises)
end
private :authorize!
def max_intervention_count_per_day
@ -39,7 +40,7 @@ class ExercisesController < ApplicationController
@exercises = Exercise.all
authorize!
@exercises = params[:exercises].values.map do |exercise_params|
exercise = Exercise.find(exercise_params.delete(:id))
exercise = Exercise.find_by(id: exercise_params.delete(:id))
exercise.update(exercise_params)
exercise
end
@ -50,7 +51,7 @@ class ExercisesController < ApplicationController
exercise = @exercise.duplicate(public: false, token: nil, user: current_user)
exercise.send(:generate_token)
if exercise.save
redirect_to(exercise, notice: t('shared.object_cloned', model: Exercise.model_name.human))
redirect_to(exercise_path(exercise), notice: t('shared.object_cloned', model: Exercise.model_name.human))
else
flash[:danger] = t('shared.message_failure')
redirect_to(@exercise)
@ -66,6 +67,7 @@ class ExercisesController < ApplicationController
end
subpaths.flatten.uniq
end
private :collect_paths
def create
@ -192,12 +194,14 @@ class ExercisesController < ApplicationController
api_key = authorization_header&.split(' ')&.second
user_by_codeharbor_token(api_key)
end
private :user_from_api_key
def user_by_codeharbor_token(api_key)
link = CodeharborLink.find_by(api_key: api_key)
link&.user
end
private :user_by_codeharbor_token
def exercise_params
@ -225,6 +229,7 @@ class ExercisesController < ApplicationController
)
end
end
private :exercise_params
def handle_file_uploads
@ -241,6 +246,7 @@ class ExercisesController < ApplicationController
end
end
end
private :handle_file_uploads
def handle_exercise_tips
@ -258,6 +264,7 @@ class ExercisesController < ApplicationController
redirect_to(edit_exercise_path(@exercise))
end
end
private :handle_exercise_tips
def update_exercise_tips(exercise_tips, parent_exercise_tip_id, rank)
@ -283,6 +290,7 @@ class ExercisesController < ApplicationController
end
result
end
private :update_exercise_tips
def implement
@ -348,6 +356,7 @@ class ExercisesController < ApplicationController
@course_token = '702cbd2a-c84c-4b37-923a-692d7d1532d0'
end
end
private :set_course_token
def set_available_tips
@ -374,13 +383,14 @@ class ExercisesController < ApplicationController
# Return an array with top-level tips
@tips = nested_tips.values.select {|tip| tip.parent_exercise_tip_id.nil? }
end
private :set_available_tips
def working_times
working_time_accumulated = @exercise.accumulated_working_time_for_only(current_user)
working_time_75_percentile = @exercise.get_quantiles([0.75]).first
render(json: {working_time_75_percentile: working_time_75_percentile,
working_time_accumulated: working_time_accumulated})
working_time_accumulated: working_time_accumulated})
end
def intervention
@ -401,8 +411,9 @@ working_time_accumulated: working_time_accumulated})
search_text = params[:search_text]
search = Search.new(user: current_user, exercise: @exercise, search: search_text)
begin search.save
render(json: {success: 'true'})
begin
search.save
render(json: {success: 'true'})
rescue StandardError
render(json: {success: 'false', error: "could not save search: #{$ERROR_INFO}"})
end
@ -424,25 +435,29 @@ working_time_accumulated: working_time_accumulated})
def set_execution_environments
@execution_environments = ExecutionEnvironment.all.order(:name)
end
private :set_execution_environments
def set_exercise_and_authorize
@exercise = Exercise.find(params[:id])
@exercise = Exercise.find_by(id: params[:id])
authorize!
end
private :set_exercise_and_authorize
def set_external_user_and_authorize
if params[:external_user_id]
@external_user = ExternalUser.find(params[:external_user_id])
@external_user = ExternalUser.find_by(id: params[:external_user_id])
authorize!
end
end
private :set_external_user_and_authorize
def set_file_types
@file_types = FileType.all.order(:name)
end
private :set_file_types
def collect_set_and_unset_exercise_tags
@ -452,9 +467,10 @@ working_time_accumulated: working_time_accumulated})
checked_tags = checked_exercise_tags.collect(&:tag).to_set
unchecked_tags = Tag.all.to_set.subtract checked_tags
@exercise_tags = checked_exercise_tags + unchecked_tags.collect do |tag|
ExerciseTag.new(exercise: @exercise, tag: tag)
end
ExerciseTag.new(exercise: @exercise, tag: tag)
end
end
private :collect_set_and_unset_exercise_tags
def show
@ -468,20 +484,24 @@ working_time_accumulated: working_time_accumulated})
def statistics
# Show general statistic page for specific exercise
user_statistics = {'InternalUser' => {}, 'ExternalUser' => {}}
additional_filter = if policy(@exercise).detailed_statistics?
''
elsif !policy(@exercise).detailed_statistics? && current_user.study_groups.count.positive?
"AND study_group_id IN (#{current_user.study_groups.pluck(:id).join(', ')}) AND cause = 'submit'"
else
# e.g. internal user without any study groups, show no submissions
'AND FALSE'
end
query = "SELECT user_id, user_type, MAX(score) AS maximum_score, COUNT(id) AS runs
FROM submissions WHERE exercise_id = #{@exercise.id} #{additional_filter}
GROUP BY user_id, user_type;"
ApplicationRecord.connection.execute(query).each do |tuple|
query = Submission.select('user_id, user_type, MAX(score) AS maximum_score, COUNT(id) AS runs')
.where(exercise_id: @exercise.id)
.group('user_id, user_type')
query = if policy(@exercise).detailed_statistics?
query
elsif !policy(@exercise).detailed_statistics? && current_user.study_groups.count.positive?
query.where(study_groups: current_user.study_groups.pluck(:id), cause: 'submit')
else
# e.g. internal user without any study groups, show no submissions
query.where('false')
end
query.each do |tuple|
user_statistics[tuple['user_type']][tuple['user_id'].to_i] = tuple
end
render locals: {
user_statistics: user_statistics,
}
@ -554,6 +574,7 @@ working_time_accumulated: working_time_accumulated})
end
end
end
private :transmit_lti_score
def update

View File

@ -15,7 +15,7 @@ class ExternalUsersController < ApplicationController
end
def show
@user = ExternalUser.find(params[:id])
@user = ExternalUser.find_by(id: params[:id])
authorize!
end
@ -58,7 +58,7 @@ class ExternalUsersController < ApplicationController
end
def statistics
@user = ExternalUser.find(params[:id])
@user = ExternalUser.find_by(id: params[:id])
authorize!
statistics = {}
@ -73,7 +73,7 @@ class ExternalUsersController < ApplicationController
end
def tag_statistics
@user = ExternalUser.find(params[:id])
@user = ExternalUser.find_by(id: params[:id])
authorize!
statistics = []

View File

@ -87,7 +87,7 @@ class FileTemplatesController < ApplicationController
# Use callbacks to share common setup or constraints between actions.
def set_file_template
@file_template = FileTemplate.find(params[:id])
@file_template = FileTemplate.find_by(id: params[:id])
end
def file_template_params

View File

@ -51,7 +51,7 @@ class FileTypesController < ApplicationController
private :set_editor_modes
def set_file_type
@file_type = FileType.find(params[:id])
@file_type = FileType.find_by(id: params[:id])
authorize!
end
private :set_file_type

View File

@ -121,7 +121,7 @@ class InternalUsersController < ApplicationController
private :set_up_password
def set_user
@user = InternalUser.find(params[:id])
@user = InternalUser.find_by(id: params[:id])
authorize!
end
private :set_user

View File

@ -15,7 +15,7 @@ class ProxyExercisesController < ApplicationController
user: current_user)
proxy_exercise.send(:generate_token)
if proxy_exercise.save
redirect_to(proxy_exercise, notice: t('shared.object_cloned', model: ProxyExercise.model_name.human))
redirect_to(proxy_exercise_path(proxy_exercise), notice: t('shared.object_cloned', model: ProxyExercise.model_name.human))
else
flash[:danger] = t('shared.message_failure')
redirect_to(@proxy_exercise)
@ -24,7 +24,7 @@ class ProxyExercisesController < ApplicationController
def create
myparams = proxy_exercise_params
myparams[:exercises] = Exercise.find(myparams[:exercise_ids].compact_blank)
myparams[:exercises] = Exercise.find_by(id: myparams[:exercise_ids].compact_blank)
@proxy_exercise = ProxyExercise.new(myparams)
authorize!
@ -63,7 +63,7 @@ class ProxyExercisesController < ApplicationController
end
def set_exercise_and_authorize
@proxy_exercise = ProxyExercise.find(params[:id])
@proxy_exercise = ProxyExercise.find_by(id: params[:id])
authorize!
end
private :set_exercise_and_authorize
@ -78,7 +78,7 @@ class ProxyExercisesController < ApplicationController
def update
myparams = proxy_exercise_params
myparams[:exercises] = Exercise.find(myparams[:exercise_ids].compact_blank)
myparams[:exercises] = Exercise.find_by(id: myparams[:exercise_ids].compact_blank)
update_and_respond(object: @proxy_exercise, params: myparams)
end
end

View File

@ -58,7 +58,7 @@ class RequestForCommentsController < ApplicationController
# GET /exercises/:id/request_for_comments
def rfcs_for_exercise
exercise = Exercise.find(params[:exercise_id])
exercise = Exercise.find_by(id: params[:exercise_id])
@search = RequestForComment
.with_last_activity
.where(exercise_id: exercise.id)
@ -141,7 +141,7 @@ class RequestForCommentsController < ApplicationController
# Use callbacks to share common setup or constraints between actions.
def set_request_for_comment
@request_for_comment = RequestForComment.find(params[:id])
@request_for_comment = RequestForComment.find_by(id: params[:id])
end
def request_for_comment_params

View File

@ -24,7 +24,7 @@ class SessionsController < ApplicationController
store_lti_session_data(consumer: @consumer, parameters: params)
store_nonce(params[:oauth_nonce])
if params[:custom_redirect_target]
redirect_to(params[:custom_redirect_target])
redirect_to(URI.parse(params[:custom_redirect_target].to_s).path)
else
redirect_to(implement_exercise_path(@exercise),
notice: t("sessions.create_through_lti.session_#{lti_outcome_service?(@exercise.id, @current_user.id) ? 'with' : 'without'}_outcome",
@ -42,7 +42,11 @@ class SessionsController < ApplicationController
end
def destroy_through_lti
@submission = Submission.find(params[:submission_id])
@submission = Submission.find_by(id: params[:submission_id])
authorize(@submission, :show?)
lti_parameter = LtiParameter.where(external_users_id: @submission.user_id, exercises_id: @submission.exercise_id).last
@url = consumer_return_url(build_tool_provider(consumer: @submission.user.consumer, parameters: lti_parameter&.lti_parameters))
clear_lti_session_data(@submission.exercise_id, @submission.user_id)
end

View File

@ -23,7 +23,7 @@ class StudyGroupsController < ApplicationController
def update
myparams = study_group_params
myparams[:external_users] =
StudyGroupMembership.find(myparams[:study_group_membership_ids].compact_blank).map(&:user)
StudyGroupMembership.find_by(id: myparams[:study_group_membership_ids].compact_blank).map(&:user)
myparams.delete(:study_group_membership_ids)
update_and_respond(object: @study_group, params: myparams)
end
@ -38,7 +38,7 @@ class StudyGroupsController < ApplicationController
private :study_group_params
def set_group
@study_group = StudyGroup.find(params[:id])
@study_group = StudyGroup.find_by(id: params[:id])
authorize!
end
private :set_group

View File

@ -373,7 +373,7 @@ class SubmissionsController < ApplicationController
end
def set_submission
@submission = Submission.find(params[:id])
@submission = Submission.find_by(id: params[:id])
authorize!
end

View File

@ -22,7 +22,7 @@ class SubscriptionsController < ApplicationController
# DELETE /subscriptions/1
# DELETE /subscriptions/1.json
def destroy
@subscription = Subscription.find(params[:id])
@subscription = Subscription.find_by(id: params[:id])
rescue StandardError
skip_authorization
respond_to do |format|
@ -47,7 +47,7 @@ class SubscriptionsController < ApplicationController
end
def set_subscription
@subscription = Subscription.find(params[:id])
@subscription = Subscription.find_by(id: params[:id])
authorize!
end
private :set_subscription

View File

@ -38,7 +38,7 @@ class TagsController < ApplicationController
end
def set_tag
@tag = Tag.find(params[:id])
@tag = Tag.find_by(id: params[:id])
authorize!
end
private :set_tag

View File

@ -44,7 +44,7 @@ class TipsController < ApplicationController
end
def set_tip
@tip = Tip.find(params[:id])
@tip = Tip.find_by(id: params[:id])
authorize!
end
private :set_tip

View File

@ -25,7 +25,7 @@ class UserExerciseFeedbacksController < ApplicationController
def create
Sentry.set_extras(params: uef_params)
@exercise = Exercise.find(uef_params[:exercise_id])
@exercise = Exercise.find_by(id: uef_params[:exercise_id])
rfc = RequestForComment.unsolved.where(exercise_id: @exercise.id, user_id: current_user.id).first
submission = begin
current_user.submissions.where(exercise_id: @exercise.id).order('created_at DESC').first
@ -67,7 +67,7 @@ class UserExerciseFeedbacksController < ApplicationController
else
params[:user_exercise_feedback][:exercise_id]
end
@exercise = Exercise.find(exercise_id)
@exercise = Exercise.find_by(id: exercise_id)
@uef = UserExerciseFeedback.find_or_initialize_by(user: current_user, exercise: @exercise)
authorize!
end
@ -105,7 +105,7 @@ class UserExerciseFeedbacksController < ApplicationController
end
def set_user_exercise_feedback
@uef = UserExerciseFeedback.find(params[:id])
@uef = UserExerciseFeedback.find_by(id: params[:id])
@exercise = @uef.exercise
end