Merge pull request doubtfire-lms#468 from b0ink/feat/task-discussed-state

feat: task discussed comment + staff notes

Author: macite

app/api/api_root.rb

@@ -75,6 +75,7 @@ class ApiRoot < Grape::API
   mount Similarity::TaskSimilarityApi
   mount TeachingPeriodsPublicApi
   mount TeachingPeriodsAuthenticatedApi
+  mount StaffNotesApi
 
   mount Tii::TurnItInApi
   mount Tii::TurnItInHooksApi
@@ -122,6 +123,7 @@ class ApiRoot < Grape::API
   AuthenticationHelpers.add_auth_to TaskCommentsApi
   AuthenticationHelpers.add_auth_to TaskDefinitionsApi
   AuthenticationHelpers.add_auth_to TeachingPeriodsAuthenticatedApi
+  AuthenticationHelpers.add_auth_to StaffNotesApi
 
   AuthenticationHelpers.add_auth_to Tii::TurnItInApi
   AuthenticationHelpers.add_auth_to Tii::TiiGroupAttachmentApi

app/api/entities/staff_note_entity.rb

@@ -0,0 +1,14 @@
+module Entities
+  class StaffNoteEntity < Grape::Entity
+    expose :id
+
+    expose :note
+    expose :user_id
+
+    expose :created_at
+    expose :updated_at
+
+    expose :reply_to_id
+
+  end
+end

app/api/staff_notes_api.rb

@@ -0,0 +1,94 @@
+require 'grape'
+
+class StaffNotesApi < Grape::API
+  helpers AuthenticationHelpers
+  helpers AuthorisationHelpers
+
+  before do
+    authenticated?
+  end
+
+  desc "Get all the staff notes for a project"
+  params do
+    requires :project_id, type: Integer, desc: 'Project to fetch staff notes for'
+  end
+  get '/projects/:project_id/staff_notes' do
+    project = Project.find(params[:project_id])
+
+    unless authorise? current_user, project, :get_staff_note
+      error!({ error: 'You do not have permission to access this project' }, 403)
+    end
+
+    result = project.staff_notes
+
+    present result, with: Entities::StaffNoteEntity, user: current_user
+  end
+
+  desc "Create a new staff note for a project"
+  params do
+    requires :project_id,   type: Integer, desc: 'Project to add the staff note for'
+    requires :note,         type: String, desc: 'The text to add to the staff note'
+    optional :reply_to_id,  type: Integer, desc: 'ID of the staff note this is being replied to'
+  end
+  post '/projects/:project_id/staff_notes' do
+    project = Project.find(params[:project_id])
+
+    unless authorise? current_user, project, :create_staff_note
+      error!({ error: 'You do not have permission to access this project' }, 403)
+    end
+
+    text_note = params[:note]
+
+    reply_to_id = params[:reply_to_id]
+    if reply_to_id.present?
+      original_staff_note = StaffNote.find(reply_to_id)
+      error!(error: 'You do not have permission to read the replied staff note') unless authorise?(current_user, original_staff_note.project, :get)
+      error!(error: 'Original staff note is not in this project.') if project.staff_notes.find(reply_to_id).blank?
+    end
+
+    result = project.add_staff_note(current_user, text_note, reply_to_id)
+
+    if result.nil?
+      error!({ error: 'Duplicate note.' }, 403)
+    else
+      present result, with: Entities::StaffNoteEntity, user: current_user
+    end
+  end
+
+  desc "Delete a staff note for a project"
+  delete '/projects/:project_id/staff_notes/:id' do
+    project = Project.find(params[:project_id])
+    staff_note = StaffNote.find(params[:id])
+
+    unless authorise?(current_user, project, :delete_staff_note) || staff_note.user.id == current_user.id
+      error!({ error: 'You do not have permission to delete this note.' }, 403)
+    end
+
+    error!({ error: 'Note does not belong to this project' }, 404) if staff_note.project_id != project.id
+
+    staff_note.destroy
+    error!({ error: staff_note.errors.full_messages.last }, 403) unless staff_note.destroyed?
+
+    present staff_note.destroyed?, with: Grape::Presenters::Presenter
+  end
+
+  desc "Update a staff note for a project"
+  params do
+    requires :id, type: Integer, desc: 'The staff note id to update'
+    requires :note, type: String, desc: 'The text to update the staff note with'
+  end
+  put '/projects/:project_id/staff_notes/:id' do
+    project = Project.find(params[:project_id])
+    staff_note = StaffNote.find(params[:id])
+
+    unless authorise?(current_user, project, :create_staff_note) && staff_note.user.id == current_user.id
+      error!({ error: 'You do not have permission to edit this note.' }, 403)
+    end
+
+    error!({ error: 'Note does not belong to this project' }, 404) if staff_note.project_id != project.id
+
+    staff_note.update!(note: params[:note])
+    present staff_note, with: Entities::StaffNoteEntity, user: current_user
+  end
+
+end

app/api/tasks_api.rb

@@ -155,6 +155,7 @@ class TasksApi < Grape::API
     optional :include_in_portfolio, type: Boolean, desc: 'Indicate if this task should be in the portfolio'
     optional :grade, type: Integer, desc: 'Grade value if task is a graded task (required if task definition is a graded task)'
     optional :quality_pts, type: Integer, desc: 'Quality points value if task has quality assessment'
+    optional :discussed, type: Boolean, desc: 'Mark task as discussed'
   end
   put '/projects/:id/task_def_id/:task_definition_id' do
     project = Project.find(params[:id])
@@ -166,6 +167,10 @@ class TasksApi < Grape::API
     if authorise? current_user, project, :make_submission
       task = project.task_for_task_definition(task_definition)
 
+      if !params[:discussed].nil? && authorise?(current_user, project, :assess)
+        task.add_discussed_comment(current_user)
+      end
+
       # if trigger supplied...
       unless params[:trigger].nil?
         # Check if they should be using portfolio_evidence api

app/models/comments/task_discussed_comment.rb

@@ -0,0 +1,16 @@
+class TaskDiscussedComment < TaskComment
+  before_create do
+    self.content_type = :discussed_in_class
+  end
+
+  after_create do
+    mark_as_read(self.recipient)
+  end
+
+  def serialize(user)
+    json = super(user)
+    json[:recipient_read_time] = nil
+    json[:date] = self.created_at
+    json
+  end
+end

app/models/project.rb

@@ -29,6 +29,8 @@ class Project < ApplicationRecord
   has_many :task_engagements, through: :tasks
   has_many :comments, through: :tasks
 
+  has_many :staff_notes, dependent: :destroy
+
   # Callbacks - methods called are private
   before_destroy :can_destroy?
 
@@ -60,18 +62,24 @@ def self.permissions
       :get_submission,
       :change,
       :assess,
-      :change_campus
+      :change_campus,
+      :get_staff_note,
+      :create_staff_note
     ]
+
     # What can admins do with projects?
     admin_role_permissions = [
       :get,
       :get_submission
     ]
+
     # What can auditors do with projects?
     auditor_role_permissions = [
       :get,
-      :get_submission
+      :get_submission,
+      :get_staff_note
     ]
+
     # What can nil users do with projects?
     nil_role_permissions = []
 
@@ -661,6 +669,24 @@ def archive_submissions(out)
     FileUtils.rm_f(portfolio_path) if portfolio_available
   end
 
+  def add_staff_note(user, text, reply_to_id = nil)
+    text = text.strip
+    return nil if user.nil? || text.nil? || text.empty?
+
+    ln = staff_notes.last
+
+    # don't add if duplicate note
+    return if ln && ln.user == user && ln.note == text
+
+    note = StaffNote.create
+    note.note = text
+    note.user = user
+    note.project = self
+    note.reply_to_id = reply_to_id
+    note.save!
+    note
+  end
+
   private
 
   def can_destroy?

app/models/staff_note.rb

@@ -0,0 +1,7 @@
+class StaffNote < ApplicationRecord
+  belongs_to :project
+  belongs_to :user
+
+  # belongs_to :reply_to, class_name: 'StaffNote', optional: true, inverse_of: :replies
+  # has_many :replies, class_name: 'StaffNote', dependent: :restrict_with_exception, inverse_of: :reply_to
+end

app/models/task.rb

@@ -750,6 +750,16 @@ def add_status_comment(current_user, status)
     comment
   end
 
+  def add_discussed_comment(current_user)
+    discussed = TaskDiscussedComment.create
+    discussed.task = self
+    discussed.user = current_user
+    discussed.comment = "Discussed in class"
+    discussed.recipient = current_user == project.student ? project.tutor_for(task_definition) : project.student
+    discussed.save!
+    discussed
+  end
+
   def add_discussion_comment(user, prompts)
     # don't allow if group task.
     discussion = DiscussionComment.create

app/models/unit.rb

@@ -440,6 +440,7 @@ def student_query(enrolled)
         .joins('LEFT OUTER JOIN tutorial_enrolments ON tutorial_enrolments.project_id = projects.id')
         .joins('LEFT OUTER JOIN tutorials ON tutorials.id = tutorial_enrolments.tutorial_id')
         .joins('LEFT OUTER JOIN tutorial_streams ON tutorials.tutorial_stream_id = tutorial_streams.id')
+        .joins('LEFT OUTER JOIN staff_notes ON staff_notes.project_id = projects.id')
         .group(
           'projects.id',
           'projects.target_grade',
@@ -481,7 +482,8 @@ def student_query(enrolled)
           # Get tutorial for each stream in unit
           *tutorial_streams.map { |s| "MAX(CASE WHEN tutorials.tutorial_stream_id = #{s.id} OR tutorials.tutorial_stream_id IS NULL THEN tutorials.id ELSE NULL END) AS tutorial_#{s.id}" },
           # Get tutorial for case when no stream
-          "MAX(CASE WHEN tutorial_streams.id IS NULL THEN tutorials.id ELSE NULL END) AS tutorial"
+          "MAX(CASE WHEN tutorial_streams.id IS NULL THEN tutorials.id ELSE NULL END) AS tutorial",
+          'COUNT(DISTINCT staff_notes.id) as staff_note_count'
         )
         .order('users.first_name')
 
@@ -515,6 +517,7 @@ def student_query(enrolled)
         similarity_flag: t.task_similarities_max_pct > 0,
         has_portfolio: !t.portfolio_production_date.nil?,
         stats: map_stats.call(t),
+        staff_note_count: t.staff_note_count,
         tutorial_enrolments: tutorial_streams.map do |s|
           {
             stream_abbr: s.abbreviation,

db/migrate/20250709010605_create_staff_notes.rb

@@ -0,0 +1,12 @@
+class CreateStaffNotes < ActiveRecord::Migration[8.0]
+  def change
+    create_table :staff_notes do |t|
+      t.text :note
+      t.references :project, index: true, null: false
+      t.references :user, index: true, null: false
+      t.references :staff_notes, :reply_to, index: true, null: true
+
+      t.timestamps
+    end
+  end
+end