Skip to content

Student self enrolment active model update - #2

Merged
JosephKS10 merged 1 commit into
JosephKS10:feature/student_self_enrolmentfrom
nouri-devv:student_self_enrolment_active_model_update
Sep 16, 2025
Merged

Student self enrolment active model update#2
JosephKS10 merged 1 commit into
JosephKS10:feature/student_self_enrolmentfrom
nouri-devv:student_self_enrolment_active_model_update

Conversation

@nouri-devv

Copy link
Copy Markdown

Description

For this task I updated the TaskDefinition and TutorialStream models to add support for tutorial self-enrolment.
The change adds two new fields on the task_definitions table:

  • tutorial_self_enrolment_enabled → a boolean flag that turns the feature on/off
  • tutorial_self_enrolment_stream_id → a foreign key pointing to a tutorial stream that students can self-enrol into

I also added the associations between models, validation to make sure the enrolment stream belongs to the same unit, and a reverse association on the TutorialStream side.

Type of change

This is part of a new feature for the student self enrollment.

  • [ x] New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Manual Testing in Rails Console

  • Created a valid TaskDefinition with self-enrolment enabled → it saved successfully, associations linked up correctly.
  • Tried creating one pointing to a stream from a different unit → failed as expected with the correct error.

Checklist:

  • [ x] My code follows the style guidelines of this project
  • [ x] I have performed a self-review of my own code
  • [ x] I have commented my code, particularly in hard-to-understand areas
  • [ x] I have made corresponding changes to the documentation if appropriate
  • [ x] My changes generate no new warnings
  • [ x] I have added tests that prove my fix is effective or that my feature works
  • [ x] I have created or extended unit tests to address my new additions
  • [ x] New and existing unit tests pass locally with my changes
  • [ x] Any dependent changes have been merged and published in downstream modules

If you have any questions, please contact @macite or @jakerenzella.

Automated Test (test/models/task_definition_test.rb)

def test_tutorial_self_enrolment_valid_and_invalid
  unit = FactoryBot.create(:unit)
  activity = FactoryBot.create(:activity_type)
  stream1 = FactoryBot.create(:tutorial_stream, unit: unit, activity_type: activity)
  stream2 = FactoryBot.create(:tutorial_stream, unit: unit, activity_type: activity)

  # valid case, should save
  td_valid = TaskDefinition.new(
    unit: unit,
    tutorial_stream: stream1,
    tutorial_self_enrolment_enabled: true,
    tutorial_self_enrolment_stream: stream2,
    name: "ok task",
    abbreviation: "VAL1",
    weighting: 10,
    target_grade: 0,
    start_date: unit.start_date || Time.zone.today,
    target_date: Time.zone.today + 1.week
  )
  assert td_valid.valid?, "should be valid when stream is in the same unit"

  # invalid case (wrong unit)
  other_unit = FactoryBot.create(:unit)
  other_stream = FactoryBot.create(:tutorial_stream, unit: other_unit, activity_type: activity)
  td_invalid = td_valid.dup
  td_invalid.abbreviation = "INV1"
  td_invalid.tutorial_self_enrolment_stream = other_stream

  assert_not td_invalid.valid?, "should fail when stream from another unit"
  assert_includes td_invalid.errors[:tutorial_self_enrolment_stream], "must belong to the same unit"
end

@nouri-devv
nouri-devv changed the base branch from development to feature/student_self_enrolment September 14, 2025 02:56

@ibi420 ibi420 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hello Nouri-devv, I’ve tested this and can confirm it has been well implemented. All tests pass successfully, and the implementation is solid. Great work on this, and thank you for the opportunity to review your contribution.

@JosephKS10 JosephKS10 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Great work @nouri-devv!

I ran into a few testing issues on my side and wanted to share in case it helps the team:

  • Had to rebuild the Docker container with --no-cache to ensure a clean environment.

  • The test database was missing some critical records:

  1. Roles: Student (1), Tutor (2), Convenor (3), Admin (4), Auditor (5)
  2. TeachingPeriod: id=3
  3. Basic unit/tutorial stream data

To fix the errors, I manually added these records in the Rails console:

rails console

Roles

Role.find_or_create_by(id: 1) { |r| r.name = "Student"; r.description = "Student" }
Role.find_or_create_by(id: 2) { |r| r.name = "Tutor"; r.description = "Tutor" }
Role.find_or_create_by(id: 3) { |r| r.name = "Convenor"; r.description = "Convenor" }
Role.find_or_create_by(id: 4) { |r| r.name = "Admin"; r.description = "Admin" }
Role.find_or_create_by(id: 5) { |r| r.name = "Auditor"; r.description = "Auditor" }

**TeachingPeriod**
TeachingPeriod.create!(
  id: 3,
  period: 'T1-2025',
  start_date: '2025-01-01',
  end_date: '2025-04-30',
  year: 2025,
  active_until: '2025-12-31'
)

After this, I was able to successfully run the specific test:

rails test test/models/task_definition_test.rb:272

Everything looks good, great work!

@gauravmyana2002 gauravmyana2002 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey @nouri-devv nice work on updating the model

Did face the same issue as @JosephKS10 did but now it is solved so according to me everything looks good.
Thank you.

@JosephKS10
JosephKS10 merged commit 46fe809 into JosephKS10:feature/student_self_enrolment Sep 16, 2025
@nouri-devv

Copy link
Copy Markdown
Author

Thank you @JosephKS10 and @gauravmyana2002, yes me and Joseph went through that and narrowed down the issue. From my understanding, it seems that db:reset does not create the testing db. I think the issue Joseph was facing was a configuration issue beyond this work, I ran some other tests and was having the issue if the db:reset was ran before it.

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.

4 participants