Add school_project model and refactor routes to get and set the finished flag (#490)

loiswells97 · jamiebenstead · chrisroos · web-flow · commit 7183dc761bc4 · 2025-02-13T15:09:34.000Z
## Status closes #485 --------- Co-authored-by: Jamie Benstead <jamie.benstead@gmail.com> Co-authored-by: Chris Roos <chris@seagul.co.uk> Co-authored-by: Dan Halson <dan.halson@raspberrypi.org>
diff --git a/app/controllers/api/projects/remixes_controller.rb b/app/controllers/api/projects/remixes_controller.rb
@@ -8,7 +8,7 @@ class RemixesController < ApiController
 
       def index
         projects = Project.where(remixed_from_id: project.id).accessible_by(current_ability)
-        @projects_with_users = projects.with_users(@current_user)
+        @projects_with_users = projects.includes(:school_project).with_users(@current_user)
         render index: @projects_with_users, formats: [:json]
       end
 
@@ -47,6 +47,9 @@ def remix_params
                       :project_type,
                       :locale,
                       :user_id,
+                      :videos,
+                      :audio,
+                      :instructions,
                       image_list: [],
                       components: %i[id name extension content index])
       end
diff --git a/app/controllers/api/projects_controller.rb b/app/controllers/api/projects_controller.rb
@@ -5,7 +5,7 @@
 module Api
   class ProjectsController < ApiController
     before_action :authorize_user, only: %i[create update index destroy]
-    before_action :load_project, only: %i[show update toggle_finished destroy]
+    before_action :load_project, only: %i[show update destroy]
     before_action :load_projects, only: %i[index]
     load_and_authorize_resource
     before_action :verify_lesson_belongs_to_school, only: :create
@@ -36,16 +36,6 @@ def create
       end
     end
 
-    def toggle_finished
-      result = Project::ToggleFinished.call(project: @project)
-
-      if result.success?
-        head :ok
-      else
-        render json: { error: result[:error] }, status: :unprocessable_entity
-      end
-    end
-
     def update
       result = Project::Update.call(project: @project, update_hash: project_params, current_user: @current_user)
 
diff --git a/app/controllers/api/school_projects_controller.rb b/app/controllers/api/school_projects_controller.rb
@@ -0,0 +1,25 @@
+# frozen_string_literal: true
+
+module Api
+  class SchoolProjectsController < ApiController
+    before_action :authorize_user
+    load_and_authorize_resource :project
+
+    def show_finished
+      @school_project = Project.find_by(identifier: params[:id]).school_project
+      render :finished, formats: [:json], status: :ok
+    end
+
+    def set_finished
+      project = Project.find_by(identifier: params[:id])
+      result = SchoolProject::SetFinished.call(school_project: project.school_project, finished: params[:finished])
+
+      if result.success?
+        @school_project = result[:school_project]
+        render :finished, formats: [:json], status: :ok
+      else
+        render json: { error: result[:error] }, status: :unprocessable_entity
+      end
+    end
+  end
+end
diff --git a/app/models/ability.rb b/app/models/ability.rb
@@ -9,7 +9,6 @@ def initialize(user)
     return unless user
 
     define_authenticated_non_student_abilities(user)
-
     user.schools.each do |school|
       define_school_student_abilities(user:, school:) if user.school_student?(school)
       define_school_teacher_abilities(user:, school:) if user.school_teacher?(school)
@@ -99,16 +98,7 @@ def define_school_student_abilities(user:, school:)
     can(%i[read], Lesson, school_id: school.id, visibility: 'students', school_class: { members: { student_id: user.id } })
     can(%i[read create update], Project, school_id: school.id, user_id: user.id, lesson_id: nil)
     can(%i[read], Project, lesson: { school_id: school.id, school_class: { members: { student_id: user.id } } })
-    can(%i[toggle_finished], Project) do |project|
-      school_student_can_toggle_finished?(user:, school:, project:)
-    end
-  end
-
-  def school_student_can_toggle_finished?(user:, school:, project:)
-    is_my_project = project.user_id == user.id && project.school_id == school.id
-    is_a_remix = project.remixed_from_id.present?
-
-    is_my_project && is_a_remix
+    can(%i[show_finished set_finished], SchoolProject, project: { user_id: user.id, lesson_id: nil }, school_id: school.id)
   end
 
   def school_teacher_can_manage_lesson?(user:, school:, lesson:)
