Conversation
Allow users to select a branch when configuring the Tolgee chrome plugin. The branch field is only shown when branching is enabled on the project, determined by the branchingEnabled flag from the API key validation response.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds optional branch support across the extension: new constant, branch persisted in sessionStorage by the content script, branch persisted/loaded in popup storage, branch selection UI in the popup, and types/state updates to carry branch and branchingEnabled info. Changes
Sequence Diagram(s)sequenceDiagram
participant Popup as Popup UI
participant Storage as Popup Storage
participant Detector as useDetectorForm
participant Content as Content Script
participant Session as sessionStorage
Popup->>Storage: saveValues(apiUrl, apiKey, branch?)
Storage-->>Popup: confirm saved
Popup->>Detector: APPLY_VALUES (includes branch if branchingEnabled)
Detector->>Content: POST SET_CREDENTIALS {apiKey, apiUrl, branch?}
Content->>Session: if branch -> setItem('__tolgee_branch', branch)
Content->>Session: else -> removeItem('__tolgee_branch')
Content-->>Detector: acknowledge
Detector->>Popup: refresh UI (may fetch branches)
Detector->>Storage: loadValues() -> returns branch?
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
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. Comment |
Contributor
Author
Related PRs |
This was referenced Mar 16, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/popup/tools.ts (1)
18-20:⚠️ Potential issue | 🟠 MajorFix
compareValuesequality logic (URL typo + missing branch comparison).The current condition has a tautology (
values2.apiUrl === values2.apiUrl), so URL changes are ignored. With branch now part ofValues, branch equality should be explicit too.Suggested fix
export const compareValues = ( values1?: Values | null, values2?: Values | null ) => { return ( - values1?.apiKey === values2?.apiKey && values2?.apiUrl === values2?.apiUrl + values1?.apiKey === values2?.apiKey && + values1?.apiUrl === values2?.apiUrl && + (values1?.branch || '') === (values2?.branch || '') ); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/popup/tools.ts` around lines 18 - 20, The equality check in compareValues currently contains a tautology (values2.apiUrl === values2.apiUrl) and misses comparing the new branch field; update compareValues to explicitly compare values1.apiKey === values2.apiKey, values1.apiUrl === values2.apiUrl, and values1.branch === values2.branch (handling possible undefined/nulls consistently) so all three fields from the Values type are properly compared.src/popup/useDetectorForm.tsx (1)
103-117:⚠️ Potential issue | 🟠 MajorDon’t persist/apply
branchwhen branching is disabled.
APPLY_VALUESalways forwardsstate.values.branch. When branching is disabled, the field is hidden in UI, so stale branch values can continue being sent without user control.Suggested fix
- case 'APPLY_VALUES': - // sync values with storage/localStorage - apply(); - return { + case 'APPLY_VALUES': { + // sync values with storage/localStorage + apply(); + const branchEnabled = + typeof state.credentialsCheck === 'object' && + state.credentialsCheck?.branchingEnabled; + const effectiveBranch = branchEnabled ? state.values?.branch : undefined; + return { ...state, appliedValues: { apiKey: state.values?.apiKey, apiUrl: state.values?.apiUrl, - branch: state.values?.branch, + branch: effectiveBranch, }, storedValues: { apiKey: state.values?.apiKey, apiUrl: state.values?.apiUrl, - branch: state.values?.branch, + branch: effectiveBranch, }, }; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/popup/useDetectorForm.tsx` around lines 103 - 117, APPLY_VALUES currently always copies state.values.branch into appliedValues and storedValues which can persist a stale branch when branching is disabled; change the reducer handling case 'APPLY_VALUES' (and the apply() call if it writes storage) to conditionally include the branch property only when the branching-enabled flag is true (e.g., state.branchingEnabled or state.hasBranching) — when branching is disabled omit branch from appliedValues and storedValues and explicitly clear any stored branch value so it is not sent; update the construction of appliedValues/storedValues in the APPLY_VALUES case to build the object conditionally based on that flag.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/popup/tools.ts`:
- Around line 18-20: The equality check in compareValues currently contains a
tautology (values2.apiUrl === values2.apiUrl) and misses comparing the new
branch field; update compareValues to explicitly compare values1.apiKey ===
values2.apiKey, values1.apiUrl === values2.apiUrl, and values1.branch ===
values2.branch (handling possible undefined/nulls consistently) so all three
fields from the Values type are properly compared.
In `@src/popup/useDetectorForm.tsx`:
- Around line 103-117: APPLY_VALUES currently always copies state.values.branch
into appliedValues and storedValues which can persist a stale branch when
branching is disabled; change the reducer handling case 'APPLY_VALUES' (and the
apply() call if it writes storage) to conditionally include the branch property
only when the branching-enabled flag is true (e.g., state.branchingEnabled or
state.hasBranching) — when branching is disabled omit branch from appliedValues
and storedValues and explicitly clear any stored branch value so it is not sent;
update the construction of appliedValues/storedValues in the APPLY_VALUES case
to build the object conditionally based on that flag.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f682a1ca-01b2-404a-a6b6-7977983e5fb1
📒 Files selected for processing (7)
src/constants.tssrc/content/contentScript.tssrc/popup/TolgeeDetector.tsxsrc/popup/storage.tssrc/popup/tools.tssrc/popup/useDetectorForm.tsxsrc/types.ts
JanCizmar
pushed a commit
to tolgee/tolgee-platform
that referenced
this pull request
Mar 17, 2026
) ## Summary - Add `branchingEnabled` boolean to `/v2/api-keys/current` response (`ApiKeyWithLanguagesModel`) so clients can conditionally show branch-related UI - Use `ProjectFeatureGuard.isFeatureEnabled(Feature.BRANCHING)` which checks both org-level feature flag and project-level `useBranching` setting - Rename `BRANCHING_NOT_ENABLED_FOR_PROJECT` to generic `FEATURE_NOT_ENABLED_FOR_PROJECT` with feature name as parameter - Switch `ProjectFeatureGuard` from `ValidationException` to `BadRequestException` for consistency with `OrganizationFeatureGuard` - Add frontend error translation for `feature_not_enabled_for_project` ## Related PRs - chrome-plugin: tolgee/chrome-plugin#38 - tolgee-js: tolgee/tolgee-js#3513 ## Test plan - [ ] Verify `/v2/api-keys/current` response includes `branchingEnabled: true` when branching is enabled - [ ] Verify `branchingEnabled: false` when branching feature is disabled at org level - [ ] Verify `branchingEnabled: false` when project has `useBranching = false` - [ ] Verify `FEATURE_NOT_ENABLED_FOR_PROJECT` error includes the feature name - [ ] Verify frontend displays translated error message for `feature_not_enabled_for_project` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * API key info now includes whether branching is enabled for the project (visible in API responses and client schema). * **Improvements** * Error reporting for disabled features renamed and clarified; messages now indicate which feature is unavailable and client error codes/translations were updated accordingly. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
JanCizmar
requested changes
Mar 17, 2026
Fetch available branches from the platform API after credential validation and display them in an Autocomplete dropdown. Users can select from existing branches or type a custom name. The dropdown only appears when branching is enabled on the project.
dkrizan
force-pushed
the
dkrizan/chrome-plugin-branching
branch
from
15ccebf to
9639f31
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/popup/useDetectorForm.tsx (1)
312-314: Don't stop after the first 100 branches.Line 314 hard-codes
size=100, so larger projects will silently miss options in the new autocomplete. Since the whole point of this control is branch discovery, this should paginate until exhaustion or switch to server-side search.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/popup/useDetectorForm.tsx` around lines 312 - 314, The branch fetch URL currently hard-codes `&size=100` in the string `${url}/v2/projects/${check.projectId}/branches?ak=${checkableValues!.apiKey}&size=100`, which truncates results; change the fetch logic in useDetectorForm to either paginate (preferred) or switch to server-side search: implement a loop that requests pages (use the API's pagination params — e.g. size and page/offset/cursor) starting at page 0, appending returned branches to the results until a page returns fewer than the requested size (or no cursor is returned), and replace the single request that uses `&size=100` with this paginated fetch; ensure the combined result is deduplicated and used by the autocomplete.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/popup/TolgeeDetector.tsx`:
- Around line 143-149: Prettier is flagging the multi-line object literal in the
slotProps.popper.modifiers array; update the popper config inside slotProps (the
object assigned to "popper" within "slotProps" in TolgeeDetector.tsx) to use a
single-line form for the modifiers array (e.g., collapse { name: 'flip',
enabled: false } into the single-line array element) and do the same for the
second occurrence later in the file (the other popper/modifiers block around the
lines referenced) so both instances conform to Prettier's single-line
expectation.
In `@src/popup/useDetectorForm.tsx`:
- Around line 316-330: The fetch chain converts non-2xx responses to null so the
.catch never runs and stale branches persist; update the response handling in
useDetectorForm.tsx (the promise starting with .then((r) => (r.ok ? r.json() :
null))) to explicitly handle non-ok responses by dispatching dispatch({ type:
'SET_BRANCHES', payload: null }) when !r.ok (respecting the cancelled flag) or
by throwing an error to reach the .catch; ensure the subsequent .then only
processes valid JSON (data) and that cancelled is still checked before
dispatching SET_BRANCHES with real branch data.
---
Nitpick comments:
In `@src/popup/useDetectorForm.tsx`:
- Around line 312-314: The branch fetch URL currently hard-codes `&size=100` in
the string
`${url}/v2/projects/${check.projectId}/branches?ak=${checkableValues!.apiKey}&size=100`,
which truncates results; change the fetch logic in useDetectorForm to either
paginate (preferred) or switch to server-side search: implement a loop that
requests pages (use the API's pagination params — e.g. size and
page/offset/cursor) starting at page 0, appending returned branches to the
results until a page returns fewer than the requested size (or no cursor is
returned), and replace the single request that uses `&size=100` with this
paginated fetch; ensure the combined result is deduplicated and used by the
autocomplete.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e7e5935b-6b3e-4882-85c1-26e81130d186
📒 Files selected for processing (3)
.github/workflows/test.ymlsrc/popup/TolgeeDetector.tsxsrc/popup/useDetectorForm.tsx
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/popup/useDetectorForm.tsx (1)
316-330:⚠️ Potential issue | 🟠 MajorClear stale branch options on failed or malformed branch responses.
Line 316 maps non-OK responses to
null, so.catchis skipped and previousbranchescan remain. Also, if the JSON shape is missing_embedded.branches, state is not reset.💡 Proposed fix
fetch( `${url}/v2/projects/${check.projectId}/branches?ak=${ checkableValues!.apiKey }&size=100` ) - .then((r) => (r.ok ? r.json() : null)) + .then((r) => { + if (!r.ok) { + throw new Error('Failed to load branches'); + } + return r.json(); + }) .then((data) => { - if (!cancelled && data?._embedded?.branches) { + if (!cancelled) { dispatch({ type: 'SET_BRANCHES', - payload: data._embedded.branches.map((b: any) => ({ - name: b.name, - isDefault: b.isDefault, - })), + payload: + data?._embedded?.branches?.map((b: any) => ({ + name: b.name, + isDefault: b.isDefault, + })) ?? null, }); } }) .catch(() => { if (!cancelled) { dispatch({ type: 'SET_BRANCHES', payload: null }); } });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/popup/useDetectorForm.tsx` around lines 316 - 330, The current fetch chain in useDetectorForm (the promise that does .then((r) => (r.ok ? r.json() : null)) and subsequent .then((data) => { ... dispatch({ type: 'SET_BRANCHES', ... }) })) can leave stale branches when the response is non-OK or when data lacks _embedded.branches; change the flow so non-OK responses and malformed JSON both result in clearing branches: either throw or return a rejected promise when r.ok is false so the .catch runs, and/or inside the second .then check if data?._embedded?.branches exists and if not dispatch({ type: 'SET_BRANCHES', payload: null }); ensure all branches updates go through the same dispatch code path and keep the cancelled guard around dispatches.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/popup/useDetectorForm.tsx`:
- Around line 316-330: The current fetch chain in useDetectorForm (the promise
that does .then((r) => (r.ok ? r.json() : null)) and subsequent .then((data) =>
{ ... dispatch({ type: 'SET_BRANCHES', ... }) })) can leave stale branches when
the response is non-OK or when data lacks _embedded.branches; change the flow so
non-OK responses and malformed JSON both result in clearing branches: either
throw or return a rejected promise when r.ok is false so the .catch runs, and/or
inside the second .then check if data?._embedded?.branches exists and if not
dispatch({ type: 'SET_BRANCHES', payload: null }); ensure all branches updates
go through the same dispatch code path and keep the cancelled guard around
dispatches.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 523fd096-85e8-4691-b328-64e4f9c9f350
📒 Files selected for processing (2)
src/popup/TolgeeDetector.tsxsrc/popup/useDetectorForm.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/popup/TolgeeDetector.tsx
Fix prettier multiline formatting issue. Also fix a bug where non-2xx branch fetch responses returned null instead of throwing, leaving stale branches from a previous project visible in the dropdown.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/popup/useDetectorForm.tsx (1)
327-330: Consider adding a type for the API response branch.The
anytype could be replaced with an inline type or interface for better type safety.💡 Optional type annotation
- data?._embedded?.branches?.map((b: any) => ({ + data?._embedded?.branches?.map((b: { name: string; isDefault: boolean }) => ({ name: b.name, isDefault: b.isDefault, })) ?? null,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/popup/useDetectorForm.tsx` around lines 327 - 330, The branch mapping uses (b: any) which loses type safety; define an interface or inline type (e.g., Branch with name: string and isDefault: boolean) and use it in the map parameter and/or cast data?._embedded?.branches to Branch[] so the map becomes (b: Branch) => ({ name: b.name, isDefault: b.isDefault }) and preserves null/undefined handling (keep the ?? null). Update any related types in useDetectorForm.tsx to consume the new Branch type.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/popup/useDetectorForm.tsx`:
- Around line 327-330: The branch mapping uses (b: any) which loses type safety;
define an interface or inline type (e.g., Branch with name: string and
isDefault: boolean) and use it in the map parameter and/or cast
data?._embedded?.branches to Branch[] so the map becomes (b: Branch) => ({ name:
b.name, isDefault: b.isDefault }) and preserves null/undefined handling (keep
the ?? null). Update any related types in useDetectorForm.tsx to consume the new
Branch type.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 14172c2e-e619-4f34-a22f-9417e1bc874b
📒 Files selected for processing (1)
src/popup/useDetectorForm.tsx
dkrizan
added a commit
to tolgee/tolgee-js
that referenced
this pull request
Mar 19, 2026
## Summary - Add `branch` to `DevCredentials` type so `overrideCredentials` can pass branch to the SDK - Read `__tolgee_branch` from sessionStorage in `BrowserExtensionPlugin` and include it in credentials - Add `branch` to `LibConfig` type so the SDK sends its configured branch in the `TOLGEE_READY` handshake message - Clear `__tolgee_branch` from sessionStorage on credential reset ## Related PRs - chrome-plugin: tolgee/chrome-plugin#38 - tolgee-platform: tolgee/tolgee-platform#3536 ## Test plan - [x] Verify SDK reads branch from sessionStorage when set by chrome extension - [x] Verify `?branch=X` query parameter is appended to translation API calls - [x] Verify SDK's own branch config is not overridden when sessionStorage key is absent - [x] Verify branch is included in `TOLGEE_READY` handshake message to the extension <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added an optional "branch" configuration to the SDK and browser extension so users can target specific content branches and have that choice persist across sessions. * **Tests** * Expanded test coverage to validate branch handling, propagation, and session persistence. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
JanCizmar
approved these changes
Mar 19, 2026
TolgeeMachine
pushed a commit
that referenced
this pull request
Mar 19, 2026
# [1.9.0](v1.8.2...v1.9.0) (2026-03-19) ### Features * add branching support ([#38](#38)) ([fd394d4](fd394d4))
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary
branchingEnabledfrom/v2/api-keys/currentresponse to conditionally show the branch field__tolgee_branchsessionStorage keyRelated PRs
Test plan
dist-chrome/Summary by CodeRabbit
New Features
Chores