diff --git a/app/controllers/api/lessons_controller.rb b/app/controllers/api/lessons_controller.rb new file mode 100644 index 000000000..244ef1214 --- /dev/null +++ b/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 diff --git a/app/controllers/api/projects_controller.rb b/app/controllers/api/projects_controller.rb index 9e2dd6995..2d9807a26 100644 --- a/app/controllers/api/projects_controller.rb +++ b/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,7 +65,20 @@ 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, @@ -72,6 +90,14 @@ def project_params ) 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') diff --git a/app/controllers/api/school_classes_controller.rb b/app/controllers/api/school_classes_controller.rb index 20ab093c3..8be892a22 100644 --- a/app/controllers/api/school_classes_controller.rb +++ b/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. diff --git a/app/controllers/api_controller.rb b/app/controllers/api_controller.rb index 9c1a4f5ba..737d382dd 100644 --- a/app/controllers/api_controller.rb +++ b/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 diff --git a/app/models/ability.rb b/app/models/ability.rb index 5f87d13af..504b050a4 100644 --- a/app/models/ability.rb +++ b/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 diff --git a/app/models/class_member.rb b/app/models/class_member.rb index 4f7abbf0f..217e2d4b1 100644 --- a/app/models/class_member.rb +++ b/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 diff --git a/app/models/lesson.rb b/app/models/lesson.rb new file mode 100644 index 000000000..553728bbc --- /dev/null +++ b/app/models/lesson.rb @@ -0,0 +1,80 @@ +# frozen_string_literal: true + +class Lesson < ApplicationRecord + belongs_to :school, optional: true + belongs_to :school_class, optional: true + belongs_to :parent, optional: true, class_name: :Lesson, foreign_key: :copied_from_id, inverse_of: :copies + has_many :copies, dependent: :nullify, class_name: :Lesson, foreign_key: :copied_from_id, inverse_of: :parent + has_many :projects, dependent: :nullify + + before_validation :assign_school_from_school_class + before_destroy -> { throw :abort } + + validates :user_id, presence: true + validates :name, presence: true + validates :visibility, presence: true, inclusion: { in: %w[private teachers students public] } + + validate :user_has_the_school_owner_or_school_teacher_role_for_the_school + validate :user_is_the_school_teacher_for_the_school_class + + scope :archived, -> { where.not(archived_at: nil) } + scope :unarchived, -> { where(archived_at: nil) } + + def self.users + User.from_userinfo(ids: pluck(:user_id)) + end + + def self.with_users + by_id = users.index_by(&:id) + all.map { |instance| [instance, by_id[instance.user_id]] } + end + + def with_user + [self, User.from_userinfo(ids: user_id).first] + end + + def archived? + archived_at.present? + end + + def archive! + return if archived? + + self.archived_at = Time.now.utc + save!(validate: false) + end + + def unarchive! + return unless archived? + + self.archived_at = nil + save!(validate: false) + end + + private + + def assign_school_from_school_class + self.school ||= school_class&.school + end + + # rubocop:disable Metrics/AbcSize + def user_has_the_school_owner_or_school_teacher_role_for_the_school + return unless user_id_changed? && errors.blank? && school + + _, user = with_user + + return if user.blank? + return if user.school_owner?(organisation_id: school.id) + return if user.school_teacher?(organisation_id: school.id) + + msg = "'#{user_id}' does not have the 'school-owner' or 'school-teacher' role for organisation '#{school.id}'" + errors.add(:user, msg) + end + # rubocop:enable Metrics/AbcSize + + def user_is_the_school_teacher_for_the_school_class + return if !school_class || user_id == school_class.teacher_id + + errors.add(:user, "'#{user_id}' is not the 'school-teacher' for school_class '#{school_class.id}'") + end +end diff --git a/app/models/project.rb b/app/models/project.rb index 4500590d0..cc39a48e7 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1,17 +1,43 @@ # frozen_string_literal: true class Project < ApplicationRecord - before_validation :check_unique_not_null, on: :create - validates :identifier, presence: true, uniqueness: { scope: :locale } - validate :identifier_cannot_be_taken_by_another_user - validates :locale, presence: true, unless: :user_id - belongs_to :parent, class_name: 'Project', foreign_key: 'remixed_from_id', optional: true, inverse_of: :remixes + belongs_to :school, optional: true + belongs_to :lesson, optional: true + belongs_to :parent, optional: true, class_name: :Project, foreign_key: :remixed_from_id, inverse_of: :remixes + has_many :remixes, dependent: :nullify, class_name: :Project, foreign_key: :remixed_from_id, inverse_of: :parent has_many :components, -> { order(default: :desc, name: :asc) }, dependent: :destroy, inverse_of: :project - has_many :remixes, class_name: 'Project', foreign_key: 'remixed_from_id', dependent: :nullify, inverse_of: :parent has_many :project_errors, dependent: :nullify has_many_attached :images + accepts_nested_attributes_for :components + before_validation :check_unique_not_null, on: :create + + 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_the_owner_of_the_lesson + + def self.users + User.from_userinfo(ids: pluck(:user_id)) + end + + def self.with_users + by_id = users.index_by(&:id) + all.map { |instance| [instance, by_id[instance.user_id]] } + end + + def with_user + [self, User.from_userinfo(ids: user_id).first] + end + + # Work around a CanCanCan issue with accepts_nested_attributes_for. + # https://github.com/CanCanCommunity/cancancan/issues/774 + def components=(array) + super(array.map { |o| o.is_a?(Hash) ? Component.new(o) : o }) + end + private def check_unique_not_null @@ -23,4 +49,22 @@ def identifier_cannot_be_taken_by_another_user errors.add(:identifier, "can't be taken by another user") end + + def user_has_a_role_within_the_school + return unless user_id_changed? && errors.blank? && school + + _, user = with_user + + return if user.blank? + return if user.roles(organisation_id: school_id).any? + + msg = "'#{user_id}' does not have any roles for for organisation '#{school_id}'" + errors.add(:user, msg) + end + + def user_is_the_owner_of_the_lesson + return if !lesson || user_id == lesson.user_id + + errors.add(:user, "'#{user_id}' is not the owner for lesson '#{lesson_id}'") + end end diff --git a/app/models/school.rb b/app/models/school.rb index d976d3432..0014cb1f6 100644 --- a/app/models/school.rb +++ b/app/models/school.rb @@ -2,6 +2,8 @@ class School < ApplicationRecord has_many :classes, class_name: :SchoolClass, inverse_of: :school, dependent: :destroy + has_many :lessons, dependent: :nullify + has_many :projects, dependent: :nullify validates :id, presence: true, uniqueness: { case_sensitive: false } validates :name, presence: true diff --git a/app/models/school_class.rb b/app/models/school_class.rb index 581dd60b2..2f8788111 100644 --- a/app/models/school_class.rb +++ b/app/models/school_class.rb @@ -3,6 +3,7 @@ class SchoolClass < ApplicationRecord belongs_to :school has_many :members, class_name: :ClassMember, inverse_of: :school_class, dependent: :destroy + has_many :lessons, dependent: :nullify validates :teacher_id, presence: true validates :name, presence: true @@ -13,8 +14,8 @@ def self.teachers end def self.with_teachers - users = teachers.index_by(&:id) - all.map { |instance| [instance, users[instance.teacher_id]] } + by_id = teachers.index_by(&:id) + all.map { |instance| [instance, by_id[instance.teacher_id]] } end def with_teacher diff --git a/app/models/user.rb b/app/models/user.rb index d58ffbc41..38105a316 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -29,9 +29,12 @@ def organisation_ids organisations&.keys || [] end + def roles(organisation_id:) + organisations[organisation_id.to_s]&.to_s&.split(',')&.map(&:strip) || [] + end + def role?(organisation_id:, role:) - roles = organisations[organisation_id.to_s] - roles.to_s.split(',').map(&:strip).include?(role.to_s) if roles + roles(organisation_id:).include?(role.to_s) end def school_owner?(organisation_id:) diff --git a/app/views/api/lessons/index.json.jbuilder b/app/views/api/lessons/index.json.jbuilder new file mode 100644 index 000000000..db5d83d18 --- /dev/null +++ b/app/views/api/lessons/index.json.jbuilder @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +json.array!(@lessons_with_users) do |lesson, user| + json.call( + lesson, + :id, + :school_id, + :school_class_id, + :copied_from_id, + :user_id, + :name, + :description, + :visibility, + :due_date, + :archived_at, + :created_at, + :updated_at + ) + + json.user_name(user&.name) +end diff --git a/app/views/api/lessons/show.json.jbuilder b/app/views/api/lessons/show.json.jbuilder new file mode 100644 index 000000000..d8cbddd5d --- /dev/null +++ b/app/views/api/lessons/show.json.jbuilder @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +lesson, user = @lesson_with_user + +json.call( + lesson, + :id, + :school_id, + :school_class_id, + :copied_from_id, + :user_id, + :name, + :description, + :visibility, + :due_date, + :archived_at, + :created_at, + :updated_at +) + +json.user_name(user&.name) diff --git a/app/views/api/projects/index.json.jbuilder b/app/views/api/projects/index.json.jbuilder index f29fe2640..654cb5763 100644 --- a/app/views/api/projects/index.json.jbuilder +++ b/app/views/api/projects/index.json.jbuilder @@ -1,3 +1,10 @@ # frozen_string_literal: true -json.array! @paginated_projects, :identifier, :project_type, :name, :user_id, :updated_at +json.array!( + @paginated_projects, + :identifier, + :project_type, + :name, + :user_id, + :updated_at +) diff --git a/app/views/api/projects/show.json.jbuilder b/app/views/api/projects/show.json.jbuilder index 4f1803aa6..fc663d076 100644 --- a/app/views/api/projects/show.json.jbuilder +++ b/app/views/api/projects/show.json.jbuilder @@ -1,12 +1,31 @@ # frozen_string_literal: true -json.call(@project, :identifier, :project_type, :locale, :name, :user_id) +json.call( + @project, + :identifier, + :project_type, + :locale, + :name, + :user_id +) -json.parent(@project.parent, :name, :identifier) if @project.parent +if @project.parent + json.parent( + @project.parent, + :name, + :identifier + ) +end -json.components @project.components, :id, :name, :extension, :content +json.components( + @project.components, + :id, + :name, + :extension, + :content +) -json.image_list @project.images do |image| - json.filename image.filename - json.url rails_blob_url(https://github.com/image) +json.image_list(@project.images) do |image| + json.filename(image.filename) + json.url(https://github.com/rails_blob_url(image)) end diff --git a/config/routes.rb b/config/routes.rb index 94b904f74..f2654e0d9 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true +# rubocop:disable Metrics/BlockLength Rails.application.routes.draw do post '/graphql', to: 'graphql#execute' mount GraphiQL::Rails::Engine, at: '/', graphql_path: '/graphql#execute' unless Rails.env.production? @@ -28,7 +29,12 @@ post :batch, on: :collection, to: 'school_students#create_batch' end end + + resources :lessons, only: %i[index create show update destroy] do + post :copy, on: :member, to: 'lessons#create_copy' + end end resource :github_webhooks, only: :create, defaults: { formats: :json } end +# rubocop:enable Metrics/BlockLength diff --git a/db/migrate/20240217144009_create_lessons.rb b/db/migrate/20240217144009_create_lessons.rb new file mode 100644 index 000000000..7de6e4207 --- /dev/null +++ b/db/migrate/20240217144009_create_lessons.rb @@ -0,0 +1,23 @@ +class CreateLessons < ActiveRecord::Migration[7.0] + def change + create_table :lessons, id: :uuid do |t| + t.references :school, type: :uuid, foreign_key: true, index: true + t.references :school_class, type: :uuid, foreign_key: true, index: true + t.references :copied_from, type: :uuid, foreign_key: { to_table: :lessons }, index: true + + t.uuid :user_id, null: false + t.string :name, null: false + t.string :description + t.string :visibility, null: false, default: 'private' + + t.datetime :due_date + t.datetime :archived_at + t.timestamps + end + + add_index :lessons, :user_id + add_index :lessons, :name + add_index :lessons, :visibility + add_index :lessons, :archived_at + end +end diff --git a/db/migrate/20240223113155_add_school_id_to_projects.rb b/db/migrate/20240223113155_add_school_id_to_projects.rb new file mode 100644 index 000000000..7af93cefd --- /dev/null +++ b/db/migrate/20240223113155_add_school_id_to_projects.rb @@ -0,0 +1,5 @@ +class AddSchoolIdToProjects < ActiveRecord::Migration[7.0] + def change + add_reference :projects, :school, type: :uuid, foreign_key: true, index: true + end +end diff --git a/db/migrate/20240223150228_add_lesson_id_to_projects.rb b/db/migrate/20240223150228_add_lesson_id_to_projects.rb new file mode 100644 index 000000000..85be34945 --- /dev/null +++ b/db/migrate/20240223150228_add_lesson_id_to_projects.rb @@ -0,0 +1,5 @@ +class AddLessonIdToProjects < ActiveRecord::Migration[7.0] + def change + add_reference :projects, :lesson, type: :uuid, foreign_key: true, index: true + end +end diff --git a/db/schema.rb b/db/schema.rb index 739dce6d3..15e3bf2ad 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.0].define(version: 2024_02_01_171700) do +ActiveRecord::Schema[7.0].define(version: 2024_02_23_150228) do # These are extensions that must be enabled in order to support this database enable_extension "pg_stat_statements" enable_extension "pgcrypto" @@ -124,6 +124,27 @@ t.index ["scheduled_at"], name: "index_good_jobs_on_scheduled_at", where: "(finished_at IS NULL)" end + create_table "lessons", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| + t.uuid "school_id" + t.uuid "school_class_id" + t.uuid "copied_from_id" + t.uuid "user_id", null: false + t.string "name", null: false + t.string "description" + t.string "visibility", default: "private", null: false + t.datetime "due_date" + t.datetime "archived_at" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["archived_at"], name: "index_lessons_on_archived_at" + t.index ["copied_from_id"], name: "index_lessons_on_copied_from_id" + t.index ["name"], name: "index_lessons_on_name" + t.index ["school_class_id"], name: "index_lessons_on_school_class_id" + t.index ["school_id"], name: "index_lessons_on_school_id" + t.index ["user_id"], name: "index_lessons_on_user_id" + t.index ["visibility"], name: "index_lessons_on_visibility" + end + create_table "project_errors", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| t.uuid "project_id" t.string "error", null: false @@ -144,9 +165,38 @@ t.uuid "remixed_from_id" t.string "locale" t.string "remix_origin" + t.uuid "school_id" + t.uuid "lesson_id" t.index ["identifier", "locale"], name: "index_projects_on_identifier_and_locale", unique: true t.index ["identifier"], name: "index_projects_on_identifier" + t.index ["lesson_id"], name: "index_projects_on_lesson_id" t.index ["remixed_from_id"], name: "index_projects_on_remixed_from_id" + t.index ["school_id"], name: "index_projects_on_school_id" + end + + create_table "school_classes", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| + t.uuid "school_id", null: false + t.uuid "teacher_id", null: false + t.string "name", null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["school_id", "teacher_id"], name: "index_school_classes_on_school_id_and_teacher_id" + t.index ["school_id"], name: "index_school_classes_on_school_id" + end + + create_table "schools", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| + t.string "name", null: false + t.string "reference" + t.string "address_line_1", null: false + t.string "address_line_2" + t.string "municipality", null: false + t.string "administrative_area" + t.string "postal_code" + t.string "country_code", null: false + t.datetime "verified_at" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["reference"], name: "index_schools_on_reference", unique: true end create_table "school_classes", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| @@ -183,6 +233,11 @@ add_foreign_key "active_storage_variant_records", "active_storage_blobs", column: "blob_id" add_foreign_key "class_members", "school_classes" add_foreign_key "components", "projects" + add_foreign_key "lessons", "lessons", column: "copied_from_id" + add_foreign_key "lessons", "school_classes" + add_foreign_key "lessons", "schools" add_foreign_key "project_errors", "projects" + add_foreign_key "projects", "lessons" + add_foreign_key "projects", "schools" add_foreign_key "school_classes", "schools" end diff --git a/lib/concepts/lesson/operations/archive.rb b/lib/concepts/lesson/operations/archive.rb new file mode 100644 index 000000000..3bd8bd547 --- /dev/null +++ b/lib/concepts/lesson/operations/archive.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class Lesson + class Archive + class << self + def call(lesson:) + response = OperationResponse.new + lesson.archive! + response + rescue StandardError => e + Sentry.capture_exception(e) + response[:error] = "Error archiving lesson: #{e}" + response + end + end + end +end diff --git a/lib/concepts/lesson/operations/create.rb b/lib/concepts/lesson/operations/create.rb new file mode 100644 index 000000000..a078ea8d6 --- /dev/null +++ b/lib/concepts/lesson/operations/create.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +class Lesson + class Create + class << self + def call(lesson_params:) + response = OperationResponse.new + response[:lesson] = Lesson.new(lesson_params) + response[:lesson].save! + response + rescue StandardError => e + Sentry.capture_exception(e) + errors = response[:lesson].errors.full_messages.join(',') + response[:error] = "Error creating lesson: #{errors}" + response + end + end + end +end diff --git a/lib/concepts/lesson/operations/create_copy.rb b/lib/concepts/lesson/operations/create_copy.rb new file mode 100644 index 000000000..0906bf24c --- /dev/null +++ b/lib/concepts/lesson/operations/create_copy.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +class Lesson + class CreateCopy + class << self + def call(lesson:, lesson_params:) + response = OperationResponse.new + response[:lesson] = build_copy(lesson, lesson_params) + response[:lesson].save! + response + rescue StandardError => e + Sentry.capture_exception(e) + errors = response[:lesson].errors.full_messages.join(',') + response[:error] = "Error creating copy of lesson: #{errors}" + response + end + + private + + # TODO: copy projects + def build_copy(lesson, lesson_params) + copy = Lesson.new(parent: lesson, name: lesson.name, description: lesson.description) + copy.assign_attributes(lesson_params) + copy + end + end + end +end diff --git a/lib/concepts/lesson/operations/unarchive.rb b/lib/concepts/lesson/operations/unarchive.rb new file mode 100644 index 000000000..f7c3170dc --- /dev/null +++ b/lib/concepts/lesson/operations/unarchive.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class Lesson + class Unarchive + class << self + def call(lesson:) + response = OperationResponse.new + lesson.unarchive! + response + rescue StandardError => e + Sentry.capture_exception(e) + response[:error] = "Error unarchiving lesson: #{e}" + response + end + end + end +end diff --git a/lib/concepts/lesson/operations/update.rb b/lib/concepts/lesson/operations/update.rb new file mode 100644 index 000000000..8f4de6c02 --- /dev/null +++ b/lib/concepts/lesson/operations/update.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +class Lesson + class Update + class << self + def call(lesson:, lesson_params:) + response = OperationResponse.new + response[:lesson] = lesson + response[:lesson].assign_attributes(lesson_params) + response[:lesson].save! + response + rescue StandardError => e + Sentry.capture_exception(e) + errors = response[:lesson].errors.full_messages.join(',') + response[:error] = "Error updating lesson: #{errors}" + response + end + end + end +end diff --git a/spec/concepts/lesson/archive_spec.rb b/spec/concepts/lesson/archive_spec.rb new file mode 100644 index 000000000..09bc40e2c --- /dev/null +++ b/spec/concepts/lesson/archive_spec.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Lesson::Archive, type: :unit do + before do + stub_user_info_api + end + + let(:lesson) { create(:lesson) } + + it 'returns a successful operation response' do + response = described_class.call(lesson:) + expect(response.success?).to be(true) + end + + it 'archives the lesson' do + described_class.call(lesson:) + expect(lesson.reload.archived?).to be(true) + end +end diff --git a/spec/concepts/lesson/create_copy_spec.rb b/spec/concepts/lesson/create_copy_spec.rb new file mode 100644 index 000000000..5fff2b43d --- /dev/null +++ b/spec/concepts/lesson/create_copy_spec.rb @@ -0,0 +1,84 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Lesson::CreateCopy, type: :unit do + let(:teacher_index) { user_index_by_role('school-teacher') } + let(:teacher_id) { user_id_by_index(teacher_index) } + + let!(:lesson) do + create(:lesson, name: 'Test Lesson', description: 'Description', user_id: teacher_id) + end + + let(:lesson_params) do + { user_id: teacher_id } + end + + it 'returns a successful operation response' do + response = described_class.call(lesson:, lesson_params:) + expect(response.success?).to be(true) + end + + it 'creates a lesson' do + expect { described_class.call(lesson:, lesson_params:) }.to change(Lesson, :count).by(1) + end + + it 'returns the new lesson in the operation response' do + response = described_class.call(lesson:, lesson_params:) + expect(response[:lesson]).to be_a(Lesson) + end + + it 'assigns the parent' do + response = described_class.call(lesson:, lesson_params:) + expect(response[:lesson].parent).to eq(lesson) + end + + it 'assigns the name from the parent lesson' do + response = described_class.call(lesson:, lesson_params:) + expect(response[:lesson].name).to eq('Test Lesson') + end + + it 'assigns the description from the parent lesson' do + response = described_class.call(lesson:, lesson_params:) + expect(response[:lesson].description).to eq('Description') + end + + it 'can specify the name of the new copy' do + new_params = lesson_params.merge(name: 'New Name') + response = described_class.call(lesson:, lesson_params: new_params) + expect(response[:lesson].name).to eq('New Name') + end + + it 'can specify the description of the new copy' do + new_params = lesson_params.merge(description: 'New Description') + response = described_class.call(lesson:, lesson_params: new_params) + expect(response[:lesson].description).to eq('New Description') + end + + context 'when creating a copy fails' do + let(:lesson_params) { { name: ' ' } } + + before do + allow(Sentry).to receive(:capture_exception) + end + + it 'does not create a lesson' do + expect { described_class.call(lesson:, lesson_params:) }.not_to change(Lesson, :count) + end + + it 'returns a failed operation response' do + response = described_class.call(lesson:, lesson_params:) + expect(response.failure?).to be(true) + end + + it 'returns the error message in the operation response' do + response = described_class.call(lesson:, lesson_params:) + expect(response[:error]).to match(/Error creating copy of lesson/) + end + + it 'sent the exception to Sentry' do + described_class.call(lesson:, lesson_params:) + expect(Sentry).to have_received(:capture_exception).with(kind_of(StandardError)) + end + end +end diff --git a/spec/concepts/lesson/create_spec.rb b/spec/concepts/lesson/create_spec.rb new file mode 100644 index 000000000..f44b18661 --- /dev/null +++ b/spec/concepts/lesson/create_spec.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Lesson::Create, type: :unit do + let(:teacher_index) { user_index_by_role('school-teacher') } + let(:teacher_id) { user_id_by_index(teacher_index) } + + let(:lesson_params) do + { name: 'Test Lesson', user_id: teacher_id } + end + + before do + stub_user_info_api + end + + it 'returns a successful operation response' do + response = described_class.call(lesson_params:) + expect(response.success?).to be(true) + end + + it 'creates a lesson' do + expect { described_class.call(lesson_params:) }.to change(Lesson, :count).by(1) + end + + it 'returns the lesson in the operation response' do + response = described_class.call(lesson_params:) + expect(response[:lesson]).to be_a(Lesson) + end + + it 'assigns the name' do + response = described_class.call(lesson_params:) + expect(response[:lesson].name).to eq('Test Lesson') + end + + it 'assigns the user_id' do + response = described_class.call(lesson_params:) + expect(response[:lesson].user_id).to eq(teacher_id) + end + + context 'when creation fails' do + let(:lesson_params) { {} } + + before do + allow(Sentry).to receive(:capture_exception) + end + + it 'does not create a lesson' do + expect { described_class.call(lesson_params:) }.not_to change(Lesson, :count) + end + + it 'returns a failed operation response' do + response = described_class.call(lesson_params:) + expect(response.failure?).to be(true) + end + + it 'returns the error message in the operation response' do + response = described_class.call(lesson_params:) + expect(response[:error]).to match(/Error creating lesson/) + end + + it 'sent the exception to Sentry' do + described_class.call(lesson_params:) + expect(Sentry).to have_received(:capture_exception).with(kind_of(StandardError)) + end + end +end diff --git a/spec/concepts/lesson/unarchive_spec.rb b/spec/concepts/lesson/unarchive_spec.rb new file mode 100644 index 000000000..31ade2c8b --- /dev/null +++ b/spec/concepts/lesson/unarchive_spec.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Lesson::Unarchive, type: :unit do + before do + stub_user_info_api + end + + let(:lesson) { create(:lesson, archived_at: Time.now.utc) } + + it 'returns a successful operation response' do + response = described_class.call(lesson:) + expect(response.success?).to be(true) + end + + it 'unarchives the lesson' do + described_class.call(lesson:) + expect(lesson.reload.archived?).to be(false) + end +end diff --git a/spec/concepts/lesson/update_spec.rb b/spec/concepts/lesson/update_spec.rb new file mode 100644 index 000000000..61cca3457 --- /dev/null +++ b/spec/concepts/lesson/update_spec.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Lesson::Update, type: :unit do + let(:lesson) { create(:lesson, name: 'Test Lesson') } + + let(:lesson_params) do + { name: 'New Name' } + end + + before do + stub_user_info_api + end + + it 'returns a successful operation response' do + response = described_class.call(lesson:, lesson_params:) + expect(response.success?).to be(true) + end + + it 'updates the lesson' do + response = described_class.call(lesson:, lesson_params:) + expect(response[:lesson].name).to eq('New Name') + end + + it 'returns the lesson in the operation response' do + response = described_class.call(lesson:, lesson_params:) + expect(response[:lesson]).to be_a(Lesson) + end + + context 'when updating fails' do + let(:lesson_params) { { name: ' ' } } + + before do + allow(Sentry).to receive(:capture_exception) + end + + it 'does not update the lesson' do + response = described_class.call(lesson:, lesson_params:) + expect(response[:lesson].reload.name).to eq('Test Lesson') + end + + it 'returns a failed operation response' do + response = described_class.call(lesson:, lesson_params:) + expect(response.failure?).to be(true) + end + + it 'returns the error message in the operation response' do + response = described_class.call(lesson:, lesson_params:) + expect(response[:error]).to match(/Error updating lesson/) + end + + it 'sent the exception to Sentry' do + described_class.call(lesson:, lesson_params:) + expect(Sentry).to have_received(:capture_exception).with(kind_of(StandardError)) + end + end +end diff --git a/spec/factories/lesson.rb b/spec/factories/lesson.rb new file mode 100644 index 000000000..ca1fec2cb --- /dev/null +++ b/spec/factories/lesson.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :lesson do + user_id { '11111111-1111-1111-1111-111111111111' } # Matches users.json. + sequence(:name) { |n| "Lesson #{n}" } + visibility { 'private' } + end +end diff --git a/spec/factories/project.rb b/spec/factories/project.rb index e1da44760..72d067681 100644 --- a/spec/factories/project.rb +++ b/spec/factories/project.rb @@ -2,7 +2,7 @@ FactoryBot.define do factory :project do - user_id { SecureRandom.uuid } + user_id { '22222222-2222-2222-2222-222222222222' } # Matches users.json. name { Faker::Book.title } identifier { "#{Faker::Verb.base}-#{Faker::Verb.base}-#{Faker::Verb.base}" } project_type { 'python' } diff --git a/spec/features/class_member/creating_a_class_member_spec.rb b/spec/features/class_member/creating_a_class_member_spec.rb index 4f9a71ec5..199a5466f 100644 --- a/spec/features/class_member/creating_a_class_member_spec.rb +++ b/spec/features/class_member/creating_a_class_member_spec.rb @@ -50,8 +50,9 @@ it "responds with nil attributes for the student if their user profile doesn't exist" do student_id = SecureRandom.uuid + new_params = { class_member: params[:class_member].merge(student_id:) } - post("/api/schools/#{school.id}/classes/#{school_class.id}/members", headers:, params: { class_member: { student_id: } }) + post("/api/schools/#{school.id}/classes/#{school_class.id}/members", headers:, params: new_params) data = JSON.parse(response.body, symbolize_names: true) expect(data[:student_name]).to be_nil diff --git a/spec/features/lesson/archiving_a_lesson_spec.rb b/spec/features/lesson/archiving_a_lesson_spec.rb new file mode 100644 index 000000000..ab6f5c950 --- /dev/null +++ b/spec/features/lesson/archiving_a_lesson_spec.rb @@ -0,0 +1,83 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'Archiving a lesson', type: :request do + before do + stub_hydra_public_api + stub_user_info_api + end + + let(:headers) { { Authorization: UserProfileMock::TOKEN } } + let!(:lesson) { create(:lesson, user_id: owner_id) } + let(:owner_index) { user_index_by_role('school-owner') } + let(:owner_id) { user_id_by_index(owner_index) } + + it 'responds 204 No Content' do + delete("/api/lessons/#{lesson.id}", headers:) + expect(response).to have_http_status(:no_content) + end + + it 'responds 204 No Content if the lesson is already archived' do + lesson.archive! + + delete("/api/lessons/#{lesson.id}", headers:) + expect(response).to have_http_status(:no_content) + end + + it 'archives the lesson' do + delete("/api/lessons/#{lesson.id}", headers:) + expect(lesson.reload.archived?).to be(true) + end + + it 'unarchives the lesson when the ?undo=true query parameter is set' do + lesson.archive! + + delete("/api/lessons/#{lesson.id}?undo=true", headers:) + expect(lesson.reload.archived?).to be(false) + end + + it 'responds 401 Unauthorized when no token is given' do + delete "/api/lessons/#{lesson.id}" + expect(response).to have_http_status(:unauthorized) + end + + it "responds 403 Forbidden when the user is not the lesson's owner" do + lesson.update!(user_id: SecureRandom.uuid) + + delete("/api/lessons/#{lesson.id}", headers:) + expect(response).to have_http_status(:forbidden) + end + + context 'when the lesson is associated with a school (library)' do + let(:school) { create(:school) } + let!(:lesson) { create(:lesson, school:, visibility: 'teachers') } + + it 'responds 204 No Content when the user is a school-owner' do + delete("/api/lessons/#{lesson.id}", headers:) + expect(response).to have_http_status(:no_content) + end + + it "responds 403 Forbidden when the user a school-owner but visibility is 'private'" do + lesson.update!(visibility: 'private') + + delete("/api/lessons/#{lesson.id}", headers:) + expect(response).to have_http_status(:forbidden) + end + + it 'responds 403 Forbidden when the user is another school-teacher in the school' do + stub_hydra_public_api(user_index: user_index_by_role('school-teacher')) + lesson.update!(user_id: SecureRandom.uuid) + + delete("/api/lessons/#{lesson.id}", headers:) + expect(response).to have_http_status(:forbidden) + end + + it 'responds 403 Forbidden when the user is a school-student' do + stub_hydra_public_api(user_index: user_index_by_role('school-student')) + + delete("/api/lessons/#{lesson.id}", headers:) + expect(response).to have_http_status(:forbidden) + end + end +end diff --git a/spec/features/lesson/creating_a_copy_of_a_lesson_spec.rb b/spec/features/lesson/creating_a_copy_of_a_lesson_spec.rb new file mode 100644 index 000000000..5f53e7a69 --- /dev/null +++ b/spec/features/lesson/creating_a_copy_of_a_lesson_spec.rb @@ -0,0 +1,123 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'Creating a copy of a lesson', type: :request do + before do + stub_hydra_public_api + stub_user_info_api + end + + let(:headers) { { Authorization: UserProfileMock::TOKEN } } + let!(:lesson) { create(:lesson, name: 'Test Lesson', visibility: 'public') } + let(:params) { {} } + + it 'responds 201 Created' do + post("/api/lessons/#{lesson.id}/copy", headers:, params:) + expect(response).to have_http_status(:created) + end + + it 'responds with the lesson JSON' do + post("/api/lessons/#{lesson.id}/copy", headers:, params:) + data = JSON.parse(response.body, symbolize_names: true) + + expect(data[:name]).to eq('Test Lesson') + end + + it 'responds with the user JSON which is set from the current user' do + post("/api/lessons/#{lesson.id}/copy", headers:, params:) + data = JSON.parse(response.body, symbolize_names: true) + + expect(data[:user_name]).to eq('School Owner') + end + + # See spec/concepts/lesson/create_copy_spec.rb for more examples. + it 'only copies a subset of fields from the lesson' do + post("/api/lessons/#{lesson.id}/copy", headers:, params:) + + data = JSON.parse(response.body, symbolize_names: true) + values = data.slice(:copied_from_id, :name, :visibility).values + + expect(values).to eq [lesson.id, 'Test Lesson', 'private'] + end + + it 'can override fields from the request params' do + new_params = { lesson: { name: 'New Name', visibility: 'public' } } + post("/api/lessons/#{lesson.id}/copy", headers:, params: new_params) + + data = JSON.parse(response.body, symbolize_names: true) + values = data.slice(:name, :visibility).values + + expect(values).to eq ['New Name', 'public'] + end + + it 'responds 422 Unprocessable Entity when params are invalid' do + post("/api/lessons/#{lesson.id}/copy", headers:, params: { lesson: { name: ' ' } }) + expect(response).to have_http_status(:unprocessable_entity) + end + + it 'responds 401 Unauthorized when no token is given' do + post("/api/lessons/#{lesson.id}/copy", params:) + expect(response).to have_http_status(:unauthorized) + end + + context "when the lesson's visibility is 'private'" do + let!(:lesson) { create(:lesson, name: 'Test Lesson', visibility: 'private') } + let(:owner_index) { user_index_by_role('school-owner') } + let(:owner_id) { user_id_by_index(owner_index) } + + it 'responds 201 Created when the user owns the lesson' do + lesson.update!(user_id: owner_id) + + post("/api/lessons/#{lesson.id}/copy", headers:, params:) + expect(response).to have_http_status(:created) + end + + it 'responds 403 Forbidden when the user does not own the lesson' do + post("/api/lessons/#{lesson.id}/copy", headers:, params:) + expect(response).to have_http_status(:forbidden) + end + end + + context "when the lesson's visibility is 'teachers'" do + let(:school) { create(:school) } + let!(:lesson) { create(:lesson, school:, name: 'Test Lesson', visibility: 'teachers') } + let(:owner_index) { user_index_by_role('school-owner') } + let(:owner_id) { user_id_by_index(owner_index) } + + let(:params) do + { + lesson: { + user_id: owner_id + } + } + end + + it 'responds 201 Created when the user owns the lesson' do + lesson.update!(user_id: owner_id) + + post("/api/lessons/#{lesson.id}/copy", headers:, params:) + expect(response).to have_http_status(:created) + end + + it 'responds 201 Created when the user is a school-owner or school-teacher within the school' do + post("/api/lessons/#{lesson.id}/copy", headers:, params:) + expect(response).to have_http_status(:created) + end + + it 'responds 403 Forbidden when the user is a school-owner for a different school' do + school = create(:school, id: SecureRandom.uuid) + lesson.update!(school_id: school.id) + + post("/api/lessons/#{lesson.id}/copy", headers:, params:) + expect(response).to have_http_status(:forbidden) + end + + it 'responds 403 Forbidden when the user is a school-student' do + stub_hydra_public_api(user_index: user_index_by_role('school-student')) + + post("/api/lessons/#{lesson.id}/copy", headers:, params:) + expect(response).to have_http_status(:forbidden) + end + end +end diff --git a/spec/features/lesson/creating_a_lesson_spec.rb b/spec/features/lesson/creating_a_lesson_spec.rb new file mode 100644 index 000000000..3612f9e76 --- /dev/null +++ b/spec/features/lesson/creating_a_lesson_spec.rb @@ -0,0 +1,178 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'Creating a lesson', type: :request do + before do + stub_hydra_public_api + stub_user_info_api + end + + let(:headers) { { Authorization: UserProfileMock::TOKEN } } + + let(:params) do + { + lesson: { + name: 'Test Lesson' + } + } + end + + it 'responds 201 Created' do + post('/api/lessons', headers:, params:) + expect(response).to have_http_status(:created) + end + + it 'responds with the lesson JSON' do + post('/api/lessons', headers:, params:) + data = JSON.parse(response.body, symbolize_names: true) + + expect(data[:name]).to eq('Test Lesson') + end + + it 'responds with the user JSON which is set from the current user' do + post('/api/lessons', headers:, params:) + data = JSON.parse(response.body, symbolize_names: true) + + expect(data[:user_name]).to eq('School Owner') + end + + it 'responds 422 Unprocessable Entity when params are invalid' do + post('/api/lessons', headers:, params: { lesson: { name: ' ' } }) + expect(response).to have_http_status(:unprocessable_entity) + end + + it 'responds 401 Unauthorized when no token is given' do + post('/api/lessons', params:) + expect(response).to have_http_status(:unauthorized) + end + + context 'when the lesson is associated with a school (library)' do + let(:school) { create(:school) } + let(:teacher_index) { user_index_by_role('school-teacher') } + let(:teacher_id) { user_id_by_index(teacher_index) } + + let(:params) do + { + lesson: { + name: 'Test Lesson', + school_id: school.id, + user_id: teacher_id + } + } + end + + it 'responds 201 Created' do + post('/api/lessons', headers:, params:) + expect(response).to have_http_status(:created) + end + + it 'responds 201 Created when the user is a school-teacher for the school' do + stub_hydra_public_api(user_index: user_index_by_role('school-teacher')) + + post('/api/lessons', headers:, params:) + expect(response).to have_http_status(:created) + end + + it 'sets the lesson user to the specified user for school-owner users' do + post('/api/lessons', headers:, params:) + data = JSON.parse(response.body, symbolize_names: true) + + expect(data[:user_id]).to eq(teacher_id) + end + + it 'sets the lesson user to the current user for school-teacher users' do + stub_hydra_public_api(user_index: teacher_index) + new_params = { lesson: params[:lesson].merge(user_id: 'ignored') } + + post('/api/lessons', headers:, params: new_params) + data = JSON.parse(response.body, symbolize_names: true) + + expect(data[:user_id]).to eq(teacher_id) + end + + it 'responds 403 Forbidden when the user is a school-owner for a different school' do + school.update!(id: SecureRandom.uuid) + + post('/api/lessons', headers:, params:) + expect(response).to have_http_status(:forbidden) + end + + it 'responds 403 Forbidden when the user is a school-student' do + stub_hydra_public_api(user_index: user_index_by_role('school-student')) + + post('/api/lessons', headers:, params:) + expect(response).to have_http_status(:forbidden) + end + end + + context 'when the lesson is associated with a school class' do + let(:school_class) { create(:school_class) } + let(:school) { school_class.school } + let(:teacher_index) { user_index_by_role('school-teacher') } + let(:teacher_id) { user_id_by_index(teacher_index) } + + let(:params) do + { + lesson: { + name: 'Test Lesson', + school_id: school.id, + school_class_id: school_class.id, + user_id: teacher_id + } + } + end + + it 'responds 201 Created' do + post('/api/lessons', headers:, params:) + expect(response).to have_http_status(:created) + end + + it 'responds 201 Created when the user is the school-teacher for the class' do + teacher_index = user_index_by_role('school-teacher') + + stub_hydra_public_api(user_index: teacher_index) + school_class.update!(teacher_id: user_id_by_index(teacher_index)) + + post('/api/lessons', headers:, params:) + expect(response).to have_http_status(:created) + end + + it 'responds 422 Unprocessable if school_id is missing' do + new_params = { lesson: params[:lesson].without(:school_id) } + + post('/api/lessons', headers:, params: new_params) + expect(response).to have_http_status(:unprocessable_entity) + end + + it 'responds 422 Unprocessable if school_class_id does not correspond to school_id' do + new_params = { lesson: params[:lesson].merge(school_id: SecureRandom.uuid) } + + post('/api/lessons', headers:, params: new_params) + expect(response).to have_http_status(:unprocessable_entity) + end + + it 'responds 403 Forbidden when the user is a school-owner for a different school' do + school = create(:school, id: SecureRandom.uuid) + school_class.update!(school_id: school.id) + + post('/api/lessons', headers:, params:) + expect(response).to have_http_status(:forbidden) + end + + it 'responds 403 Forbidden when the current user is a school-teacher for a different class' do + stub_hydra_public_api(user_index: user_index_by_role('school-teacher')) + school_class.update!(teacher_id: SecureRandom.uuid) + + post('/api/lessons', headers:, params:) + expect(response).to have_http_status(:forbidden) + end + + it 'responds 422 Unprocessable Entity when the user_id is a school-teacher for a different class' do + new_params = { lesson: params[:lesson].merge(user_id: SecureRandom.uuid) } + + post('/api/lessons', headers:, params: new_params) + expect(response).to have_http_status(:unprocessable_entity) + end + end +end diff --git a/spec/features/lesson/listing_lessons_spec.rb b/spec/features/lesson/listing_lessons_spec.rb new file mode 100644 index 000000000..bae920b85 --- /dev/null +++ b/spec/features/lesson/listing_lessons_spec.rb @@ -0,0 +1,174 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'Listing lessons', type: :request do + before do + stub_hydra_public_api + stub_user_info_api + end + + let(:headers) { { Authorization: UserProfileMock::TOKEN } } + let!(:lesson) { create(:lesson, name: 'Test Lesson', visibility: 'public') } + + it 'responds 200 OK' do + get('/api/lessons', headers:) + expect(response).to have_http_status(:ok) + end + + it 'responds 200 OK when no token is given' do + get '/api/lessons' + expect(response).to have_http_status(:ok) + end + + it 'responds with the lessons JSON' do + get('/api/lessons', headers:) + data = JSON.parse(response.body, symbolize_names: true) + + expect(data.first[:name]).to eq('Test Lesson') + end + + it 'responds with the user JSON' do + get('/api/lessons', headers:) + data = JSON.parse(response.body, symbolize_names: true) + + expect(data.first[:user_name]).to eq('School Teacher') + end + + it "responds with nil attributes for the user if their user profile doesn't exist" do + lesson.update!(user_id: SecureRandom.uuid) + + get('/api/lessons', headers:) + data = JSON.parse(response.body, symbolize_names: true) + + expect(data.first[:user_name]).to be_nil + end + + it 'does not include archived lessons' do + lesson.archive! + + get('/api/lessons', headers:) + data = JSON.parse(response.body, symbolize_names: true) + + expect(data.size).to eq(0) + end + + it 'includes archived lessons if ?include_archived=true is set' do + lesson.archive! + + get('/api/lessons?include_archived=true', headers:) + data = JSON.parse(response.body, symbolize_names: true) + + expect(data.size).to eq(1) + end + + context "when the lesson's visibility is 'private'" do + let!(:lesson) { create(:lesson, name: 'Test Lesson', visibility: 'private') } + let(:owner_index) { user_index_by_role('school-owner') } + let(:owner_id) { user_id_by_index(owner_index) } + + it 'includes the lesson when the user owns the lesson' do + lesson.update!(user_id: owner_id) + + get('/api/lessons', headers:) + data = JSON.parse(response.body, symbolize_names: true) + + expect(data.size).to eq(1) + end + + it 'does not include the lesson whent he user does not own the lesson' do + get('/api/lessons', headers:) + data = JSON.parse(response.body, symbolize_names: true) + + expect(data.size).to eq(0) + end + end + + context "when the lesson's visibility is 'teachers'" do + let(:school) { create(:school) } + let!(:lesson) { create(:lesson, school:, name: 'Test Lesson', visibility: 'teachers') } + let(:owner_index) { user_index_by_role('school-owner') } + let(:owner_id) { user_id_by_index(owner_index) } + + it 'includes the lesson when the user owns the lesson' do + lesson.update!(user_id: owner_id) + + get('/api/lessons', headers:) + data = JSON.parse(response.body, symbolize_names: true) + + expect(data.size).to eq(1) + end + + it 'includes the lesson when the user is a school-owner or school-teacher within the school' do + get('/api/lessons', headers:) + data = JSON.parse(response.body, symbolize_names: true) + + expect(data.size).to eq(1) + end + + it 'does not include the lesson when the user is a school-owner for a different school' do + school = create(:school, id: SecureRandom.uuid) + lesson.update!(school_id: school.id) + + get('/api/lessons', headers:) + data = JSON.parse(response.body, symbolize_names: true) + + expect(data.size).to eq(0) + end + + it 'does not include the lesson when the user is a school-student' do + stub_hydra_public_api(user_index: user_index_by_role('school-student')) + + get('/api/lessons', headers:) + data = JSON.parse(response.body, symbolize_names: true) + + expect(data.size).to eq(0) + end + end + + context "when the lesson's visibility is 'students'" do + let(:school_class) { create(:school_class) } + let!(:lesson) { create(:lesson, school_class:, name: 'Test Lesson', visibility: 'students') } + let(:teacher_index) { user_index_by_role('school-teacher') } + let(:teacher_id) { user_id_by_index(teacher_index) } + + it 'includes the lesson when the user owns the lesson' do + stub_hydra_public_api(user_index: teacher_index) + lesson.update!(user_id: teacher_id) + + get('/api/lessons', headers:) + data = JSON.parse(response.body, symbolize_names: true) + + expect(data.size).to eq(1) + end + + it "includes the lesson when the user is a school-student within the lesson's class" do + stub_hydra_public_api(user_index: user_index_by_role('school-student')) + create(:class_member, school_class:) + + get('/api/lessons', headers:) + data = JSON.parse(response.body, symbolize_names: true) + + expect(data.size).to eq(1) + end + + it "does not include the lesson when the user is not a school-student within the lesson's class" do + stub_hydra_public_api(user_index: user_index_by_role('school-student')) + + get('/api/lessons', headers:) + data = JSON.parse(response.body, symbolize_names: true) + + expect(data.size).to eq(0) + end + + it 'does not include the lesson when the user is a school-owner for a different school' do + school = create(:school, id: SecureRandom.uuid) + lesson.update!(school_id: school.id) + + get('/api/lessons', headers:) + data = JSON.parse(response.body, symbolize_names: true) + + expect(data.size).to eq(0) + end + end +end diff --git a/spec/features/lesson/showing_a_lesson_spec.rb b/spec/features/lesson/showing_a_lesson_spec.rb new file mode 100644 index 000000000..07a4d0e7b --- /dev/null +++ b/spec/features/lesson/showing_a_lesson_spec.rb @@ -0,0 +1,141 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'Showing a lesson', type: :request do + before do + stub_hydra_public_api + stub_user_info_api + end + + let!(:lesson) { create(:lesson, name: 'Test Lesson', visibility: 'public') } + let(:headers) { { Authorization: UserProfileMock::TOKEN } } + + it 'responds 200 OK' do + get("/api/lessons/#{lesson.id}", headers:) + expect(response).to have_http_status(:ok) + end + + it 'responds 200 OK when no token is given' do + get "/api/lessons/#{lesson.id}" + expect(response).to have_http_status(:ok) + end + + it 'responds with the lesson JSON' do + get("/api/lessons/#{lesson.id}", headers:) + data = JSON.parse(response.body, symbolize_names: true) + + expect(data[:name]).to eq('Test Lesson') + end + + it 'responds with the user JSON' do + get("/api/lessons/#{lesson.id}", headers:) + data = JSON.parse(response.body, symbolize_names: true) + + expect(data[:user_name]).to eq('School Teacher') + end + + it "responds with nil attributes for the user if their user profile doesn't exist" do + lesson.update!(user_id: SecureRandom.uuid) + + get("/api/lessons/#{lesson.id}", headers:) + data = JSON.parse(response.body, symbolize_names: true) + + expect(data[:user_name]).to be_nil + end + + it 'responds 404 Not Found when no lesson exists' do + get('/api/lessons/not-a-real-id', headers:) + expect(response).to have_http_status(:not_found) + end + + context "when the lesson's visibility is 'private'" do + let!(:lesson) { create(:lesson, name: 'Test Lesson', visibility: 'private') } + let(:owner_index) { user_index_by_role('school-owner') } + let(:owner_id) { user_id_by_index(owner_index) } + + it 'responds 200 OK when the user owns the lesson' do + lesson.update!(user_id: owner_id) + + get("/api/lessons/#{lesson.id}", headers:) + expect(response).to have_http_status(:ok) + end + + it 'responds 403 Forbidden when the user does not own the lesson' do + get("/api/lessons/#{lesson.id}", headers:) + expect(response).to have_http_status(:forbidden) + end + end + + context "when the lesson's visibility is 'teachers'" do + let(:school) { create(:school) } + let!(:lesson) { create(:lesson, school:, name: 'Test Lesson', visibility: 'teachers') } + let(:owner_index) { user_index_by_role('school-owner') } + let(:owner_id) { user_id_by_index(owner_index) } + + it 'responds 200 OK when the user owns the lesson' do + lesson.update!(user_id: owner_id) + + get("/api/lessons/#{lesson.id}", headers:) + expect(response).to have_http_status(:ok) + end + + it 'responds 200 OK when the user is a school-owner or school-teacher within the school' do + get("/api/lessons/#{lesson.id}", headers:) + expect(response).to have_http_status(:ok) + end + + it 'responds 403 Forbidden when the user is a school-owner for a different school' do + school = create(:school, id: SecureRandom.uuid) + lesson.update!(school_id: school.id) + + get("/api/lessons/#{lesson.id}", headers:) + expect(response).to have_http_status(:forbidden) + end + + it 'responds 403 Forbidden when the user is a school-student' do + stub_hydra_public_api(user_index: user_index_by_role('school-student')) + + get("/api/lessons/#{lesson.id}", headers:) + expect(response).to have_http_status(:forbidden) + end + end + + context "when the lesson's visibility is 'students'" do + let(:school_class) { create(:school_class) } + let!(:lesson) { create(:lesson, school_class:, name: 'Test Lesson', visibility: 'students') } + let(:teacher_index) { user_index_by_role('school-teacher') } + let(:teacher_id) { user_id_by_index(teacher_index) } + + it 'responds 200 OK when the user owns the lesson' do + stub_hydra_public_api(user_index: teacher_index) + lesson.update!(user_id: teacher_id) + + get("/api/lessons/#{lesson.id}", headers:) + expect(response).to have_http_status(:ok) + end + + it "responds 200 OK when the user is a school-student within the lesson's class" do + stub_hydra_public_api(user_index: user_index_by_role('school-student')) + create(:class_member, school_class:) + + get("/api/lessons/#{lesson.id}", headers:) + expect(response).to have_http_status(:ok) + end + + it "responds 403 Forbidden when the user is a school-student but isn't within the lesson's class" do + stub_hydra_public_api(user_index: user_index_by_role('school-student')) + + get("/api/lessons/#{lesson.id}", headers:) + expect(response).to have_http_status(:forbidden) + end + + it 'responds 403 Forbidden when the user is a school-owner for a different school' do + school = create(:school, id: SecureRandom.uuid) + lesson.update!(school_id: school.id) + + get("/api/lessons/#{lesson.id}", headers:) + expect(response).to have_http_status(:forbidden) + end + end +end diff --git a/spec/features/lesson/updating_a_lesson_spec.rb b/spec/features/lesson/updating_a_lesson_spec.rb new file mode 100644 index 000000000..3eb0723cc --- /dev/null +++ b/spec/features/lesson/updating_a_lesson_spec.rb @@ -0,0 +1,127 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'Updating a lesson', type: :request do + before do + stub_hydra_public_api + stub_user_info_api + end + + let(:headers) { { Authorization: UserProfileMock::TOKEN } } + let!(:lesson) { create(:lesson, name: 'Test Lesson', user_id: owner_id) } + let(:owner_index) { user_index_by_role('school-owner') } + let(:owner_id) { user_id_by_index(owner_index) } + + let(:params) do + { + lesson: { + name: 'New Name' + } + } + end + + it 'responds 200 OK' do + put("/api/lessons/#{lesson.id}", headers:, params:) + expect(response).to have_http_status(:ok) + end + + it 'responds with the lesson JSON' do + put("/api/lessons/#{lesson.id}", headers:, params:) + data = JSON.parse(response.body, symbolize_names: true) + + expect(data[:name]).to eq('New Name') + end + + it 'responds with the user JSON' do + put("/api/lessons/#{lesson.id}", headers:, params:) + data = JSON.parse(response.body, symbolize_names: true) + + expect(data[:user_name]).to eq('School Owner') + end + + it 'responds 422 Unprocessable Entity when params are invalid' do + put("/api/lessons/#{lesson.id}", headers:, params: { lesson: { name: ' ' } }) + expect(response).to have_http_status(:unprocessable_entity) + end + + it 'responds 401 Unauthorized when no token is given' do + put("/api/lessons/#{lesson.id}", params:) + expect(response).to have_http_status(:unauthorized) + end + + it "responds 403 Forbidden when the user is not the lesson's owner" do + lesson.update!(user_id: SecureRandom.uuid) + + put("/api/lessons/#{lesson.id}", headers:, params:) + expect(response).to have_http_status(:forbidden) + end + + context 'when the lesson is associated with a school (library)' do + let(:school) { create(:school) } + let!(:lesson) { create(:lesson, school:, name: 'Test Lesson', visibility: 'teachers') } + + it 'responds 200 OK when the user is a school-owner' do + put("/api/lessons/#{lesson.id}", headers:, params:) + expect(response).to have_http_status(:ok) + end + + it 'responds 200 OK when assigning the lesson to a school class' do + school_class = create(:school_class, school:) + + new_params = { lesson: params[:lesson].merge(school_class_id: school_class.id) } + put("/api/lessons/#{lesson.id}", headers:, params: new_params) + + expect(response).to have_http_status(:ok) + end + + it "responds 403 Forbidden when the user a school-owner but visibility is 'private'" do + lesson.update!(visibility: 'private') + + put("/api/lessons/#{lesson.id}", headers:, params:) + expect(response).to have_http_status(:forbidden) + end + + it 'responds 403 Forbidden when the user is another school-teacher in the school' do + stub_hydra_public_api(user_index: user_index_by_role('school-teacher')) + lesson.update!(user_id: SecureRandom.uuid) + + put("/api/lessons/#{lesson.id}", headers:, params:) + expect(response).to have_http_status(:forbidden) + end + + it 'responds 403 Forbidden when the user is a school-student' do + stub_hydra_public_api(user_index: user_index_by_role('school-student')) + + put("/api/lessons/#{lesson.id}", headers:, params:) + expect(response).to have_http_status(:forbidden) + end + end + + context 'when the lesson is associated with a school class' do + let(:school_class) { create(:school_class) } + let!(:lesson) { create(:lesson, school_class:, name: 'Test Lesson', visibility: 'students') } + + it 'responds 200 OK when the user is a school-owner' do + put("/api/lessons/#{lesson.id}", headers:, params:) + expect(response).to have_http_status(:ok) + end + + it 'responds 422 Unprocessable Entity when trying to re-assign the lesson to a different class' do + school = create(:school, id: SecureRandom.uuid) + school_class = create(:school_class, school:, teacher_id: SecureRandom.uuid) + + new_params = { lesson: params[:lesson].merge(school_class_id: school_class.id) } + put("/api/lessons/#{lesson.id}", headers:, params: new_params) + + expect(response).to have_http_status(:unprocessable_entity) + end + + it 'responds 422 Unprocessable Entity when trying to re-assign the lesson to a different user' do + new_params = { lesson: params[:lesson].merge(user_id: owner_id) } + put("/api/lessons/#{lesson.id}", headers:, params: new_params) + + expect(response).to have_http_status(:unprocessable_entity) + end + end +end diff --git a/spec/features/project/creating_a_project_spec.rb b/spec/features/project/creating_a_project_spec.rb new file mode 100644 index 000000000..5b9bc1a38 --- /dev/null +++ b/spec/features/project/creating_a_project_spec.rb @@ -0,0 +1,188 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'Creating a project', type: :request do + before do + stub_hydra_public_api + stub_user_info_api + mock_phrase_generation + end + + let(:headers) { { Authorization: UserProfileMock::TOKEN } } + + let(:params) do + { + project: { + name: 'Test Project', + components: [ + { name: 'main', extension: 'py', content: 'print("hi")' } + ] + } + } + end + + it 'responds 201 Created' do + post('/api/projects', headers:, params:) + expect(response).to have_http_status(:created) + end + + it 'responds with the project JSON' do + post('/api/projects', headers:, params:) + data = JSON.parse(response.body, symbolize_names: true) + + expect(data[:name]).to eq('Test Project') + end + + it 'responds with the components JSON' do + post('/api/projects', headers:, params:) + data = JSON.parse(response.body, symbolize_names: true) + + expect(data[:components].first[:content]).to eq('print("hi")') + end + + it 'responds 422 Unprocessable Entity when params are invalid' do + post('/api/projects', headers:, params: { project: { components: [{ name: ' ' }] } }) + expect(response).to have_http_status(:unprocessable_entity) + end + + it 'responds 401 Unauthorized when no token is given' do + post('/api/projects', params:) + expect(response).to have_http_status(:unauthorized) + end + + context 'when the project is associated with a school (library)' do + let(:school) { create(:school) } + let(:teacher_index) { user_index_by_role('school-teacher') } + let(:teacher_id) { user_id_by_index(teacher_index) } + + let(:params) do + { + project: { + name: 'Test Project', + components: [], + school_id: school.id, + user_id: teacher_id + } + } + end + + it 'responds 201 Created' do + post('/api/projects', headers:, params:) + expect(response).to have_http_status(:created) + end + + it 'responds 201 Created when the user is a school-teacher for the school' do + stub_hydra_public_api(user_index: user_index_by_role('school-teacher')) + + post('/api/projects', headers:, params:) + expect(response).to have_http_status(:created) + end + + it 'responds 201 Created when the user is a school-student for the school' do + stub_hydra_public_api(user_index: user_index_by_role('school-student')) + + post('/api/projects', headers:, params:) + expect(response).to have_http_status(:created) + end + + it 'sets the lesson user to the specified user for school-owner users' do + post('/api/projects', headers:, params:) + data = JSON.parse(response.body, symbolize_names: true) + + expect(data[:user_id]).to eq(teacher_id) + end + + it 'sets the project user to the current user for school-teacher users' do + stub_hydra_public_api(user_index: teacher_index) + new_params = { project: params[:project].merge(user_id: 'ignored') } + + post('/api/projects', headers:, params: new_params) + data = JSON.parse(response.body, symbolize_names: true) + + expect(data[:user_id]).to eq(teacher_id) + end + + it 'responds 403 Forbidden when the user is a school-owner for a different school' do + school.update!(id: SecureRandom.uuid) + + post('/api/projects', headers:, params:) + expect(response).to have_http_status(:forbidden) + end + end + + context 'when the project is associated with a lesson' do + let(:school) { create(:school) } + let(:lesson) { create(:lesson, school:) } + let(:teacher_index) { user_index_by_role('school-teacher') } + let(:teacher_id) { user_id_by_index(teacher_index) } + + let(:params) do + { + project: { + name: 'Test Project', + components: [], + school_id: school.id, + lesson_id: lesson.id, + user_id: teacher_id + } + } + end + + it 'responds 201 Created' do + post('/api/projects', headers:, params:) + expect(response).to have_http_status(:created) + end + + it 'responds 201 Created when the current user is the owner of the lesson' do + stub_hydra_public_api(user_index: teacher_index) + lesson.update!(user_id: user_id_by_index(teacher_index)) + + post('/api/projects', headers:, params:) + expect(response).to have_http_status(:created) + end + + it 'responds 422 Unprocessable when when the user_id is not the owner of the lesson' do + new_params = { project: params[:project].merge(user_id: SecureRandom.uuid) } + + post('/api/projects', headers:, params: new_params) + expect(response).to have_http_status(:unprocessable_entity) + end + + it 'responds 422 Unprocessable when lesson_id is provided but school_id is missing' do + new_params = { project: params[:project].without(:school_id) } + + post('/api/projects', headers:, params: new_params) + expect(response).to have_http_status(:unprocessable_entity) + end + + it 'responds 422 Unprocessable when lesson_id does not correspond to school_id' do + new_params = { project: params[:project].merge(lesson_id: SecureRandom.uuid) } + + post('/api/projects', headers:, params: new_params) + expect(response).to have_http_status(:unprocessable_entity) + end + + it 'responds 403 Forbidden when the user is a school-owner for a different school' do + new_params = { project: params[:project].without(:lesson_id).merge(school_id: SecureRandom.uuid) } + + post('/api/projects', headers:, params: new_params) + expect(response).to have_http_status(:forbidden) + end + + it 'responds 403 Forbidden when the current user is not the owner of the lesson' do + stub_hydra_public_api(user_index: teacher_index) + lesson.update!(user_id: SecureRandom.uuid) + + post('/api/projects', headers:, params:) + expect(response).to have_http_status(:forbidden) + end + + it 'responds 403 Forbidden when the user is a school-student' do + stub_hydra_public_api(user_index: user_index_by_role('school-student')) + + post('/api/projects', headers:, params:) + expect(response).to have_http_status(:forbidden) + end + end +end diff --git a/spec/features/project/updating_a_project_spec.rb b/spec/features/project/updating_a_project_spec.rb new file mode 100644 index 000000000..eede8b5cf --- /dev/null +++ b/spec/features/project/updating_a_project_spec.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'Updating a project', type: :request do + before do + stub_hydra_public_api + stub_user_info_api + + create(:component, project:, name: 'main', extension: 'py', content: 'print("hi")') + end + + let(:headers) { { Authorization: UserProfileMock::TOKEN } } + let!(:project) { create(:project, name: 'Test Project', user_id: owner_id) } + let(:owner_index) { user_index_by_role('school-owner') } + let(:owner_id) { user_id_by_index(owner_index) } + + let(:params) do + { + project: { + name: 'New Name', + components: [ + { name: 'main', extension: 'py', content: 'print("hello")' } + ] + } + } + end + + it 'responds 200 OK' do + put("/api/projects/#{project.id}", headers:, params:) + expect(response).to have_http_status(:ok) + end + + it 'responds with the project JSON' do + put("/api/projects/#{project.id}", headers:, params:) + data = JSON.parse(response.body, symbolize_names: true) + + expect(data[:name]).to eq('New Name') + end + + it 'responds with the components JSON' do + put("/api/projects/#{project.id}", headers:, params:) + data = JSON.parse(response.body, symbolize_names: true) + + expect(data[:components].first[:content]).to eq('print("hello")') + end + + it 'responds 422 Unprocessable Entity when params are invalid' do + put("/api/projects/#{project.id}", headers:, params: { project: { components: [{ name: ' ' }] } }) + expect(response).to have_http_status(:unprocessable_entity) + end + + it 'responds 401 Unauthorized when no token is given' do + put("/api/projects/#{project.id}", params:) + expect(response).to have_http_status(:unauthorized) + end +end diff --git a/spec/features/school_class/creating_a_school_class_spec.rb b/spec/features/school_class/creating_a_school_class_spec.rb index 3366e4ec2..805c5a6be 100644 --- a/spec/features/school_class/creating_a_school_class_spec.rb +++ b/spec/features/school_class/creating_a_school_class_spec.rb @@ -50,8 +50,9 @@ it "responds with nil attributes for the teacher if their user profile doesn't exist" do teacher_id = SecureRandom.uuid + new_params = { school_class: params[:school_class].merge(teacher_id:) } - post("/api/schools/#{school.id}/classes", headers:, params: { school_class: { teacher_id: } }) + post("/api/schools/#{school.id}/classes", headers:, params: new_params) data = JSON.parse(response.body, symbolize_names: true) expect(data[:teacher_name]).to be_nil diff --git a/spec/features/school_class/updating_a_school_class_spec.rb b/spec/features/school_class/updating_a_school_class_spec.rb index c3522b2dc..b9338204b 100644 --- a/spec/features/school_class/updating_a_school_class_spec.rb +++ b/spec/features/school_class/updating_a_school_class_spec.rb @@ -50,8 +50,9 @@ it "responds with nil attributes for the teacher if their user profile doesn't exist" do teacher_id = SecureRandom.uuid + new_params = { school_class: params[:school_class].merge(teacher_id:) } - put("/api/schools/#{school.id}/classes/#{school_class.id}", headers:, params: { school_class: { teacher_id: } }) + put("/api/schools/#{school.id}/classes/#{school_class.id}", headers:, params: new_params) data = JSON.parse(response.body, symbolize_names: true) expect(data[:teacher_name]).to be_nil diff --git a/spec/models/lesson_spec.rb b/spec/models/lesson_spec.rb new file mode 100644 index 000000000..b9ee0ab56 --- /dev/null +++ b/spec/models/lesson_spec.rb @@ -0,0 +1,272 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Lesson do + before do + stub_user_info_api + end + + describe 'associations' do + it 'optionally belongs to a school (library)' do + lesson = create(:lesson, school: build(:school)) + expect(lesson.school).to be_a(School) + end + + it 'optionally belongs to a school class' do + school_class = create(:school_class) + + lesson = create(:lesson, school_class:, school: school_class.school) + expect(lesson.school_class).to be_a(SchoolClass) + end + + it 'optionally belongs to a parent' do + lesson = create(:lesson, parent: build(:lesson)) + expect(lesson.parent).to be_a(described_class) + end + + it 'has many copies' do + lesson = create(:lesson, copies: [build(:lesson), build(:lesson)]) + expect(lesson.copies.size).to eq(2) + end + + it 'has many projects' do + user_id = SecureRandom.uuid + lesson = create(:lesson, user_id:, projects: [build(:project, user_id:)]) + expect(lesson.projects.size).to eq(1) + end + end + + describe 'callbacks' do + it 'cannot be destroyed and should be archived instead' do + lesson = create(:lesson) + expect { lesson.destroy! }.to raise_error(ActiveRecord::RecordNotDestroyed) + end + end + + describe 'validations' do + subject(:lesson) { build(:lesson) } + + it 'has a valid default factory' do + expect(lesson).to be_valid + end + + it 'can save the default factory' do + expect { lesson.save! }.not_to raise_error + end + + it 'requires a user_id' do + lesson.user_id = ' ' + expect(lesson).to be_invalid + end + + it 'requires a UUID user_id' do + lesson.user_id = 'invalid' + expect(lesson).to be_invalid + end + + context 'when the lesson has a school' do + before do + lesson.update!(school: create(:school)) + end + + it 'requires that the user that has the school-owner or school-teacher role for the school' do + lesson.user_id = '22222222-2222-2222-2222-222222222222' # school-student + expect(lesson).to be_invalid + end + end + + context 'when the lesson has a school_class' do + before do + lesson.update!(school_class: create(:school_class)) + end + + it 'requires that the user that is the school-teacher for the school_class' do + lesson.user_id = '00000000-0000-0000-0000-000000000000' # school-owner + expect(lesson).to be_invalid + end + end + + it 'requires a name' do + lesson.name = ' ' + expect(lesson).to be_invalid + end + + it 'requires a visibility' do + lesson.visibility = ' ' + expect(lesson).to be_invalid + end + + it "requires a visibility that is either 'private', 'teachers', 'students' or 'public'" do + lesson.visibility = 'invalid' + expect(lesson).to be_invalid + end + end + + describe '.archived' do + let!(:archived_lesson) { create(:lesson, archived_at: Time.now.utc) } + let!(:unarchived_lesson) { create(:lesson) } + + it 'includes archived lessons' do + expect(described_class.archived).to include(archived_lesson) + end + + it 'excludes unarchived lessons' do + expect(described_class.archived).not_to include(unarchived_lesson) + end + end + + describe '.unarchived' do + let!(:archived_lesson) { create(:lesson, archived_at: Time.now.utc) } + let!(:unarchived_lesson) { create(:lesson) } + + it 'includes unarchived lessons' do + expect(described_class.unarchived).to include(unarchived_lesson) + end + + it 'excludes archived lessons' do + expect(described_class.unarchived).not_to include(archived_lesson) + end + end + + describe '#school' do + it 'is set from the school_class' do + lesson = create(:lesson, school_class: build(:school_class)) + expect(lesson.school).to eq(lesson.school_class.school) + end + + it 'is not nullified when there is no school_class' do + lesson = create(:lesson, school: build(:school)) + expect(lesson.school).not_to eq(lesson.school_class&.school) + end + end + + describe '.users' do + it 'returns User instances for the current scope' do + create(:lesson) + + user = described_class.all.users.first + expect(user.name).to eq('School Teacher') + end + + it 'ignores members where no profile account exists' do + create(:lesson, user_id: SecureRandom.uuid) + + user = described_class.all.users.first + expect(user).to be_nil + end + + it 'ignores members not included in the current scope' do + create(:lesson) + + user = described_class.none.users.first + expect(user).to be_nil + end + end + + describe '.with_users' do + it 'returns an array of class members paired with their User instance' do + lesson = create(:lesson) + + pair = described_class.all.with_users.first + user = described_class.all.users.first + + expect(pair).to eq([lesson, user]) + end + + it 'returns nil values for members where no profile account exists' do + lesson = create(:lesson, user_id: SecureRandom.uuid) + + pair = described_class.all.with_users.first + expect(pair).to eq([lesson, nil]) + end + + it 'ignores members not included in the current scope' do + create(:lesson) + + pair = described_class.none.with_users.first + expect(pair).to be_nil + end + end + + describe '#with_user' do + it 'returns the class member paired with their User instance' do + lesson = create(:lesson) + + pair = lesson.with_user + user = described_class.all.users.first + + expect(pair).to eq([lesson, user]) + end + + it 'returns a nil value if the member has no profile account' do + lesson = create(:lesson, user_id: SecureRandom.uuid) + + pair = lesson.with_user + expect(pair).to eq([lesson, nil]) + end + end + + describe '#archive!' do + let(:lesson) { build(:lesson) } + + it 'archives the lesson' do + lesson.archive! + expect(lesson.archived?).to be(true) + end + + it 'sets archived_at' do + lesson.archive! + expect(lesson.archived_at).to be_present + end + + it 'does not set archived_at if it was already set' do + lesson.update!(archived_at: 1.day.ago) + + lesson.archive! + expect(lesson.archived_at).to be < 23.hours.ago + end + + it 'saves the record' do + lesson.archive! + expect(lesson).to be_persisted + end + + it 'is infallible to other validation errors' do + lesson.save! + lesson.name = ' ' + lesson.save!(validate: false) + + lesson.archive! + expect(lesson.archived?).to be(true) + end + end + + describe '#unarchive!' do + let(:lesson) { build(:lesson, archived_at: Time.now.utc) } + + it 'unarchives the lesson' do + lesson.unarchive! + expect(lesson.archived?).to be(false) + end + + it 'clears archived_at' do + lesson.unarchive! + expect(lesson.archived_at).to be_nil + end + + it 'saves the record' do + lesson.unarchive! + expect(lesson).to be_persisted + end + + it 'is infallible to other validation errors' do + lesson.archive! + lesson.name = ' ' + lesson.save!(validate: false) + + lesson.unarchive! + expect(lesson.archived?).to be(false) + end + end +end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 8c9e149a2..480eb596d 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -3,11 +3,17 @@ require 'rails_helper' RSpec.describe Project do + before do + stub_user_info_api + end + describe 'associations', :sample_words do - it { is_expected.to have_many(:components) } + it { is_expected.to belong_to(:school).optional(true) } + it { is_expected.to belong_to(:lesson).optional(true) } + it { is_expected.to belong_to(:parent).optional(true) } it { is_expected.to have_many(:remixes).dependent(:nullify) } + it { is_expected.to have_many(:components) } it { is_expected.to have_many(:project_errors).dependent(:nullify) } - it { is_expected.to belong_to(:parent).optional(true) } it { is_expected.to have_many_attached(:images) } it 'purges attached images' do @@ -19,6 +25,14 @@ let(:project) { create(:project) } let(:identifier) { project.identifier } + it 'has a valid default factory' do + expect(build(:project)).to be_valid + end + + it 'can save the default factory' do + expect { build(:project).save! }.not_to raise_error + end + it 'is invalid if no user or locale' do invalid_project = build(:project, locale: nil, user_id: nil) expect(invalid_project).to be_invalid @@ -56,6 +70,29 @@ expect(new_project).to be_invalid end end + + context 'when the project has a school' do + before do + project.update!(school: create(:school)) + end + + it 'requires that the user that has a role within the school' do + project.user_id = SecureRandom.uuid + expect(project).to be_invalid + end + end + + context 'when the project has a lesson' do + before do + lesson = create(:lesson) + project.update!(lesson:, user_id: lesson.user_id, identifier: 'something') + end + + it 'requires that the user be the owner of the lesson' do + project.user_id = SecureRandom.uuid + expect(project).to be_invalid + end + end end describe 'check_unique_not_null', :sample_words do @@ -66,4 +103,70 @@ expect { unsaved_project.valid? }.to change { unsaved_project.identifier.nil? }.from(true).to(false) end end + + describe '.users' do + it 'returns User instances for the current scope' do + create(:project) + + user = described_class.all.users.first + expect(user.name).to eq('School Student') + end + + it 'ignores members where no profile account exists' do + create(:project, user_id: SecureRandom.uuid) + + user = described_class.all.users.first + expect(user).to be_nil + end + + it 'ignores members not included in the current scope' do + create(:project) + + user = described_class.none.users.first + expect(user).to be_nil + end + end + + describe '.with_users' do + it 'returns an array of class members paired with their User instance' do + project = create(:project) + + pair = described_class.all.with_users.first + user = described_class.all.users.first + + expect(pair).to eq([project, user]) + end + + it 'returns nil values for members where no profile account exists' do + project = create(:project, user_id: SecureRandom.uuid) + + pair = described_class.all.with_users.first + expect(pair).to eq([project, nil]) + end + + it 'ignores members not included in the current scope' do + create(:project) + + pair = described_class.none.with_users.first + expect(pair).to be_nil + end + end + + describe '#with_user' do + it 'returns the class member paired with their User instance' do + project = create(:project) + + pair = project.with_user + user = described_class.all.users.first + + expect(pair).to eq([project, user]) + end + + it 'returns a nil value if the member has no profile account' do + project = create(:project, user_id: SecureRandom.uuid) + + pair = project.with_user + expect(pair).to eq([project, nil]) + end + end end diff --git a/spec/models/school_class_spec.rb b/spec/models/school_class_spec.rb index 35f6cc37d..f1468e223 100644 --- a/spec/models/school_class_spec.rb +++ b/spec/models/school_class_spec.rb @@ -18,12 +18,26 @@ expect(school_class.members.size).to eq(1) end + it 'has many lessons' do + school_class = create(:school_class, lessons: [build(:lesson)]) + expect(school_class.lessons.size).to eq(1) + end + context 'when a school_class is destroyed' do - let!(:school_class) { create(:school_class, members: [build(:class_member)]) } + let!(:school_class) { create(:school_class, members: [build(:class_member)], lessons: [build(:lesson)]) } it 'also destroys class members to avoid making them invalid' do expect { school_class.destroy! }.to change(ClassMember, :count).by(-1) end + + it 'does not destroy lessons' do + expect { school_class.destroy! }.not_to change(Lesson, :count) + end + + it 'nullifies school_class_id on lessons' do + school_class.destroy! + expect(Lesson.last.school_class).to be_nil + end end end diff --git a/spec/models/school_spec.rb b/spec/models/school_spec.rb index 984a01064..f27932529 100644 --- a/spec/models/school_spec.rb +++ b/spec/models/school_spec.rb @@ -13,9 +13,23 @@ expect(school.classes.size).to eq(2) end + it 'has many lessons' do + school = create(:school, lessons: [build(:lesson), build(:lesson)]) + expect(school.lessons.size).to eq(2) + end + + it 'has many projects' do + school = create(:school, projects: [build(:project), build(:project)]) + expect(school.projects.size).to eq(2) + end + context 'when a school is destroyed' do - let!(:school_class) { build(:school_class, members: [build(:class_member)]) } - let!(:school) { create(:school, classes: [school_class]) } + let(:lesson1) { build(:lesson) } + let(:lesson2) { build(:lesson) } + let(:project) { build(:project) } + + let!(:school_class) { build(:school_class, members: [build(:class_member)], lessons: [lesson1]) } + let!(:school) { create(:school, classes: [school_class], lessons: [lesson2], projects: [project]) } it 'also destroys school classes to avoid making them invalid' do expect { school.destroy! }.to change(SchoolClass, :count).by(-1) @@ -24,6 +38,28 @@ it 'also destroys class members to avoid making them invalid' do expect { school.destroy! }.to change(ClassMember, :count).by(-1) end + + it 'does not destroy lessons' do + expect { school.destroy! }.not_to change(Lesson, :count) + end + + it 'nullifies school_id and school_class_id fields on lessons' do + school.destroy! + + lessons = [lesson1, lesson2].map(&:reload) + values = lessons.flat_map { |l| [l.school_id, l.school_class_id] } + + expect(values).to eq [nil, nil, nil, nil] + end + + it 'does not destroy projects' do + expect { school.destroy! }.not_to change(Project, :count) + end + + it 'nullifies the school_id field on projects' do + school.destroy! + expect(project.reload.school_id).to be_nil + end end end diff --git a/spec/requests/projects/create_spec.rb b/spec/requests/projects/create_spec.rb index 184e53d05..81cb64318 100644 --- a/spec/requests/projects/create_spec.rb +++ b/spec/requests/projects/create_spec.rb @@ -20,7 +20,7 @@ it 'returns success' do post('/api/projects', headers:) - expect(response).to have_http_status(:ok) + expect(response).to have_http_status(:created) end end @@ -36,7 +36,7 @@ it 'returns error' do post('/api/projects', headers:) - expect(response).to have_http_status(:internal_server_error) + expect(response).to have_http_status(:unprocessable_entity) end end end diff --git a/spec/requests/projects/update_spec.rb b/spec/requests/projects/update_spec.rb index 638eb3e09..f6792ef18 100644 --- a/spec/requests/projects/update_spec.rb +++ b/spec/requests/projects/update_spec.rb @@ -71,7 +71,7 @@ it 'returns error response' do put("/api/projects/#{project.identifier}", params:, headers:) - expect(response).to have_http_status(:bad_request) + expect(response).to have_http_status(:unprocessable_entity) end end end