Skip to content

Lms id fix#329

Open
noahnizamian wants to merge 6 commits intomainfrom
lms-id-fix
Open

Lms id fix#329
noahnizamian wants to merge 6 commits intomainfrom
lms-id-fix

Conversation

@noahnizamian
Copy link

General Info

CDSS issue: berkeley-cdss#304

Changes

New controller with role based auth
Added teacher? method to UserToCourse model for role checking
Uses proper foreign key instead of possibly invalid string LMS references
LMS information can be centralized in the LMS table and changes now occur in a single place

Testing

Run the full rspec test suite to test for changes

Documentation

No documentation needed

Checklist

  • Name of branch corresponds to story
  • 80%+ test coverage with all tests passing

- Implemented user_to_courses_controller.rb with role-based authorization
- PATCH endpoint to toggle allow_extended_requests on enrollments
- Authorization: only teachers can toggle
- Uses lms_id FK pattern from LMS credentials refactoring
- Added teacher? method to UserToCourse model for role checking
- Complete spec with 7 test scenarios (instructor, student, missing resources)
- All tests passing: 365 examples, 0 failures, 80.88% coverage
@noahnizamian noahnizamian self-assigned this Mar 6, 2026
@@ -1 +1 @@
ruby 3.3.8

Choose a reason for hiding this comment

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

We don't need to change the ruby version. 3.3 seems to work well in production

gem 'lograge'

# Environment variable management
gem 'dotenv-rails', require: 'dotenv/load'

Choose a reason for hiding this comment

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

These are specific for development environment

diff-lcs (1.6.2)
docile (1.4.1)
domain_name (0.6.20240107)
dotenv (3.2.0)

Choose a reason for hiding this comment

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

These are specific for development environment

cucumber-rails
database_cleaner-active_record
debug
dotenv-rails

Choose a reason for hiding this comment

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

Not needed for production app

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.

2 participants