Skip to content

Course seat management#187

Open
g-bar wants to merge 225 commits into
mainfrom
course-seat-management
Open

Course seat management#187
g-bar wants to merge 225 commits into
mainfrom
course-seat-management

Conversation

@g-bar

@g-bar g-bar commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

This PR implements most of the functionality required for the education plan: https://github.com/openbraininstitute/PMO/issues/96

Courses Provisioning

Relationships

                        ┌─────────────┐
                        │ Institution │
                        └──────┬──────┘
                               │ 1
                               │
                               *
┌─────────────┐         ┌─────────────┐
│ Virtual Lab │1──────1 │ Course                                                    │
└─────────────┘         └──────┬──────┘
                               │ 1
                               │
                               *
                        ┌─────────────┐
                        │    Seat     │
                        └──────┬──────┘
                               │ 0..1
                               │
                               1
┌─────────────┐         ┌─────────────┐
│   Project   │1───────1│  Enrolment  │
└─────────────┘         └─────────────┘

Course Lifecycle

Provisioning

  1. OBI admin creates a virtual lab belonging to obi-virtual-lab.
    POST /virtual-labs
  2. OBI admin creates a template project for that virtual lab.
    POST /virtual-labs/{vlab_id}/projects
  3. OBI admin creates a course pointing to the virtual lab and template project.
    POST /courses
  4. OBI admin sets start_date, last_drop_date, and end_date.
    PATCH /courses/{course_id}
  5. OBI admin activates the course (all dates must be set).
    POST /courses/{course_id}/activate
  6. OBI admin invites faculty as virtual lab admin.
    POST /virtual-labs/{vlab_id}/invites
  7. OBI admin provisions seats (only active courses can be provisioned).
    POST /seats/provision

Enrolment

  1. Faculty assigns available seats to students (identified by student id + email).
    POST /seats/courses/{course_id}/assign
    This creates an enrolment and a project per student and sends an invite email.

  2. Student claims the enrolment, storing their user_id.
    POST /courses/{course_id}/claim

  3. On login (after start_date, before end_date), the student's enrolment is activated (added to KC groups).
    POST /courses/activate-enrolments

Dropping

Faculty can drop a student and recover the seat if all conditions are met:

  • seat.previously_dropped == False
  • now < course.last_drop_date
  • The student's project has spent fewer than 50 credits.

POST /seats/courses/{course_id}/drop

When dropped, the student's project budget is depleted and the student is removed from KC groups (vlab and project).

Course Expiry

A daily cronjob checks courses whose end_date has passed, drops every remaining student, and depletes all credits from all projects and the virtual lab.

@g-bar g-bar mentioned this pull request Jun 23, 2026

@pgetta pgetta 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.

Great work Gil!

I've added some comments and questions.

Comment thread virtual_labs/usecases/course/expire_courses.py
Comment thread virtual_labs/usecases/course/assign_seats.py Outdated
Comment thread alembic/versions/10e0d7bf4813_add_credit_value_to_seat.py Outdated
Comment thread virtual_labs/infrastructure/settings.py
Comment thread virtual_labs/domain/institution.py
pgetta
pgetta previously approved these changes Jun 26, 2026

@pgetta pgetta 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.

Thank you Gil.

I'm good with the change, but I'll let you finish the discussion with @bilalesi.

As for the institutions - we can revise that later, either future use cases will confirm they are needed or we just refactor and delete them.

@bilalesi bilalesi left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Great work @g-bar,

about your question about the ledger:
think of it like an "undo notebook." when the use case is doing a bunch of steps, creating a Keycloak group, setting up accounting, create seat, ..., each step pushes an undo action onto the ledger. If something blows up halfway, the ledger runs all those undos in reverse (last one first), so you don't end up with half-created stuff floating around

the pattern can be applied to any use cases, you just need to figure out how you order, function implementation, and either throw directly/postpone, this make the mental model and steps of creation very clear both for us and the machine, the other nice thing about the ledger is to have domain errors with the translator which can be very helpful in debugging

still have few comments/questions but it can be in another improvement PR/chat

Comment thread virtual_labs/domain/course.py Outdated
Comment thread virtual_labs/domain/institution.py
Comment thread virtual_labs/infrastructure/email/send_enrolment_claim_email.py Outdated
Comment thread virtual_labs/usecases/seat/list_seats.py Outdated
Comment on lines +82 to +84
async def assign_seats(
db: AsyncSession,
*,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

i would also suggest to see the ledger and compensation tools,
the tool is allow you to create actions and reverse actions, all managed by the ledger (virtual lab creation)
here the catch path only soft-deletes the project and does not reverse/deplete the granted credits

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 refactored this to use the ledger now I create the project, init accounting, fund project, create an enrolment and assign a seat using the ledger to roll back failed ops, all while not committing anything to the db until after all iterations are done to avoid releasing the seats prematurely.

Comment on lines +68 to +70
async def create_course(
db: AsyncSession,
payload: CourseCreateBody,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this is also best use case for the ledger pattern

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.

this one not quite, the create_course doesnt create a virtual lab, it merely points to an existing virtual lab created previously.

there is nothing to clean up in case of failure, merely the course row doesnt'get created the virtual lab and project remain valid.

Comment on lines +109 to +115
# Post-commit: refresh vlab (now has course relationship) and fund template project
await db.refresh(vlab)
await accounting_cases.fund_project(
virtual_lab_id=vlab.id,
project_id=payload.template_project_id,
amount=settings.CREDITS_PER_SEAT,
)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

the endpoint can create a project even the funding was not successful, is that okey ?

@g-bar g-bar Jun 29, 2026

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.

same as above, the project doesn't get created here.

@github-actions

Copy link
Copy Markdown

/opt/hostedtoolcache/Python/3.12.13/x64/lib/python3.12/site-packages/safety/auth/main.py:6: AuthlibDeprecationWarning: authlib.jose module is deprecated, please use joserfc instead.
It will be compatible before version 2.0.0.
from authlib.jose import jwt

poetry audit report

Loading...
Scanning 109 packages...

• starlette installed 0.52.1 affected <1.0.1 CVE CVE-2026-48710

1 vulnerabilities found in 1 packages

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants