Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Programming Question Audit Trail #7595

Open
wants to merge 24 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
8f2652b
feat(programming-audit-trail): modify db
syoopie Sep 25, 2024
03b021a
feat(programming-audit-trail): preserve old questions and tests
syoopie Oct 22, 2024
90f4fc9
feat(custom-slider): add custom slider
syoopie Oct 2, 2024
f7757d9
feat(accordion): expand accordion functionality
syoopie Oct 3, 2024
fbc1a33
refactor(all-attempts-display): update accordion
syoopie Oct 3, 2024
fcc4891
feat(question-attempts): add title card
syoopie Oct 22, 2024
a1951eb
refactor(statistics): change naming of types
syoopie Oct 3, 2024
ce47b56
refactor(all-attempts-display): use new slider
syoopie Oct 3, 2024
eef5da2
refactor(all-attempts-display): change created_at to submitted_at
syoopie Oct 3, 2024
e5d1afa
perf(all-attempts): improve answer retrieval
syoopie Oct 3, 2024
76a2afd
feat(all-attempts-display): add more programming question details
syoopie Oct 15, 2024
c5da986
refactor(attempts-stats): refactor backend
syoopie Oct 22, 2024
9e408e8
refactor(statistics): use seperate API for latest answer
syoopie Oct 22, 2024
a1b387f
feat(statistics-marks-table): add card to dialog
syoopie Oct 22, 2024
dd0c028
refactor(statistics): change naming, add limit setting to FE
syoopie Oct 9, 2024
9d2fd49
feat(statistics): add BE support for multiple tests / questions
syoopie Oct 9, 2024
e36e247
feat(statistics): multiple regrading view for attempts
syoopie Oct 15, 2024
1e43e48
feat(regrading): only regrade current_answer on question edit
syoopie Oct 15, 2024
063868b
test(stats-answer-controller): rename function calls
syoopie Oct 16, 2024
9e7a7e4
test(programming-controller): fix tests
syoopie Oct 18, 2024
ef6c7ce
test(programming-management): fix tests
syoopie Oct 21, 2024
230adba
test(programming-management): fix template package spec
syoopie Oct 21, 2024
3e31863
fix(attempt-count-table): fix error when attempt does not exist
syoopie Oct 21, 2024
2d65d34
test(test-case-view): fix FE test
syoopie Oct 21, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ def edit

def update
result = @programming_question.class.transaction do
duplicate_and_replace_programming_question
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you're rearranging items, please squash the commit with the respective commit?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@question_assessment.skill_ids = programming_question_params[:question_assessment].
try(:[], :skill_ids)
@programming_question.assign_attributes(programming_question_params.
Expand All @@ -64,7 +65,7 @@ def update
end

if result
render_success_json false
render_success_json true
else
render_failure_json
end
Expand Down Expand Up @@ -147,6 +148,48 @@ def destroy

private

def duplicate_and_replace_programming_question
duplicated_programming_question = duplicate_programming_question
duplicate_associations(duplicated_programming_question)
save_duplicated_question(duplicated_programming_question)
link_to_original_question(duplicated_programming_question)
@programming_question = duplicated_programming_question
end

def duplicate_programming_question
duplicated_programming_question = @programming_question.dup
# Unique field that needs to be reset
duplicated_programming_question.import_job_id = nil
duplicated_programming_question.question = @programming_question.question
duplicated_programming_question
end

def duplicate_associations(duplicated_programming_question)
duplicated_programming_question.template_files = @programming_question.template_files.map do |template_file|
duplicated_template_file = template_file.dup
duplicated_template_file.question = duplicated_programming_question
duplicated_template_file
end

duplicated_programming_question.test_cases = @programming_question.test_cases.map do |test_case|
duplicated_test_case = test_case.dup
duplicated_test_case.question = duplicated_programming_question
duplicated_test_case
end
end

def save_duplicated_question(duplicated_programming_question)
duplicated_programming_question.save!(validate: false)
duplicated_programming_question.template_files.each(&:save!)
duplicated_programming_question.test_cases.each(&:save!)
end

def link_to_original_question(duplicated_programming_question)
@programming_question.question.actable = duplicated_programming_question
# Update the original programming question's parent_id to link to the new duplicated question
duplicated_programming_question.update_column(:parent_id, @programming_question.id)
end

def format_test_cases
@public_test_cases = []
@private_test_cases = []
Expand Down
91 changes: 66 additions & 25 deletions app/controllers/course/statistics/answers_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,45 +2,64 @@
class Course::Statistics::AnswersController < Course::Statistics::Controller
helper Course::Assessment::Submission::SubmissionsHelper.name.sub(/Helper$/, '')

MAX_ANSWERS_COUNT = 10
def latest_answer
@answer = Course::Assessment::Answer.find(answer_params[:id])
@submission = @answer.submission
@question = @answer.question
@assessment = @submission.assessment

Check warning on line 9 in app/controllers/course/statistics/answers_controller.rb

View check run for this annotation

Codecov / codecov/patch

app/controllers/course/statistics/answers_controller.rb#L6-L9

Added lines #L6 - L9 were not covered by tests

submission_id = @answer.submission_id
question_id = @answer.question_id

Check warning on line 12 in app/controllers/course/statistics/answers_controller.rb

View check run for this annotation

Codecov / codecov/patch

app/controllers/course/statistics/answers_controller.rb#L11-L12

Added lines #L11 - L12 were not covered by tests

def question_answer_details
@question_index = question_index(question_id)

Check warning on line 14 in app/controllers/course/statistics/answers_controller.rb

View check run for this annotation

Codecov / codecov/patch

app/controllers/course/statistics/answers_controller.rb#L14

Added line #L14 was not covered by tests

@submission_question = Course::Assessment::SubmissionQuestion.

Check warning on line 16 in app/controllers/course/statistics/answers_controller.rb

View check run for this annotation

Codecov / codecov/patch

app/controllers/course/statistics/answers_controller.rb#L16

Added line #L16 was not covered by tests
where(submission_id: submission_id, question_id: question_id).
includes(actable: { files: { annotations:
{ discussion_topic: { posts: :codaveri_feedback } } } },
discussion_topic: { posts: :codaveri_feedback }).first
end

def attempts
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

similarly why is this attempts not answers?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Attempt aligns with the vocab used in frontend, since we have never exposed the concept of "answer" to users

@answer = Course::Assessment::Answer.find(answer_params[:id])
@submission = @answer.submission
@question = @answer.question
@assessment = @submission.assessment

submission_id = @answer.submission_id
question_id = @answer.question_id

@question_index = question_index(question_id)

@submission_question = Course::Assessment::SubmissionQuestion.
where(submission_id: @answer.submission_id, question_id: @answer.question_id).
where(submission_id: submission_id, question_id: question_id).
includes(actable: { files: { annotations:
{ discussion_topic: { posts: :codaveri_feedback } } } },
discussion_topic: { posts: :codaveri_feedback }).first

fetch_all_answers(@answer.submission_id, @answer.question_id)
fetch_all_answers(submission_id, question_id, answer_params[:limit].to_i)
fetch_all_actable_questions(@question)
end

def all_answers
def all_attempts
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar comment, all_answers vs all_attempts

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Attempt aligns with the vocab used in frontend, since we have never exposed the concept of "answer" to users

@submission_question = Course::Assessment::SubmissionQuestion.find(submission_question_params[:id])
submission_id = @submission_question.submission_id
@submission = Course::Assessment::Submission.find(submission_id)
@submission = @submission_question.submission
@question = @submission_question.question
@assessment = @submission.assessment

submission_id = @submission_question.submission_id
question_id = @submission_question.question_id
@question = Course::Assessment::Question.find(question_id)
@assessment = @submission.assessment

@submission_question = Course::Assessment::SubmissionQuestion.
where(submission_id: submission_id, question_id: question_id).
includes({ discussion_topic: { posts: :codaveri_feedback } }).first
@question_index = question_index(question_id)
@all_answers = Course::Assessment::Answer.
unscope(:order).
order(:created_at).
where(submission_id: submission_id, question_id: question_id)

fetch_all_answers(submission_id, question_id, -1)
fetch_all_actable_questions(@question)
end

private

def answer_params
params.permit(:id)
params.permit(:id, :limit)
end

def submission_question_params
Expand All @@ -56,14 +75,36 @@
question_ids.index(question_id)
end

def fetch_all_answers(submission_id, question_id)
answers = Course::Assessment::Answer.
unscope(:order).
order(created_at: :desc).
where(submission_id: submission_id, question_id: question_id)
def fetch_all_answers(submission_id, question_id, limit)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should make limit specify the actual number (i.e. limit default is MAX_ANSWERS_COUNT, but can be value can pass as 1, 5, 50, or -1 for all.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@all_answers = if limit == -1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok I see you've updated the query to adjust the limit parameter to take in a number. Please squash the relevant commits.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Course::Assessment::Answer.
unscope(:order).
order(:submitted_at).
where(submission_id: submission_id, question_id: question_id)
else
Course::Assessment::Answer.

Check warning on line 85 in app/controllers/course/statistics/answers_controller.rb

View check run for this annotation

Codecov / codecov/patch

app/controllers/course/statistics/answers_controller.rb#L85

Added line #L85 was not covered by tests
unscope(:order).
order(:submitted_at).
where(submission_id: submission_id, question_id: question_id).
limit(limit)
end
end

def fetch_all_actable_questions(question)
unless versioned_question?(question)
@all_actable_questions = [question.actable]
return
end

question = question.actable
@all_actable_questions = [question]
while question.parent
@all_actable_questions << question.parent
question = question.parent

Check warning on line 103 in app/controllers/course/statistics/answers_controller.rb

View check run for this annotation

Codecov / codecov/patch

app/controllers/course/statistics/answers_controller.rb#L99-L103

Added lines #L99 - L103 were not covered by tests
end
end

current_answer = answers.find(&:current_answer?)
@all_answers = answers.where(current_answer: false).limit(MAX_ANSWERS_COUNT - 1).to_a.reverse
@all_answers.unshift(current_answer)
def versioned_question?(question)
question.actable.is_a?(Course::Assessment::Question::Programming)
end
end
16 changes: 16 additions & 0 deletions app/helpers/course/statistics/answers_helper.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# frozen_string_literal: true

module Course::Statistics::AnswersHelper
def get_historical_auto_gradings(programming_auto_grading)
historical = []
current = programming_auto_grading

Check warning on line 6 in app/helpers/course/statistics/answers_helper.rb

View check run for this annotation

Codecov / codecov/patch

app/helpers/course/statistics/answers_helper.rb#L5-L6

Added lines #L5 - L6 were not covered by tests

while current&.parent_id
parent = Course::Assessment::Answer::ProgrammingAutoGrading.find_by(id: current.parent_id)
historical.unshift(parent) if parent
current = parent

Check warning on line 11 in app/helpers/course/statistics/answers_helper.rb

View check run for this annotation

Codecov / codecov/patch

app/helpers/course/statistics/answers_helper.rb#L8-L11

Added lines #L8 - L11 were not covered by tests
end

historical

Check warning on line 14 in app/helpers/course/statistics/answers_helper.rb

View check run for this annotation

Codecov / codecov/patch

app/helpers/course/statistics/answers_helper.rb#L14

Added line #L14 was not covered by tests
end
end
3 changes: 3 additions & 0 deletions app/models/course/assessment/answer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,9 @@ class Course::Assessment::Answer < ApplicationRecord
def auto_grade!(redirect_to_path: nil, reduce_priority: false)
raise IllegalStateError if attempting?

# When evaluated or graded, only autograde most recent answer
return if !submitted? && !current_answer?

ensure_auto_grading!
if grade_inline?
Course::Assessment::Answer::AutoGradingService.grade(self)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ class Course::Assessment::Answer::ProgrammingAutoGrading < ApplicationRecord

validates :exit_code, numericality: { only_integer: true }, allow_nil: true

belongs_to :parent, class_name: 'Course::Assessment::Answer::ProgrammingAutoGrading', optional: true

has_one :programming_answer, through: :answer,
source: :actable,
source_type: 'Course::Assessment::Answer::Programming'
Expand Down
1 change: 1 addition & 0 deletions app/models/course/assessment/question/programming.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ class Course::Assessment::Question::Programming < ApplicationRecord # rubocop:di

belongs_to :import_job, class_name: 'TrackableJob::Job', inverse_of: nil, optional: true
belongs_to :language, class_name: 'Coursemology::Polyglot::Language', inverse_of: nil
belongs_to :parent, class_name: 'Course::Assessment::Question::Programming', optional: true
has_one_attachment
has_many :template_files, class_name: 'Course::Assessment::Question::ProgrammingTemplateFile',
dependent: :destroy, foreign_key: :question_id, inverse_of: :question
Expand Down
3 changes: 3 additions & 0 deletions app/services/course/assessment/answer/auto_grading_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,15 @@ class << self
#
# @param [Course::Assessment::Answer] answer The answer to be graded.
def grade(answer)
old_auto_grading_actable = answer.auto_grading&.actable
answer = if answer.question.auto_gradable?
pick_grader(answer.question).grade(answer)
else
assign_maximum_grade(answer)
end

answer.save!
answer.auto_grading.actable.update!(parent_id: old_auto_grading_actable.id) if old_auto_grading_actable
end

private
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
# frozen_string_literal: true
json.language question.language.name
json.fileSubmission question.multiple_file_submission

json.memoryLimit question.memory_limit if question.memory_limit
json.timeLimit question.time_limit if question.time_limit
json.attemptLimit question.attempt_limit if question.attempt_limit

json.fileSubmission question.multiple_file_submission?

json.autogradable question.auto_gradable?
json.isCodaveri question.is_codaveri
json.liveFeedbackEnabled question.live_feedback_enabled

json.isCodaveri question.is_codaveri?
json.liveFeedbackEnabled question.live_feedback_enabled?
Comment on lines +12 to +13
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason why you want to explicitly add ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typically within the codebase and in ruby booleans are all decorated with ?
So i changed it as such to indicate that it is a boolean

12 changes: 12 additions & 0 deletions app/views/course/statistics/answers/_all_questions.json.jbuilder
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# frozen_string_literal: true

json.allQuestions @all_actable_questions do |question|
json.id question.id
json.title @question.title
json.maximumGrade @question.maximum_grade
json.description format_ckeditor_rich_text(@question.description)
json.type @question.question_type
json.questionNumber @question_index + 1

json.partial! question, question: question, can_grade: false, answer: @all_answers.first
end
11 changes: 10 additions & 1 deletion app/views/course/statistics/answers/_answer.json.jbuilder
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,12 @@ specific_answer = answer.specific
json.id answer.id
json.grade answer.grade
json.questionType question.question_type
json.partial! specific_answer, answer: specific_answer, can_grade: false

if answer.actable_type == Course::Assessment::Answer::Programming.name
json.partial! 'programming_answer', answer: specific_answer, can_grade: false
else
json.partial! specific_answer, answer: specific_answer, can_grade: false
end

if answer.actable_type == Course::Assessment::Answer::Programming.name
files = answer.specific.files
Expand All @@ -16,3 +21,7 @@ if answer.actable_type == Course::Assessment::Answer::Programming.name
json.partial! post, post: post if post.published?
end
end

json.submittedAt answer.submitted_at&.iso8601
json.currentAnswer answer.current_answer
json.workflowState answer.workflow_state
13 changes: 13 additions & 0 deletions app/views/course/statistics/answers/_details.json.jbuilder
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# frozen_string_literal: true

json.partial! 'all_questions'

json.allAnswers @all_answers do |answer|
json.partial! 'answer', answer: answer, question: @question
end

posts = @submission_question.discussion_topic.posts

json.comments posts do |post|
json.partial! post, post: post if post.published?
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
# frozen_string_literal: true
question = @question.specific

# If a non current_answer is being loaded, use it instead of loading the last_attempt.
is_current_answer = answer.current_answer?
latest_answer = last_attempt(answer)
attempt = is_current_answer ? latest_answer : answer
auto_grading = attempt&.auto_grading&.specific

json.fields do
json.questionId answer.question_id
json.id answer.acting_as.id
json.files_attributes answer.files do |file|
json.(file, :id, :filename)
json.content file.content
json.highlightedContent highlight_code_block(file.content, question.language)
end
end

can_read_tests = can?(:read_tests, @submission)
show_private = can_read_tests || (@submission.published? && @assessment.show_private?)
show_evaluation = can_read_tests || (@submission.published? && @assessment.show_evaluation?)

test_cases_by_type = question.test_cases_by_type
test_cases_and_results = get_test_cases_and_results(test_cases_by_type, auto_grading)

show_stdout_and_stderr = (can_read_tests || current_course.show_stdout_and_stderr) &&
auto_grading && auto_grading&.exit_code != 0

displayed_test_case_types = ['public_test']
displayed_test_case_types << 'private_test' if show_private
displayed_test_case_types << 'evaluation_test' if show_evaluation

historical_auto_gradings = get_historical_auto_gradings(auto_grading)

json.testCases (historical_auto_gradings + [auto_grading]).each do |ag|
next if ag.nil? # To account for autogradings with no test results (programming with no autograding)

question = ag.test_results.first.test_case.question
test_cases_by_type = question.test_cases_by_type
test_cases_and_results = get_test_cases_and_results(test_cases_by_type, ag)

json.questionId question.id
displayed_test_case_types.each do |test_case_type|
show_public = (test_case_type == 'public_test') && current_course.show_public_test_cases_output
show_testcase_outputs = can_read_tests || show_public
json.set! test_case_type do
if test_cases_and_results[test_case_type].present?
json.array! test_cases_and_results[test_case_type] do |test_case, test_result|
json.identifier test_case.identifier if can_read_tests
json.expression test_case.expression
json.expected test_case.expected
if test_result
json.output get_output(test_result) if show_testcase_outputs
json.passed test_result.passed?
end
end

end
end
json.(ag, :stdout, :stderr) if show_stdout_and_stderr
end
end

if answer.codaveri_feedback_job_id && question.is_codaveri
codaveri_job = answer.codaveri_feedback_job
json.codaveriFeedback do
json.jobId answer.codaveri_feedback_job_id
json.jobStatus codaveri_job.status
json.jobUrl job_path(codaveri_job) if codaveri_job.status == 'submitted'
json.errorMessage codaveri_job.error['message'] if codaveri_job.error
end
end
Loading