diff --git a/app/models/project.rb b/app/models/project.rb
@@ -10,17 +10,21 @@ class Project < ApplicationRecord
   has_many_attached :images
   has_many_attached :videos
   has_many_attached :audio
+  has_one :school_project, dependent: :destroy
 
   accepts_nested_attributes_for :components
 
   before_validation :check_unique_not_null, on: :create
+  before_validation :create_school_project_if_needed
 
   validates :identifier, presence: true, uniqueness: { scope: :locale }
   validate :identifier_cannot_be_taken_by_another_user
   validates :locale, presence: true, unless: :user_id
   validate :user_has_a_role_within_the_school
   validate :user_is_a_member_or_the_owner_of_the_lesson
   validate :project_with_instructions_must_belong_to_school
+  validate :project_with_school_id_has_school_project
+  validate :school_project_school_matches_project_school
 
   scope :internal_projects, -> { where(user_id: nil) }
 
@@ -70,6 +74,12 @@ def check_unique_not_null
     self.identifier ||= PhraseIdentifier.generate
   end
 
+  def create_school_project_if_needed
+    return unless school.present? && school_project.nil?
+
+    self.school_project = SchoolProject.new(school:)
+  end
+
   def identifier_cannot_be_taken_by_another_user
     return if Project.where(identifier: self.identifier).where.not(user_id:).empty?
 
@@ -98,4 +108,16 @@ def project_with_instructions_must_belong_to_school
 
     errors.add(:instructions, 'Projects with instructions must belong to a school')
   end
+
+  def project_with_school_id_has_school_project
+    return unless school_id && !school_project
+
+    errors.add(:school_project, 'Project with school_id must have a school_project')
+  end
+
+  def school_project_school_matches_project_school
+    return unless school_id && school_project && school_id != school_project.school_id
+
+    errors.add(:school_project, 'School project school_id must match project school_id')
+  end
 end
diff --git a/app/models/school.rb b/app/models/school.rb
@@ -5,6 +5,7 @@ class School < ApplicationRecord
   has_many :lessons, dependent: :nullify
   has_many :projects, dependent: :nullify
   has_many :roles, dependent: :nullify
+  has_many :school_projects, dependent: :nullify
 
   VALID_URL_REGEX = %r{\A(?:https?://)?(?:www.)?[a-z0-9]+([-.]{1}[a-z0-9]+)*\.[a-z]{2,63}(\.[a-z]{2,63})*(/.*)?\z}ix
 
diff --git a/app/models/school_project.rb b/app/models/school_project.rb
@@ -0,0 +1,6 @@
+# frozen_string_literal: true
+
+class SchoolProject < ApplicationRecord
+  belongs_to :school
+  belongs_to :project
+end
diff --git a/app/views/api/projects/remixes/index.json.jbuilder b/app/views/api/projects/remixes/index.json.jbuilder
@@ -12,4 +12,5 @@ json.array!(@projects_with_users) do |project, user|
   )
 
   json.user_name(user&.name)
+  json.finished(project.school_project&.finished)
 end
diff --git a/app/views/api/projects/show.json.jbuilder b/app/views/api/projects/show.json.jbuilder
@@ -42,5 +42,3 @@ json.audio(@project.audio) do |audio_file|
 end
 
 json.user_name(@user&.name) if @user.present? && @project.parent
-
-json.finished(@project.finished) if @project.school.present? && @project.remixed_from_id.present?
diff --git a/app/views/api/school_projects/finished.json.jbuilder b/app/views/api/school_projects/finished.json.jbuilder
@@ -0,0 +1,11 @@
+# frozen_string_literal: true
+
+json.call(
+  @school_project,
+  :id,
+  :school_id,
+  :project_id,
+  :finished
+)
+
+json.identifier(@school_project.project.identifier)
diff --git a/config/routes.rb b/config/routes.rb
@@ -33,7 +33,8 @@
     end
 
     resources :projects, only: %i[index show update destroy create] do
-      put :toggle_finished, on: :member, to: 'projects#toggle_finished'
+      get :finished, on: :member, to: 'school_projects#show_finished'
+      put :finished, on: :member, to: 'school_projects#set_finished'
       resource :remix, only: %i[show create], controller: 'projects/remixes'
       resources :remixes, only: %i[index], controller: 'projects/remixes'
       resource :images, only: %i[show create], controller: 'projects/images'
diff --git a/db/migrate/20250206154935_create_school_projects.rb b/db/migrate/20250206154935_create_school_projects.rb
@@ -0,0 +1,10 @@
+class CreateSchoolProjects < ActiveRecord::Migration[7.1]
+  def change
+    create_table :school_projects, id: :uuid do |t|
+      t.references :school, foreign_key: true, type: :uuid
+      t.references :project, null: false, foreign_key: true, type: :uuid
+      t.boolean :finished, default: false
+      t.timestamps
+    end
+  end
+end
diff --git a/db/migrate/20250206160256_generate_school_projects.rb b/db/migrate/20250206160256_generate_school_projects.rb
@@ -0,0 +1,13 @@
+class GenerateSchoolProjects < ActiveRecord::Migration[7.1]
+  def up
+    Project.all.each do |project|
+      if project.school.present?
+        SchoolProject.create!(school: project.school, project:)
+      end
+    end
+  end
+
+  def down
+    SchoolProject.destroy_all
+  end
+end
diff --git a/db/migrate/20250207095204_remove_finished_column_from_projects.rb b/db/migrate/20250207095204_remove_finished_column_from_projects.rb
@@ -0,0 +1,19 @@
+class RemoveFinishedColumnFromProjects < ActiveRecord::Migration[7.1]
+  def up
+    Project.all.each do |project|
+      if project.school_project.present?
+        project.school_project.update!(finished: project.finished)
+      end
+    end
+    remove_column :projects, :finished
+  end
+
+  def down
+    add_column :projects, :finished, :boolean, default: false
+    Project.all.each do |project|
+      if project.school_project.present?
+        project.update!(finished: project.school_project.finished)
+      end
+    end
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
diff --git a/lib/concepts/project/operations/toggle_finished.rb b/lib/concepts/project/operations/toggle_finished.rb
diff --git a/lib/concepts/school_project/set_finished.rb b/lib/concepts/school_project/set_finished.rb
@@ -0,0 +1,19 @@
+# frozen_string_literal: true
+
+class SchoolProject
+  class SetFinished
+    class << self
+      def call(school_project:, finished:)
+        response = OperationResponse.new
+        response[:school_project] = school_project
+        response[:school_project].assign_attributes(finished:)
+        response[:school_project].save!
+        response
+      rescue StandardError => e
+        Sentry.capture_exception(e)
+        response[:error] = response[:school_project]&.errors
+        response
+      end
+    end
+  end
+end
diff --git a/spec/factories/project.rb b/spec/factories/project.rb
@@ -8,6 +8,19 @@
     project_type { 'python' }
     locale { %w[en es-LA fr-FR].sample }
 
+    transient do
+      # school { nil }
+      # school_id { nil }
+      finished { nil }
+    end
+
+    after(:create) do |project, evaluator|
+      if evaluator.finished.present?
+        project.school_project.finished = evaluator.finished
+        project.school_project.save!
+      end
+    end
+
     trait :with_components do
       transient do
         component_count { 1 }
diff --git a/spec/factories/school_projects.rb b/spec/factories/school_projects.rb
@@ -0,0 +1,8 @@
+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :school_project do
+    school_id { SecureRandom.uuid }
+    project_id { SecureRandom.uuid }
+  end
+end
diff --git a/spec/models/ability_spec.rb b/spec/models/ability_spec.rb
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
diff --git a/spec/models/school_project_spec.rb b/spec/models/school_project_spec.rb
diff --git a/spec/requests/projects/show_spec.rb b/spec/requests/projects/show_spec.rb
diff --git a/spec/requests/school_projects/set_finished_spec.rb b/spec/requests/school_projects/set_finished_spec.rb
diff --git a/spec/requests/school_projects/show_finished_spec.rb b/spec/requests/school_projects/show_finished_spec.rb

Original file line number	Diff line number	Diff line change
`@@ -12,4 +12,5 @@ json.array!(@projects_with_users) do \|project, user\|`
`12`	`12`	`)`
`13`	`13`
`14`	`14`	`json.user_name(user&.name)`
	`15`	`+ json.finished(project.school_project&.finished)`
`15`	`16`	`end`