You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
the application auth flow is now aligned with the JWT-based session model
preset editing now correctly detects parameter changes (audio, guest policy, general settings, etc.) and no longer shows a false “No changes made” notification when values were actually updated
the frontend has also been improved to provide a better and more consistent user experience
Changes Made
Updated the application auth state handling to work consistently with JWT-based sessions.
Fixed preset editing by keeping a safe snapshot of the original preset before opening the edit modal.
Improved change detection for preset subcategories by comparing the full preset configuration, not only the preset name.
Prevented false info toasts when preset parameters are modified and the save succeeds.
Improved the frontend design and user experience.
Testing
Verified login and authenticated navigation in the local VM.
Verified preset parameter updates are saved correctly.
Verified the false “No changes made” toast no longer appears for real preset changes.
Verified the frontend improvements and updated UI behavior.
Closes Issue(s)
N/A
Related Issue(s)
N/A
Types of changes
Bug fix (non-breaking change which fixes an issue)
We reviewed changes in 977eaab...40e1e4d on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.
Some issues found as part of this review are outside of the diff in this pull request and aren't shown in the inline review comments due to GitHub's API limitations. You can see those issues on the DeepSource dashboard.
AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.
The reason will be displayed to describe this comment to others. Learn more.
`App` has a cyclomatic complexity of 6 with "medium" risk
A function with high cyclomatic complexity can be hard to understand and
maintain. Cyclomatic complexity is a software metric that measures the number of
independent paths through a function. A higher cyclomatic complexity indicates
that the function has more decision points and is more complex.
The reason will be displayed to describe this comment to others. Learn more.
Function has a cyclomatic complexity of 6 with "medium" risk
A function with high cyclomatic complexity can be hard to understand and
maintain. Cyclomatic complexity is a software metric that measures the number of
independent paths through a function. A higher cyclomatic complexity indicates
that the function has more decision points and is more complex.
The reason will be displayed to describe this comment to others. Learn more.
`migrateLegacyAuth` has a cyclomatic complexity of 6 with "medium" risk
A function with high cyclomatic complexity can be hard to understand and
maintain. Cyclomatic complexity is a software metric that measures the number of
independent paths through a function. A higher cyclomatic complexity indicates
that the function has more decision points and is more complex.
• Align frontend/back-end auth flow with JWT-only sessions and proper Authorization propagation.
• Fix preset edits by snapshotting the original preset for reliable change detection.
• Update tests and dev configs (nginx/Vite/Yarn) to support the new behavior.
➖ May diverge from intended dev environment standardization
Recommendation: The unified AuthService storage + migration is the right direction because it centralizes JWT session handling and reduces split-brain auth state across the UI, Axios, and route guards. Consider explicitly confirming the Yarn PnP switch is intentional (and documented for contributors), since it can be disruptive; otherwise, keeping node-modules is a safer short-term choice.
Files changed (16) +168 / -97
Bug fix (9) +150 / -87
Session.phpMake session JWT-only and fix revoked-token cache check+2/-16
Make session JWT-only and fix revoked-token cache check
• Stops mirroring runtime session values into the framework SESSION bag and no-ops syncSessionState in JWT-only mode. Adjusts revoked token detection to treat cache misses correctly (false on miss).
App.tsxInitialize auth state from storage and harden permissions usage+8/-6
Initialize auth state from storage and harden permissions usage
• Bootstraps currentUser/currentSession/isLogged from AuthService to match persisted JWT sessions. Avoids crashes when permissions are missing and uses an auth-keyed UserContext provider to force consistent rerenders on auth transitions.
Presets.tsxSnapshot original preset before editing to detect real changes+13/-1
Snapshot original preset before editing to detect real changes
• Introduces a cloning helper and stores an originalPreset snapshot when opening the modal. Clones modal content and resets file state to prevent false "No changes" outcomes due to shared references.
AppHeader.tsxFix header side effects and base rendering on stored auth+19/-15
Fix header side effects and base rendering on stored auth
• Moves settings collection into a useEffect to avoid triggering state updates during render. Derives authentication state from stored user/session and safely displays the active user identity.
AppSider.tsxMake sider menu building resilient to missing permissions+1/-1
Make sider menu building resilient to missing permissions
• Passes a default empty permissions object into MenuService when the current user or permissions are absent. Avoids runtime errors when auth state is partially loaded.
AxiosInstance.tsCentralize Authorization header injection via AuthService+9/-17
Centralize Authorization header injection via AuthService
• Replaces manual localStorage parsing with AuthService session access. Simplifies token header construction and clears auth consistently on parsing errors.
auth.service.tsUnify auth storage and migrate legacy keys+95/-28
Unify auth storage and migrate legacy keys
• Adds a single 'auth' localStorage key with read/write helpers and migration from legacy 'user'/'session' keys. Hardens parsing, expiry handling, and permission access to reduce auth desync bugs.
menu.service.tsDefault permissions shape and safer action checks+2/-2
Default permissions shape and safer action checks
• Types permissions as Record<string, string[]> with a default empty object. Guards action checks with Array.isArray to avoid exceptions when permission payloads are malformed or missing.
LoginTest.phpUpdate login tests for in-memory JWT session user+2/-2
Update login tests for in-memory JWT session user
• Replaces assertions that relied on SESSION.user with assertions against the session object stored in the Registry. Aligns tests with JWT-only session semantics.
bbbeasy.confForward Authorization header and allow PnP/Vite dot-folders+3/-2
Forward Authorization header and allow PnP/Vite dot-folders
• Passes HTTP_AUTHORIZATION through fastcgi to support JWT auth in PHP. Updates dot-file deny rules to allow .yarn/ and .vite/ virtual folders required by dev mode.
Preset edits can still incorrectly show “no_changes” because editPreset returns early when the
preset name is unchanged, ignoring category/subcategory config updates. The new originalPreset
snapshot added in showModal is never used to fix this comparison, so the user-facing bug this PR
targets remains reproducible.
showModal now captures originalPreset, but the actual change detection in editPreset still
returns early based only on the name, which cannot detect parameter/category changes. Therefore the
new snapshot code does not prevent the false “no changes” notification.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
Preset editing still treats successful parameter updates as “no changes” because the comparison logic only checks `newPreset.name == oldPreset.name`. The PR adds an `originalPreset` snapshot but it is not used anywhere, so it doesn’t improve change detection.
## Issue Context
- `showModal` stores a cloned snapshot (`originalPreset`) but the save flow still passes `preset` as the “old” preset and `editPreset` only compares names.
- Subcategory changes typically do not rename presets, so the UI will continue to display the info toast and skip updating the presets state.
## Fix Focus Areas
- bbbeasy-frontend/src/components/Presets.tsx[162-173]
- bbbeasy-frontend/src/components/Presets.tsx[659-676]
## Implementation notes
- Update `editPreset` to compare the full preset configuration (e.g., deep-equal `categories` + `subcategories`, or compare a stable serialized representation).
- Alternatively, since the backend returned an updated preset, remove the name-only early-return and always update state + show success when the save API call succeeds.
- If keeping `originalPreset`, pass it as `oldPreset` from `PresetsCol` when invoking `editClickHandler` so the comparison is against an immutable snapshot.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
2. Yarn runtime path not tracked 🐞☼ Reliability
Description
Frontend config switches to nodeLinker: pnp while yarnPath points to
.yarn/releases/yarn-4.16.0.cjs, but .gitignore ignores .yarn/*, so fresh clones/CI can fail to
run Yarn unless that runtime is provisioned out-of-band. This PR increases the likelihood of
install/build failures by changing the linker mode without ensuring the configured Yarn runtime path
is available.
The repo explicitly configures a local Yarn runtime path, but also ignores .yarn/*, so the
configured Yarn runtime is not reliably present in source control; switching to PnP makes correct
Yarn setup more critical.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
The repo configures Yarn via `yarnPath: .yarn/releases/yarn-4.16.0.cjs` and now switches to `nodeLinker: pnp`. However, `.gitignore` ignores `.yarn/*`, which prevents the configured Yarn runtime from being committed by default; if the runtime isn’t present, Yarn execution/install can fail on clean environments.
## Issue Context
- Yarn Berry expects the `yarnPath` target to exist when Yarn starts.
- Ignoring `.yarn/*` means the critical `releases/` file is easy to omit from the repo.
## Fix Focus Areas
- bbbeasy-frontend/.yarnrc.yml[6-10]
- bbbeasy-frontend/.gitignore[30-31]
## Implementation notes
Choose one:
1) Commit the Yarn release file and narrow the ignore rules (e.g., ignore caches but keep `.yarn/releases/**`).
2) Remove `yarnPath` and rely on Corepack-managed Yarn, documenting the required version.
3) If CI provisions Yarn runtime separately, document that requirement explicitly and ensure pipelines install/copy the runtime before running `yarn`.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
3. Null session user indexing 🐞☼ Reliability
Description
StartTest/ViewTest now index into $currentUser['id'] without verifying the session returned an
array, which can trigger warnings/failures when no user is loaded into the session.
Session::get('user') explicitly returns null when currentUser is not set.
The session accessor returns null when unauthenticated, but the updated tests treat it as an array
and dereference ['id'] immediately, creating a null/offset hazard.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
Room tests assume `Registry::get('session')->get('user')` is always a non-null array and immediately access `$currentUser['id']`. When no authenticated user is present, this becomes an unsafe array-offset access.
## Issue Context
`Core\Session::get('user')` returns `null` if there is no authenticated `currentUser`, so tests should either (a) ensure login happens in their own setup, or (b) assert/fail clearly when `currentUser` is missing.
## Fix Focus Areas
- bbbeasy-backend/tests/src/Actions/Rooms/StartTest.php[69-72]
- bbbeasy-backend/tests/src/Actions/Rooms/ViewTest.php[69-72]
- bbbeasy-backend/app/src/Core/Session.php[99-111]
## Implementation notes
- In each test, add an explicit login/setup step before reading session user.
- Or add a guard:
- if `$currentUser === null` (or not array / missing `id`), fail the test with a clear message.
- then proceed to load the user.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
The reason will be displayed to describe this comment to others. Learn more.
1. Preset changes still ignored 🐞 Bug≡ Correctness
Preset edits can still incorrectly show “no_changes” because editPreset returns early when the
preset name is unchanged, ignoring category/subcategory config updates. The new originalPreset
snapshot added in showModal is never used to fix this comparison, so the user-facing bug this PR
targets remains reproducible.
Agent Prompt
## Issue description
Preset editing still treats successful parameter updates as “no changes” because the comparison logic only checks `newPreset.name == oldPreset.name`. The PR adds an `originalPreset` snapshot but it is not used anywhere, so it doesn’t improve change detection.
## Issue Context
- `showModal` stores a cloned snapshot (`originalPreset`) but the save flow still passes `preset` as the “old” preset and `editPreset` only compares names.
- Subcategory changes typically do not rename presets, so the UI will continue to display the info toast and skip updating the presets state.
## Fix Focus Areas
- bbbeasy-frontend/src/components/Presets.tsx[162-173]
- bbbeasy-frontend/src/components/Presets.tsx[659-676]
## Implementation notes
- Update `editPreset` to compare the full preset configuration (e.g., deep-equal `categories` + `subcategories`, or compare a stable serialized representation).
- Alternatively, since the backend returned an updated preset, remove the name-only early-return and always update state + show success when the save API call succeeds.
- If keeping `originalPreset`, pass it as `oldPreset` from `PresetsCol` when invoking `editClickHandler` so the comparison is against an immutable snapshot.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
The reason will be displayed to describe this comment to others. Learn more.
2. Null session user indexing 🐞 Bug☼ Reliability
StartTest/ViewTest now index into $currentUser['id'] without verifying the session returned an
array, which can trigger warnings/failures when no user is loaded into the session.
Session::get('user') explicitly returns null when currentUser is not set.
Agent Prompt
## Issue description
Room tests assume `Registry::get('session')->get('user')` is always a non-null array and immediately access `$currentUser['id']`. When no authenticated user is present, this becomes an unsafe array-offset access.
## Issue Context
`Core\Session::get('user')` returns `null` if there is no authenticated `currentUser`, so tests should either (a) ensure login happens in their own setup, or (b) assert/fail clearly when `currentUser` is missing.
## Fix Focus Areas
- bbbeasy-backend/tests/src/Actions/Rooms/StartTest.php[69-72]
- bbbeasy-backend/tests/src/Actions/Rooms/ViewTest.php[69-72]
- bbbeasy-backend/app/src/Core/Session.php[99-111]
## Implementation notes
- In each test, add an explicit login/setup step before reading session user.
- Or add a guard:
- if `$currentUser === null` (or not array / missing `id`), fail the test with a clear message.
- then proceed to load the user.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
The reason will be displayed to describe this comment to others. Learn more.
3. Yarn runtime path not tracked 🐞 Bug☼ Reliability
Frontend config switches to nodeLinker: pnp while yarnPath points to
.yarn/releases/yarn-4.16.0.cjs, but .gitignore ignores .yarn/*, so fresh clones/CI can fail to
run Yarn unless that runtime is provisioned out-of-band. This PR increases the likelihood of
install/build failures by changing the linker mode without ensuring the configured Yarn runtime path
is available.
Agent Prompt
## Issue description
The repo configures Yarn via `yarnPath: .yarn/releases/yarn-4.16.0.cjs` and now switches to `nodeLinker: pnp`. However, `.gitignore` ignores `.yarn/*`, which prevents the configured Yarn runtime from being committed by default; if the runtime isn’t present, Yarn execution/install can fail on clean environments.
## Issue Context
- Yarn Berry expects the `yarnPath` target to exist when Yarn starts.
- Ignoring `.yarn/*` means the critical `releases/` file is easy to omit from the repo.
## Fix Focus Areas
- bbbeasy-frontend/.yarnrc.yml[6-10]
- bbbeasy-frontend/.gitignore[30-31]
## Implementation notes
Choose one:
1) Commit the Yarn release file and narrow the ignore rules (e.g., ignore caches but keep `.yarn/releases/**`).
2) Remove `yarnPath` and rely on Corepack-managed Yarn, documenting the required version.
3) If CI provisions Yarn runtime separately, document that requirement explicitly and ensure pipelines install/copy the runtime before running `yarn`.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
The reason will be displayed to describe this comment to others. Learn more.
Arrow function expected no return value
Any code paths that do not have explicit returns will return undefined. It is recommended to replace any implicit dead-ends that return undefined with a return null statement.
The reason will be displayed to describe this comment to others. Learn more.
JSX tree is too deeply nested. Found 11 levels of nesting
Nesting JSX elements too deeply can confuse developers reading the code. To make maintenance and refactoring easier, DeepSource recommends limiting the maximum JSX tree depth to 4.
The reason will be displayed to describe this comment to others. Learn more.
Expected property shorthand
ECMAScript 6 provides a concise form for defining object literal methods and properties. This syntax can make defining complex object literals much cleaner.
The reason will be displayed to describe this comment to others. Learn more.
JSX tree is too deeply nested. Found 6 levels of nesting
Nesting JSX elements too deeply can confuse developers reading the code. To make maintenance and refactoring easier, DeepSource recommends limiting the maximum JSX tree depth to 4.
The reason will be displayed to describe this comment to others. Learn more.
JSX tree is too deeply nested. Found 6 levels of nesting
Nesting JSX elements too deeply can confuse developers reading the code. To make maintenance and refactoring easier, DeepSource recommends limiting the maximum JSX tree depth to 4.
The reason will be displayed to describe this comment to others. Learn more.
JSX tree is too deeply nested. Found 5 levels of nesting
Nesting JSX elements too deeply can confuse developers reading the code. To make maintenance and refactoring easier, DeepSource recommends limiting the maximum JSX tree depth to 4.
The reason will be displayed to describe this comment to others. Learn more.
Value must be omitted for boolean attribute `scrollToFirstError`
When using a boolean attribute in JSX, you can set the attribute value to true or omit the value. This helps to keep consistency in code.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
![deepsource-io[bot]](https://avatars.githubusercontent.com/in/16372?s=60&v=4){kind=small}
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Description
This PR fixes two user-facing issues in BBBEasy:
Changes Made
Testing
Closes Issue(s)
Related Issue(s)
Types of changes