Create models and controllers for lessons (#258)

Author: floehopper

app/controllers/api/lessons_controller.rb

@@ -0,0 +1,103 @@
+# frozen_string_literal: true
+
+module Api
+  class LessonsController < ApiController
+    before_action :authorize_user, except: %i[index show]
+    before_action :verify_school_class_belongs_to_school, only: :create
+    load_and_authorize_resource :lesson
+
+    def index
+      scope = params[:include_archived] == 'true' ? Lesson : Lesson.unarchived
+      @lessons_with_users = scope.accessible_by(current_ability).with_users
+      render :index, formats: [:json], status: :ok
+    end
+
+    def show
+      @lesson_with_user = @lesson.with_user
+      render :show, formats: [:json], status: :ok
+    end
+
+    def create
+      result = Lesson::Create.call(lesson_params:)
+
+      if result.success?
+        @lesson_with_user = result[:lesson].with_user
+        render :show, formats: [:json], status: :created
+      else
+        render json: { error: result[:error] }, status: :unprocessable_entity
+      end
+    end
+
+    def create_copy
+      result = Lesson::CreateCopy.call(lesson: @lesson, lesson_params:)
+
+      if result.success?
+        @lesson_with_user = result[:lesson].with_user
+        render :show, formats: [:json], status: :created
+      else
+        render json: { error: result[:error] }, status: :unprocessable_entity
+      end
+    end
+
+    def update
+      result = Lesson::Update.call(lesson: @lesson, lesson_params:)
+
+      if result.success?
+        @lesson_with_user = result[:lesson].with_user
+        render :show, formats: [:json], status: :ok
+      else
+        render json: { error: result[:error] }, status: :unprocessable_entity
+      end
+    end
+
+    def destroy
+      operation = params[:undo] == 'true' ? Lesson::Unarchive : Lesson::Archive
+      result = operation.call(lesson: @lesson)
+
+      if result.success?
+        head :no_content
+      else
+        render json: { error: result[:error] }, status: :unprocessable_entity
+      end
+    end
+
+    private
+
+    def verify_school_class_belongs_to_school
+      return if base_params[:school_class_id].blank?
+      return if school&.classes&.pluck(:id)&.include?(base_params[:school_class_id])
+
+      raise ParameterError, 'school_class_id does not correspond to school_id'
+    end
+
+    def lesson_params
+      if school_owner?
+        # A school owner must specify who the lesson user is.
+        base_params
+      else
+        # A school teacher may only create lessons they own.
+        base_params.merge(user_id: current_user.id)
+      end
+    end
+
+    def base_params
+      params.fetch(:lesson, {}).permit(
+        :school_id,
+        :school_class_id,
+        :user_id,
+        :name,
+        :description,
+        :visibility,
+        :due_date
+      )
+    end
+
+    def school_owner?
+      school && current_user.school_owner?(organisation_id: school.id)
+    end
+
+    def school
+      @school ||= @lesson&.school || School.find_by(id: base_params[:school_id])
+    end
+  end
+end

app/controllers/api/projects_controller.rb

@@ -7,9 +7,9 @@ class ProjectsController < ApiController
     before_action :authorize_user, only: %i[create update index destroy]
     before_action :load_project, only: %i[show update destroy]
     before_action :load_projects, only: %i[index]
-    after_action :pagination_link_header, only: [:index]
     load_and_authorize_resource
-    skip_load_resource only: :create
+    before_action :verify_lesson_belongs_to_school, only: :create
+    after_action :pagination_link_header, only: %i[index]
 
     def index
       @paginated_projects = @projects.page(params[:page])
@@ -21,25 +21,23 @@ def show
     end
 
     def create
-      project_hash = project_params.merge(user_id: current_user&.id)
-      result = Project::Create.call(project_hash:)
+      result = Project::Create.call(project_hash: project_params)
 
       if result.success?
         @project = result[:project]
-        render :show, formats: [:json]
+        render :show, formats: [:json], status: :created
       else
-        render json: { error: result[:error] }, status: :internal_server_error
+        render json: { error: result[:error] }, status: :unprocessable_entity
       end
     end
 
     def update
-      update_hash = project_params.merge(user_id: current_user&.id)
-      result = Project::Update.call(project: @project, update_hash:)
+      result = Project::Update.call(project: @project, update_hash: project_params)
 
       if result.success?
         render :show, formats: [:json]
       else
-        render json: { error: result[:error] }, status: :bad_request
+        render json: { error: result[:error] }, status: :unprocessable_entity
       end
     end
 
@@ -50,6 +48,13 @@ def destroy
 
     private
 
+    def verify_lesson_belongs_to_school
+      return if base_params[:lesson_id].blank?
+      return if school&.lessons&.pluck(:id)&.include?(base_params[:lesson_id])
+
+      raise ParameterError, 'lesson_id does not correspond to school_id'
+    end
+
     def load_project
       project_loader = ProjectLoader.new(params[:id], [params[:locale]])
       @project = project_loader.load
@@ -60,18 +65,38 @@ def load_projects
     end
 
     def project_params
+      if school_owner?
+        # A school owner must specify who the project user is.
+        base_params
+      else
+        # A school teacher may only create projects they own.
+        base_params.merge(user_id: current_user&.id)
+      end
+    end
+
+    def base_params
       params.fetch(:project, {}).permit(
+        :school_id,
+        :lesson_id,
+        :user_id,
         :identifier,
         :name,
         :project_type,
         :locale,
         {
-          image_list: [],
           components: %i[id name extension content index default]
         }
       )
     end
 
+    def school_owner?
+      school && current_user.school_owner?(organisation_id: school.id)
+    end
+
+    def school
+      @school ||= @project&.school || School.find_by(id: base_params[:school_id])
+    end
+
     def pagination_link_header
       pagination_links = []
       pagination_links << page_links(first_page, 'first')

app/controllers/api/school_classes_controller.rb

@@ -53,7 +53,7 @@ def destroy
 
     def school_class_params
       if school_owner?
-        # The school owner must specify who the class teacher is.
+        # A school owner must specify who the class teacher is.
         params.require(:school_class).permit(:teacher_id, :name)
       else
         # A school teacher may only create classes they own.

app/controllers/api_controller.rb

@@ -1,29 +1,36 @@
 # frozen_string_literal: true
 
 class ApiController < ActionController::API
+  class ::ParameterError < StandardError; end
+
   include Identifiable
 
   unless Rails.application.config.consider_all_requests_local
     rescue_from ActionController::ParameterMissing, with: -> { bad_request }
     rescue_from ActiveRecord::RecordNotFound, with: -> { not_found }
     rescue_from CanCan::AccessDenied, with: -> { denied }
+    rescue_from ParameterError, with: -> { unprocessable }
   end
 
   private
 
+  def bad_request
+    head :bad_request # 400 status
+  end
+
   def authorize_user
-    head :unauthorized unless current_user
+    head :unauthorized unless current_user # 401 status
   end
 
-  def bad_request
-    head :bad_request
+  def denied
+    head :forbidden # 403 status
   end
 
   def not_found
-    head :not_found
+    head :not_found # 404 status
   end
 
-  def denied
-    head :forbidden
+  def unprocessable
+    head :unprocessable_entity # 422 status
   end
 end

app/models/ability.rb

@@ -3,43 +3,92 @@
 class Ability
   include CanCan::Ability
 
-  # rubocop:disable Metrics/AbcSize, Layout/LineLength
+  # rubocop:disable Metrics/AbcSize
   def initialize(user)
-    can :show, Project, user_id: nil
-    can :show, Component, project: { user_id: nil }
+    # Anyone can view projects not owner by a user or a school.
+    can :show, Project, user_id: nil, school_id: nil
+    can :show, Component, project: { user_id: nil, school_id: nil }
+
+    # Anyone can read publicly shared lessons.
+    can :read, Lesson, visibility: 'public'
 
     return unless user
 
-    can %i[read create update destroy], Project, user_id: user.id
-    can %i[read create update destroy], Component, project: { user_id: user.id }
+    # Any authenticated user can create projects not owned by a school.
+    can :create, Project, user_id: user.id, school_id: nil
+    can :create, Component, project: { user_id: user.id, school_id: nil }
+
+    # Any authenticated user can manage their own projects.
+    can %i[read update destroy], Project, user_id: user.id
+    can %i[read update destroy], Component, project: { user_id: user.id }
+
+    # Any authenticated user can create a school. They agree to become the school-owner.
+    can :create, School
+
+    # Any authenticated user can create a lesson, to support a RPF library of public lessons.
+    can :create, Lesson, school_id: nil, school_class_id: nil
+
+    # Any authenticated user can create a copy of a publicly shared lesson.
+    can :create_copy, Lesson, visibility: 'public'
 
-    can %i[create], School # The user agrees to become a school-owner by creating a school.
+    # Any authenticated user can manage their own lessons.
+    can %i[read create_copy update destroy], Lesson, user_id: user.id
 
     user.organisation_ids.each do |organisation_id|
-      if user.school_owner?(organisation_id:)
-        can(%i[read update destroy], School, id: organisation_id)
-        can(%i[read create update destroy], SchoolClass, school: { id: organisation_id })
-        can(%i[read create destroy], ClassMember, school_class: { school: { id: organisation_id } })
-        can(%i[read create destroy], :school_owner)
-        can(%i[read create destroy], :school_teacher)
-        can(%i[read create create_batch update destroy], :school_student)
-      end
-
-      if user.school_teacher?(organisation_id:)
-        can(%i[read], School, id: organisation_id)
-        can(%i[create], SchoolClass, school: { id: organisation_id })
-        can(%i[read update destroy], SchoolClass, school: { id: organisation_id }, teacher_id: user.id)
-        can(%i[read create destroy], ClassMember, school_class: { school: { id: organisation_id }, teacher_id: user.id })
-        can(%i[read], :school_owner)
-        can(%i[read], :school_teacher)
-        can(%i[read create create_batch update], :school_student)
-      end
-
-      if user.school_student?(organisation_id:)
-        can(%i[read], School, id: organisation_id)
-        can(%i[read], SchoolClass, school: { id: organisation_id }, members: { student_id: user.id })
-      end
+      define_school_owner_abilities(organisation_id:) if user.school_owner?(organisation_id:)
+      define_school_teacher_abilities(user:, organisation_id:) if user.school_teacher?(organisation_id:)
+      define_school_student_abilities(user:, organisation_id:) if user.school_student?(organisation_id:)
     end
   end
-  # rubocop:enable Metrics/AbcSize, Layout/LineLength
+  # rubocop:enable Metrics/AbcSize
+
+  private
+
+  def define_school_owner_abilities(organisation_id:)
+    can(%i[read update destroy], School, id: organisation_id)
+    can(%i[read create update destroy], SchoolClass, school: { id: organisation_id })
+    can(%i[read create destroy], ClassMember, school_class: { school: { id: organisation_id } })
+    can(%i[read create destroy], :school_owner)
+    can(%i[read create destroy], :school_teacher)
+    can(%i[read create create_batch update destroy], :school_student)
+    can(%i[create create_copy], Lesson, school_id: organisation_id)
+    can(%i[read update destroy], Lesson, school_id: organisation_id, visibility: %w[teachers students public])
+    can(%i[create], Project, school_id: organisation_id)
+  end
+
+  def define_school_teacher_abilities(user:, organisation_id:)
+    can(%i[read], School, id: organisation_id)
+    can(%i[create], SchoolClass, school: { id: organisation_id })
+    can(%i[read update destroy], SchoolClass, school: { id: organisation_id }, teacher_id: user.id)
+    can(%i[read create destroy], ClassMember, school_class: { school: { id: organisation_id }, teacher_id: user.id })
+    can(%i[read], :school_owner)
+    can(%i[read], :school_teacher)
+    can(%i[read create create_batch update], :school_student)
+    can(%i[create destroy], Lesson) { |lesson| school_teacher_can_manage_lesson?(user:, organisation_id:, lesson:) }
+    can(%i[read create_copy], Lesson, school_id: organisation_id, visibility: %w[teachers students])
+    can(%i[create], Project) { |project| school_teacher_can_manage_project?(user:, organisation_id:, project:) }
+  end
+
+  # rubocop:disable Layout/LineLength
+  def define_school_student_abilities(user:, organisation_id:)
+    can(%i[read], School, id: organisation_id)
+    can(%i[read], SchoolClass, school: { id: organisation_id }, members: { student_id: user.id })
+    can(%i[read], Lesson, school_id: organisation_id, visibility: 'students', school_class: { members: { student_id: user.id } })
+    can(%i[create], Project, school_id: organisation_id, user_id: user.id, lesson_id: nil)
+  end
+  # rubocop:enable Layout/LineLength
+
+  def school_teacher_can_manage_lesson?(user:, organisation_id:, lesson:)
+    is_my_lesson = lesson.school_id == organisation_id && lesson.user_id == user.id
+    is_my_class = lesson.school_class && lesson.school_class.teacher_id == user.id
+
+    is_my_lesson && (is_my_class || !lesson.school_class)
+  end
+
+  def school_teacher_can_manage_project?(user:, organisation_id:, project:)
+    is_my_project = project.school_id == organisation_id && project.user_id == user.id
+    is_my_lesson = project.lesson && project.lesson.user_id == user.id
+
+    is_my_project && (is_my_lesson || !project.lesson)
+  end
 end

app/models/class_member.rb

@@ -16,8 +16,8 @@ def self.students
   end
 
   def self.with_students
-    users = students.index_by(&:id)
-    all.map { |instance| [instance, users[instance.student_id]] }
+    by_id = students.index_by(&:id)
+    all.map { |instance| [instance, by_id[instance.student_id]] }
   end
 
   def with_student