From 5b972fce19691ee08213688ab3e3dd6c60fe2ea0 Mon Sep 17 00:00:00 2001 From: Jude Rozairo Date: Thu, 14 Nov 2024 19:40:36 +1100 Subject: [PATCH 1/3] Added target grade change history feature by changing both front and back end --- app/api/entities/project_entity.rb | 2 ++ .../entities/target_grade_history_entity.rb | 8 ++++++ app/api/projects_api.rb | 25 ++++++++++++++++--- app/models/project.rb | 2 ++ app/models/target_grade_history.rb | 7 ++++++ ...41113065336_create_target_grade_history.rb | 14 +++++++++++ db/schema.rb | 13 +++++++++- 7 files changed, 66 insertions(+), 5 deletions(-) create mode 100644 app/api/entities/target_grade_history_entity.rb create mode 100644 app/models/target_grade_history.rb create mode 100644 db/migrate/20241113065336_create_target_grade_history.rb 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 From ebeff7505b14a9af38fff342a39ea3e209d809da Mon Sep 17 00:00:00 2001 From: Jude Roz Date: Mon, 23 Dec 2024 02:12:28 +1100 Subject: [PATCH 2/3] Added backend to retrive data based on page number and also to return the total amount of histories for the page numbers --- app/api/projects_api.rb | 69 ++++++++++++++++++++++++++++++++++++----- 1 file changed, 62 insertions(+), 7 deletions(-) diff --git a/app/api/projects_api.rb b/app/api/projects_api.rb index 3fe4e0683..40218b462 100644 --- a/app/api/projects_api.rb +++ b/app/api/projects_api.rb @@ -20,7 +20,7 @@ class ProjectsApi < Grape::API present projects, with: Entities::ProjectEntity, for_student: true, summary_only: true, user: current_user end - desc 'Get project with target grade history' + desc 'Get project with target grade history (unchanged route)' params do requires :id, type: Integer, desc: 'The id of the project to get' end @@ -28,12 +28,61 @@ class ProjectsApi < Grape::API 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 + present project, with: Entities::ProjectEntity, + user: current_user, + for_student: true, + in_project: true else error!({ error: "Couldn't find Project with id=#{params[:id]}" }, 403) end end + + desc 'Get paginated target grade histories for a project (manual pagination)' + params do + requires :id, type: Integer, desc: 'The id of the project' + optional :page, type: Integer, desc: 'Page number', default: 1 + optional :limit, type: Integer, desc: 'Records per page', default: 10 + end + get '/projects/:id/target_grade_histories' do + project = Project.find(params[:id]) + unless authorise?(current_user, project, :get) + error!({ error: "You are not authorised to view this project" }, 403) + end + + page = params[:page] + limit = params[:limit] + offset = (page - 1) * limit + + # Sort by changed_at DESC in DB, then limit/offset for pagination + total_histories = project.target_grade_histories.count + paged_histories = project.target_grade_histories + .order(changed_at: :desc) + .limit(limit) + .offset(offset) + .includes(:changed_by) + + present({ + target_grade_histories: paged_histories.map do |h| + { + id: h.id, + previous_grade: h.previous_grade, + new_grade: h.new_grade, + changed_at: h.changed_at, + changed_by: { + id: h.changed_by_id, + email: h.changed_by&.email, + first_name: h.changed_by&.first_name, + last_name: h.changed_by&.last_name, + username: h.changed_by&.username, + nickname: h.changed_by&.nickname + } + } + end, + total_histories: total_histories + }) + end + desc 'Update a project' params do optional :trigger, type: String, desc: 'The update trigger' @@ -67,7 +116,7 @@ class ProjectsApi < Grape::API error!({ error: "You cannot change the campus for this student" }, 403) end for_student = false - project.campus_id = params[:campus_id] == -1 ? nil : params[:campus_id] + project.campus_id = (params[:campus_id] == -1) ? nil : params[:campus_id] project.save! elsif !params[:enrolled].nil? unless authorise? current_user, project.unit, :change_project_enrolment @@ -128,7 +177,6 @@ class ProjectsApi < Grape::API end for_student = false - project.grade = params[:grade] project.grade_rationale = params[:grade_rationale] project.save! @@ -140,13 +188,20 @@ class ProjectsApi < Grape::API error!({ error: "You do not have permissions to change this student" }, 403) end - # if someone changes this setting manually, clear the autogenerated status project.portfolio_auto_generated = false project.compile_portfolio = params[:compile_portfolio] project.save end - Entities::ProjectEntity.represent(project, only: [:campus_id, :enrolled, :target_grade, :submitted_grade, :compile_portfolio, :portfolio_available, :uses_draft_learning_summary, :stats], for_student: for_student) + Entities::ProjectEntity.represent( + project, + only: [ + :campus_id, :enrolled, :target_grade, :submitted_grade, + :compile_portfolio, :portfolio_available, :uses_draft_learning_summary, + :stats + ], + for_student: for_student + ) end # put desc 'Enrol a student in a unit, creating them a project' @@ -199,4 +254,4 @@ class ProjectsApi < Grape::API error!({ error: "Couldn't find Unit with id=#{params[:unit_id]}" }, 403) end end -end +end \ No newline at end of file From bb545c6c2a439b44b6c597be9b2fbf2dcc254cdd Mon Sep 17 00:00:00 2001 From: Jude Roz Date: Sat, 11 Jan 2025 19:19:17 +1100 Subject: [PATCH 3/3] fixed data duplication providing 2 API calls with target grade history, paginated and unpaginated --- app/api/projects_api.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/api/projects_api.rb b/app/api/projects_api.rb index 40218b462..7e02c43a6 100644 --- a/app/api/projects_api.rb +++ b/app/api/projects_api.rb @@ -20,7 +20,7 @@ class ProjectsApi < Grape::API present projects, with: Entities::ProjectEntity, for_student: true, summary_only: true, user: current_user end - desc 'Get project with target grade history (unchanged route)' + desc 'Get project without target grade history ' params do requires :id, type: Integer, desc: 'The id of the project to get' end @@ -31,7 +31,7 @@ class ProjectsApi < Grape::API present project, with: Entities::ProjectEntity, user: current_user, for_student: true, - in_project: true + in_project: false #make this true to show all target grade histories if needed for future to the frontend. else error!({ error: "Couldn't find Project with id=#{params[:id]}" }, 403) end