diff --git a/app/api/entities/project_entity.rb b/app/api/entities/project_entity.rb index 0f3672f66..c95d6df4e 100644 --- a/app/api/entities/project_entity.rb +++ b/app/api/entities/project_entity.rb @@ -28,5 +28,7 @@ class ProjectEntity < Grape::Entity expose :grade, if: :for_staff expose :grade_rationale, if: :for_staff + + expose :target_grade_histories, using: Entities::TargetGradeHistoryEntity, if: ->(project, options) { options[:in_project] } end end diff --git a/app/api/entities/target_grade_history_entity.rb b/app/api/entities/target_grade_history_entity.rb new file mode 100644 index 000000000..a3c1a0d74 --- /dev/null +++ b/app/api/entities/target_grade_history_entity.rb @@ -0,0 +1,8 @@ +module Entities + class TargetGradeHistoryEntity < Grape::Entity + expose :previous_grade + expose :new_grade + expose :changed_at + expose :changed_by, using: Entities::Minimal::MinimalUserEntity + end +end diff --git a/app/api/projects_api.rb b/app/api/projects_api.rb index dc3cbafc6..3fe4e0683 100644 --- a/app/api/projects_api.rb +++ b/app/api/projects_api.rb @@ -20,12 +20,12 @@ class ProjectsApi < Grape::API present projects, with: Entities::ProjectEntity, for_student: true, summary_only: true, user: current_user end - desc 'Get project' + desc 'Get project with target grade history' params do requires :id, type: Integer, desc: 'The id of the project to get' end get '/projects/:id' do - project = Project.eager_load(:unit, :user).find(params[:id]) + project = Project.includes(:unit, :user, target_grade_histories: :changed_by).find(params[:id]) if authorise? current_user, project, :get present project, with: Entities::ProjectEntity, user: current_user, for_student: true, in_project: true @@ -81,8 +81,25 @@ class ProjectsApi < Grape::API error!({ error: "You do not have permissions to change this student" }, 403) end - project.target_grade = params[:target_grade] - project.save + previous_grade = project.target_grade + new_grade = params[:target_grade] + + project.target_grade = new_grade + + if project.save + # Record the grade change history + TargetGradeHistory.create( + project_id: project.id, + user_id: project.user_id, + previous_grade: previous_grade, + new_grade: new_grade, + changed_by_id: current_user.id, + changed_at: Time.current + ) + else + error!({ error: "Failed to update target grade: #{project.errors.full_messages.join(', ')}" }, 422) + end + elsif !params[:submitted_grade].nil? unless authorise? current_user, project, :change error!({ error: "You do not have permissions to change this student" }, 403) diff --git a/app/models/project.rb b/app/models/project.rb index c0770cf87..af03662b5 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -31,6 +31,8 @@ class Project < ApplicationRecord has_many :learning_outcome_task_links, through: :tasks + has_many :target_grade_histories, dependent: :destroy + # Callbacks - methods called are private before_destroy :can_destroy? diff --git a/app/models/target_grade_history.rb b/app/models/target_grade_history.rb new file mode 100644 index 000000000..8de7489a1 --- /dev/null +++ b/app/models/target_grade_history.rb @@ -0,0 +1,7 @@ +class TargetGradeHistory < ApplicationRecord + belongs_to :project + belongs_to :user + belongs_to :changed_by, class_name: 'User', foreign_key: 'changed_by_id' + + validates :project_id, :user_id, :previous_grade, :new_grade, :changed_at, presence: true +end diff --git a/db/migrate/20241113065336_create_target_grade_history.rb b/db/migrate/20241113065336_create_target_grade_history.rb new file mode 100644 index 000000000..650c12eab --- /dev/null +++ b/db/migrate/20241113065336_create_target_grade_history.rb @@ -0,0 +1,14 @@ +class CreateTargetGradeHistory < ActiveRecord::Migration[7.1] + def change + create_table :target_grade_histories do |t| + t.integer :project_id, null: false #reference to the unit project + t.integer :user_id, null: false #This is where we refer to the student by the ID + t.string :previous_grade #Previous grade of the user before we update it + t.string :new_grade #Updated grade + t.integer :changed_by_id #Referene to the user who changed the grade + t.datetime :changed_at, null: false #The date time when the grade was changed + + t.timestamps + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 6daa71ebf..656ea8cbf 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.1].define(version: 2024_05_28_223908) do +ActiveRecord::Schema[7.1].define(version: 2024_11_13_065336) do create_table "activity_types", charset: "utf8", collation: "utf8_unicode_ci", force: :cascade do |t| t.string "name", null: false t.string "abbreviation", null: false @@ -194,6 +194,17 @@ t.datetime "updated_at", null: false end + create_table "target_grade_histories", charset: "utf8mb4", collation: "utf8mb4_general_ci", force: :cascade do |t| + t.integer "project_id", null: false + t.integer "user_id", null: false + t.string "previous_grade" + t.string "new_grade" + t.integer "changed_by_id" + t.datetime "changed_at", null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + end + create_table "task_comments", charset: "utf8", collation: "utf8_unicode_ci", force: :cascade do |t| t.bigint "task_id", null: false t.bigint "user_id", null: false diff --git a/test/api/projects_api_test.rb b/test/api/projects_api_test.rb index 6d4ab3087..a98d9529b 100644 --- a/test/api/projects_api_test.rb +++ b/test/api/projects_api_test.rb @@ -23,7 +23,8 @@ def test_can_get_projects end def test_get_projects_with_streams_match - unit = FactoryBot.create :unit, stream_count: 2, campus_count: 2, tutorials: 2, unenrolled_student_count: 0, part_enrolled_student_count: 0, inactive_student_count: 0 + unit = FactoryBot.create :unit, stream_count: 2, campus_count: 2, tutorials: 2, + unenrolled_student_count: 0, part_enrolled_student_count: 0, inactive_student_count: 0 project = unit.projects.first assert_equal 2, project.tutorial_enrolments.count @@ -76,7 +77,24 @@ def test_get_project_response_is_correct # Add username and auth_token to Header add_auth_header_for(user: user) - keys = %w(id unit unit_id user_id campus_id target_grade submitted_grade portfolio_files compile_portfolio portfolio_available uses_draft_learning_summary tasks tutorial_enrolments groups task_outcome_alignments) + keys = %w( + id + unit + unit_id + user_id + campus_id + target_grade + submitted_grade + portfolio_files + compile_portfolio + portfolio_available + uses_draft_learning_summary + tasks + tutorial_enrolments + groups + task_outcome_alignments + target_grade_histories + ) key_test = keys - %w(unit user_id portfolio_available tasks tutorial_enrolments groups task_outcome_alignments) get "/api/projects/#{project.id}" @@ -100,7 +118,6 @@ def test_projects_works_with_inactive_units assert_equal 1, last_response_body.count get '/api/projects?include_inactive=true' - assert_equal 2, last_response_body.count last_response_body.each do |data| @@ -192,4 +209,44 @@ def test_download_portfolio ensure FileUtils.rm_f(project.portfolio_path) end -end + + + + def test_get_project_includes_target_grade_histories + user = FactoryBot.create(:user, :student, enrol_in: 1) + project = user.projects.first + project.update!(target_grade: 1) + + add_auth_header_for(user: user) + + data_to_put = { target_grade: 2 } + put_json "/api/projects/#{project.id}", data_to_put + assert_equal 200, last_response.status, last_response.body + + data_to_put = { target_grade: 3 } + put_json "/api/projects/#{project.id}", data_to_put + assert_equal 200, last_response.status, last_response.body + + get "/api/projects/#{project.id}" + assert_equal 200, last_response.status, last_response.body + project_json = last_response_body + + assert project_json.key?('target_grade_histories'), project_json + histories = project_json['target_grade_histories'] + assert_equal 2, histories.size, "Expected 2 history entries after 2 updates" + + first_history = histories.first + assert_equal '1', first_history['previous_grade'] + assert_equal '2', first_history['new_grade'] + assert first_history.key?('changed_at'), "Expected changed_at key" + assert first_history.key?('changed_by'), "Expected changed_by key" + + second_history = histories.second + assert_equal '2', second_history['previous_grade'] + assert_equal '3', second_history['new_grade'] + + changed_by_info = second_history['changed_by'] + assert_equal user.id, changed_by_info['id'] + assert_equal user.username, changed_by_info['username'] + end +end \ No newline at end of file