Skip to content

WIP: allow lecturers to access assessment live quizzes for testing purposes#4930

Draft
sjschlapbach wants to merge 2 commits intov3from
lecturer-live-quiz-access
Draft

WIP: allow lecturers to access assessment live quizzes for testing purposes#4930
sjschlapbach wants to merge 2 commits intov3from
lecturer-live-quiz-access

Conversation

@sjschlapbach
Copy link
Copy Markdown
Member

No description provided.

@sjschlapbach sjschlapbach marked this pull request as draft September 19, 2025 14:51
@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Sep 19, 2025
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Sep 19, 2025

Important

Review skipped

Ignore keyword(s) in the title.

⛔ Ignored keywords (1)
  • WIP

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@dosubot dosubot bot added the feature label Sep 19, 2025
@sonarqubecloud
Copy link
Copy Markdown

@cypress
Copy link
Copy Markdown

cypress bot commented Sep 19, 2025

klicker-uzh    Run #6399

Run Properties:  status check passed Passed #6399  •  git commit 398dfd99a6 ℹ️: Merge d70f0c5a48c740f781a936b934a23fb0f8b43550 into 2ba1da0c999efb7ac3b673cc01cb...
Project klicker-uzh
Branch Review lecturer-live-quiz-access
Run status status check passed Passed #6399
Run duration 10m 23s
Commit git commit 398dfd99a6 ℹ️: Merge d70f0c5a48c740f781a936b934a23fb0f8b43550 into 2ba1da0c999efb7ac3b673cc01cb...
Committer Julius Schlapbach
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 1
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 758
⚠️ You've recorded test results over your free plan limit.
Upgrade your plan to view test results.
View all changes introduced in this branch ↗︎

@claude
Copy link
Copy Markdown

claude bot commented Sep 19, 2025

Code Review

🚨 Blocking Issues

  • Debug console.log statements left in code → Fix: Remove all TODO-marked console.log statements in apps/backend-docker/src/app.ts:68-109 and packages/graphql/src/services/liveQuizzes.ts:2825,2860
  • Security bypass without proper validation → Fix: Add explicit role/permission checks before allowing lecturer access to assessment quizzes (currently any co-creator gets access regardless of context)

📚 Documentation Gaps

No new documentation gaps identified beyond what's already covered.

🧪 Test Coverage Gaps

  • Missing e2e flow: lecturer accessing assessment live quiz → Add to: cypress/e2e/manage/assessment-quiz-lecturer-access.cy.ts
    Implementation:
    1. Setup: Login as lecturer with co-creator permissions
    2. Test: Navigate to assessment live quiz from manage interface
    3. Test: Verify participant view loads correctly
    4. Test: Attempt submission (should be blocked/tracked differently)
    5. Verify: Access logged but responses not counted as participant
    

⚠️ Unique Architectural Insights

  • Mixing authentication contexts creates audit trail issues → Consider: Add explicit accessMode field to track whether access is as participant or lecturer for proper activity logging
  • Cookie fallback pattern may cause unintended access → Consider: Use separate cookie name or header for lecturer test access to maintain clear separation

@rschlaefli rschlaefli self-assigned this Sep 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature size:M This PR changes 30-99 lines, ignoring generated files.

Development

Successfully merging this pull request may close these issues.

2 participants