Skip to content

Bulk lesson creation#853

Merged
PetarSimonovic merged 4 commits into
mainfrom
bulk_lesson_creation
Jun 11, 2026
Merged

Bulk lesson creation#853
PetarSimonovic merged 4 commits into
mainfrom
bulk_lesson_creation

Conversation

@PetarSimonovic

@PetarSimonovic PetarSimonovic commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Status

What's changed?

  • Add Lessons::Batch controller
  • Add a bulk create jbuilder that renders an array, echoing an origin_identifier when supplied so the caller can match responses to submitted projects
  • Add a LessonsCreation concern to consolidate logic and params that are share between batch and single lesson creation
  • Update specs

@cla-bot cla-bot Bot added the cla-signed label Jun 8, 2026
@PetarSimonovic PetarSimonovic force-pushed the bulk_lesson_creation branch from 747fb54 to 05dc28b Compare June 8, 2026 09:31
@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown

Test coverage

91.57% line coverage reported by SimpleCov.
Run: https://github.com/RaspberryPiFoundation/editor-api/actions/runs/27277481011

@PetarSimonovic PetarSimonovic force-pushed the bulk_lesson_creation branch from 05dc28b to 1b5947c Compare June 8, 2026 10:14
@PetarSimonovic PetarSimonovic temporarily deployed to editor-api-p-bulk-lesso-kqeckt June 8, 2026 10:14 Inactive
@PetarSimonovic PetarSimonovic force-pushed the bulk_lesson_creation branch from 1b5947c to 42098d1 Compare June 8, 2026 10:21
@PetarSimonovic PetarSimonovic temporarily deployed to editor-api-p-bulk-lesso-zoj7yk June 8, 2026 10:21 Inactive
@PetarSimonovic PetarSimonovic force-pushed the bulk_lesson_creation branch from 42098d1 to aef91db Compare June 8, 2026 10:52
@PetarSimonovic PetarSimonovic temporarily deployed to editor-api-p-bulk-lesso-gdfiir June 8, 2026 10:53 Inactive
@PetarSimonovic PetarSimonovic marked this pull request as ready for review June 8, 2026 10:57
Copilot AI review requested due to automatic review settings June 8, 2026 10:57
@PetarSimonovic PetarSimonovic force-pushed the bulk_lesson_creation branch from aef91db to 7cfd87f Compare June 8, 2026 10:57
@PetarSimonovic PetarSimonovic temporarily deployed to editor-api-p-bulk-lesso-xgr0gu June 8, 2026 10:58 Inactive

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds support for bulk lesson creation via POST /api/lessons by accepting a lesson_projects array payload, returning an array response that can optionally echo origin_identifier per entry.

Changes:

  • Extend Api::LessonsController#create to handle lesson_projects bulk requests and render a new bulk JSON view.
  • Add Lesson::CreateBulk operation to create multiple lessons and propagate origin_identifier into each result.
  • Add/extend request + unit specs to cover bulk creation success, partial failure, and school_class validation.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
spec/features/lesson/creating_a_lesson_spec.rb Adds request coverage for bulk-create responses, partial errors, and school_class mismatch behavior.
spec/concepts/lesson/create_bulk_spec.rb Adds unit tests for Lesson::CreateBulk, including origin_identifier handling.
lib/concepts/lesson/operations/create_bulk.rb Introduces Lesson::CreateBulk operation to fan out to Lesson::Create.
app/views/api/lessons/create_bulk.json.jbuilder Adds bulk response renderer (array of success/error entries) with optional origin_identifier.
app/controllers/api/lessons_controller.rb Adds bulk-create branching, shared strong params helper, and bulk school_class validation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread app/controllers/api/lessons_controller.rb Outdated
Comment thread app/controllers/api/lessons_controller.rb Outdated
Comment thread spec/features/lesson/creating_a_lesson_spec.rb Outdated
Comment thread app/views/api/lessons/create_bulk.json.jbuilder Outdated
@PetarSimonovic PetarSimonovic force-pushed the bulk_lesson_creation branch from 7cfd87f to 129c583 Compare June 8, 2026 12:47
@PetarSimonovic PetarSimonovic temporarily deployed to editor-api-p-bulk-lesso-p15bjq June 8, 2026 12:47 Inactive
@PetarSimonovic PetarSimonovic force-pushed the bulk_lesson_creation branch from 129c583 to a09471d Compare June 8, 2026 12:53
@PetarSimonovic PetarSimonovic temporarily deployed to editor-api-p-bulk-lesso-p15bjq June 8, 2026 12:53 Inactive
@PetarSimonovic PetarSimonovic force-pushed the bulk_lesson_creation branch from a09471d to b50e323 Compare June 8, 2026 13:42
@PetarSimonovic PetarSimonovic temporarily deployed to editor-api-p-bulk-lesso-p15bjq June 8, 2026 13:43 Inactive
@PetarSimonovic PetarSimonovic force-pushed the bulk_lesson_creation branch from b50e323 to cda64cf Compare June 8, 2026 14:11
@PetarSimonovic PetarSimonovic temporarily deployed to editor-api-p-bulk-lesso-izlje0 June 8, 2026 14:12 Inactive
@@ -4,6 +4,23 @@ module Api
class LessonsController < ApiController

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This controller is getting longer now and I think having different code paths for the create action and many of the helpers does make it harder to follow and easier to introduce bugs (although I like having the attributes shared between them as it means that they are updated together).

Do you think it's worth improving this as part of this change? I wondered if it would be better or worse to have a different bulk controller, or if other behaviour could be moved outside of the controller.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've separated bulk creation into its own controller.

I've also created a LessonCreation concern for params and logic that are shared between single and batch lesson creation.

Let me know if LessonCreation makes it harder to follow the behaviour and I can move those methods back into the controllers.

expect(response).to have_http_status(:created)
end

it 'responds 422 Unprocessable if school_class_id does not correspond to school_id' do

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if the school_class matches the school_id but neither are associated with the current user?

I haven't confirmed if this works or not in the existing create action - Cancancan should authorise the resource in create actions by creating a new record with the params - create_params or lesson_params here but this won't work with bulk_create_params. (see cancancan docs).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added a before_action to loop through and authorize! each lesson. I've updated the specs to check that the controller responds with 403 if the user does not belong to the school.

@PetarSimonovic PetarSimonovic temporarily deployed to editor-api-p-bulk-lesso-ckjt3r June 10, 2026 10:22 Inactive
@PetarSimonovic PetarSimonovic temporarily deployed to editor-api-p-bulk-lesso-syszee June 10, 2026 11:26 Inactive
Expand the LessonsController#create to accept a lesson_projects array in the request body and create multiple lessons in one request.

Add bulk_create_params to the LessonsController, permitting origin_identifier on each entry so the caller can correlate responses with the original projects.

Add Lesson::CreateBulk to create individual Lessons and echo origin_identifier on each result.
Separate batch lesson creation from single-lesson creation

Add LessonCreation concern to handle shared params and logic
@PetarSimonovic PetarSimonovic temporarily deployed to editor-api-p-bulk-lesso-6enry9 June 10, 2026 12:51 Inactive

@zetter-rpf zetter-rpf left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found the batch controller much easier to understand, thanks for splitting that out.

@PetarSimonovic PetarSimonovic merged commit f846fa5 into main Jun 11, 2026
5 checks passed
@PetarSimonovic PetarSimonovic deleted the bulk_lesson_creation branch June 11, 2026 07:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants