Skip to content

Improve room require code error handling#3035

Open
Sabr1n4W wants to merge 6 commits intodevelopfrom
improve-room-require-code-error-handling
Open

Improve room require code error handling#3035
Sabr1n4W wants to merge 6 commits intodevelopfrom
improve-room-require-code-error-handling

Conversation

@Sabr1n4W
Copy link
Copy Markdown
Contributor

@Sabr1n4W Sabr1n4W commented Apr 9, 2026

Fixes #

Type

  • Bugfix
  • Feature
  • Documentation
  • Refactoring (e.g. Style updates, Test implementation, etc.)
  • Other (please describe):

Checklist

  • Code updated to current develop branch head
  • Passes CI checks
  • Is a part of an issue
  • Tests added for the bugfix or newly implemented feature, describe below why if not
  • Changelog is updated
  • Documentation of code and features exists

Changes

  • Improved require code handling by showing a more specific error message and not showing invalid code error message and error state in access code overlay

Other information

Summary by CodeRabbit

  • Bug Fixes

    • Clearer handling between missing room access codes and invalid codes; UI now shows a "require access code" message and resets the access-code input on reload.
    • Reload flow adjusted to avoid re-triggering invalid-code detection.
  • Documentation

    • Changelog updated to document the improved access-code error handling.
  • Tests

    • Frontend tests updated to expect the new "require access code" behavior and input reset.

@Sabr1n4W Sabr1n4W self-assigned this Apr 9, 2026
@Sabr1n4W Sabr1n4W added the frontend Pull requests that update Javascript code label Apr 9, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 9, 2026

Warning

Rate limit exceeded

@Sabr1n4W has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 15 minutes and 29 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 15 minutes and 29 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 0dce98e8-ce8a-4a50-99f8-481f7c970271

📥 Commits

Reviewing files that changed from the base of the PR and between eb78d8e and 1aa9d77.

📒 Files selected for processing (1)
  • CHANGELOG.md

Walkthrough

Adds a new requireCode emit to room-related Vue components, switches handling of 403+HTTP_ROOM_REQUIRE_CODE to emit requireCode, and wires a new RoomsView handler to clear access-code state, show a rooms.require_access_code toast, and optionally reload room details. Tests updated accordingly.

Changes

Cohort / File(s) Summary
Documentation
CHANGELOG.md
Added Unreleased "Changed" entry documenting improved handling when a room requires an access code and added link reference for #3035.
Room access components
resources/js/components/RoomJoinButton.vue, resources/js/components/RoomTabFiles.vue, resources/js/components/RoomTabRecordings.vue
Added requireCode to defineEmits and changed handlers to emit requireCode instead of invalidRoomAuthToken for HTTP 403 + HTTP_ROOM_REQUIRE_CODE cases.
Room tab wrapper
resources/js/components/RoomTabSection.vue
Added requireCode to defineEmits, propagated child requireCode events upward via @require-code="$emit('requireCode')".
Main room view
resources/js/views/RoomsView.vue
Wired @require-code on RoomJoinButton and RoomTabSection; added handleRequireCode(reloadRoomDetails = true) and changed reload() signature to reload(checkForRequireCodeError = true) to detect auth->require-code transitions and clear access-code UI/state while showing rooms.require_access_code.
E2E tests
tests/Frontend/e2e/RoomsView*.cy.js
Replaced click flows to use [data-test="reload-room-button"], removed some @roomAuthRequest waits, updated assertions to expect room_auth_token/room_auth_token_type undefined, changed toast checks from rooms.flash.access_code_invalid to rooms.require_access_code, and asserted access-code input is cleared.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested labels

UI, refactor

Suggested reviewers

  • danielmachill
  • samuelwei
🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description is largely incomplete. The required 'Fixes #' field is empty, the Type section lacks proper highlighting formatting, only one Change item is provided instead of a detailed list, and critical checklist items (CI passing, issue association, documentation) remain unchecked. Add issue number to 'Fixes' field, properly highlight 'Refactoring', expand Changes section with bullet points for each modification, and verify/update remaining checklist items before merge.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: improving error handling for room require-code scenarios. It is concise, clear, and directly related to the changeset's primary focus.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch improve-room-require-code-error-handling

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.

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 9, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.15%. Comparing base (3c8a92f) to head (1aa9d77).

Additional details and impacted files
@@              Coverage Diff              @@
##             develop    #3035      +/-   ##
=============================================
+ Coverage      96.73%   97.15%   +0.41%     
=============================================
  Files            457      179     -278     
  Lines          12988     6327    -6661     
  Branches        2079     2087       +8     
=============================================
- Hits           12564     6147    -6417     
+ Misses           424      180     -244     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@cypress
Copy link
Copy Markdown

cypress bot commented Apr 9, 2026

PILOS    Run #2943

Run Properties:  status check passed Passed #2943  •  git commit 1aa9d77a2d: Improve room require code error handling
Project PILOS
Branch Review improve-room-require-code-error-handling
Run status status check passed Passed #2943
Run duration 07m 14s
Commit git commit 1aa9d77a2d: Improve room require code error handling
Committer Sabrina Wüst
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
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 622
View all changes introduced in this branch ↗︎

@Sabr1n4W Sabr1n4W marked this pull request as ready for review April 10, 2026 07:09
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@resources/js/views/RoomsView.vue`:
- Around line 407-420: The handler handleRequireCode resets the UI but leaves
the stale roomAuthToken in place so the subsequent reload() call will still send
the old token; clear or set roomAuthToken to null/empty (or call the function
that resets it) before calling reload(false) so the follow-up GET doesn't
include the stale token and the server sees the updated "code required" state;
update handleRequireCode to reset the roomAuthToken variable (or invoke the
existing resetAuthToken helper) immediately prior to reload().
- Around line 528-535: The check is reading room.value?.authenticated after an
async reload which lets concurrent reloads both see the same old state and call
handleRequireCode(false) twice; capture the prior auth state into a local
variable before awaiting the reload (e.g., const prevAuthenticated =
room.value?.authenticated) and use that local (prevAuthenticated) in the
condition that uses checkForRequireCodeError and
response.data.data.authenticated, so handleRequireCode(false) only runs when the
single captured previous state indicates authenticated and the new response says
unauthenticated.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 3ae73c31-b2e9-4a64-bacd-ead00b82c067

📥 Commits

Reviewing files that changed from the base of the PR and between 3688a9f and eb78d8e.

📒 Files selected for processing (1)
  • resources/js/views/RoomsView.vue

Comment on lines +407 to +420
function handleRequireCode(reloadRoomDetails = true) {
// Reset access code error states to prevent confusing error state
accessCodeInvalid.value = null;
formErrors.clear();

// Reset access code input
accessCodeInput.value = "";

// Show error message
toast.error(t("rooms.require_access_code"));

if (reloadRoomDetails) {
reload(false);
}
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.

⚠️ Potential issue | 🟠 Major

Clear the stale room auth token before reloading.

This handler resets the overlay state, but reload() will still send the old roomAuthToken on the very next GET. That keeps the follow-up request out of sync with the new “code required again” state and can re-surface the wrong auth flow later.

💡 Suggested change
 function handleRequireCode(reloadRoomDetails = true) {
+  roomAuthToken.value = null;
+
   // Reset access code error states to prevent confusing error state
   accessCodeInvalid.value = null;
   formErrors.clear();
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
function handleRequireCode(reloadRoomDetails = true) {
// Reset access code error states to prevent confusing error state
accessCodeInvalid.value = null;
formErrors.clear();
// Reset access code input
accessCodeInput.value = "";
// Show error message
toast.error(t("rooms.require_access_code"));
if (reloadRoomDetails) {
reload(false);
}
function handleRequireCode(reloadRoomDetails = true) {
roomAuthToken.value = null;
// Reset access code error states to prevent confusing error state
accessCodeInvalid.value = null;
formErrors.clear();
// Reset access code input
accessCodeInput.value = "";
// Show error message
toast.error(t("rooms.require_access_code"));
if (reloadRoomDetails) {
reload(false);
}
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@resources/js/views/RoomsView.vue` around lines 407 - 420, The handler
handleRequireCode resets the UI but leaves the stale roomAuthToken in place so
the subsequent reload() call will still send the old token; clear or set
roomAuthToken to null/empty (or call the function that resets it) before calling
reload(false) so the follow-up GET doesn't include the stale token and the
server sees the updated "code required" state; update handleRequireCode to reset
the roomAuthToken variable (or invoke the existing resetAuthToken helper)
immediately prior to reload().

Comment on lines +528 to +535
// Room was authenticated but now requires an access code
if (
checkForRequireCodeError &&
room.value?.authenticated &&
!response.data.data.authenticated
) {
handleRequireCode(false);
}
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.

⚠️ Potential issue | 🟡 Minor

Use the pre-refresh auth state only once.

The transition check runs before room.value is updated. If two reload() calls resolve back-to-back, both handlers can still see the old authenticated state and fire handleRequireCode(false), which duplicates the toast/reset.

💡 Suggested change
     .then((response) => {
-      // Room was authenticated but now requires an access code
-      if (
-        checkForRequireCodeError &&
-        room.value?.authenticated &&
-        !response.data.data.authenticated
-      ) {
-        handleRequireCode(false);
-      }
-
-      room.value = response.data.data;
+      const wasAuthenticated = room.value?.authenticated;
+      room.value = response.data.data;
+
+      // Room was authenticated but now requires an access code
+      if (
+        checkForRequireCodeError &&
+        wasAuthenticated &&
+        !room.value.authenticated
+      ) {
+        handleRequireCode(false);
+      }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@resources/js/views/RoomsView.vue` around lines 528 - 535, The check is
reading room.value?.authenticated after an async reload which lets concurrent
reloads both see the same old state and call handleRequireCode(false) twice;
capture the prior auth state into a local variable before awaiting the reload
(e.g., const prevAuthenticated = room.value?.authenticated) and use that local
(prevAuthenticated) in the condition that uses checkForRequireCodeError and
response.data.data.authenticated, so handleRequireCode(false) only runs when the
single captured previous state indicates authenticated and the new response says
unauthenticated.

…re-code-error-handling

# Conflicts:
#	CHANGELOG.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

frontend Pull requests that update Javascript code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant