fix(Navigation/AccessLog): proxify request through BFF(nodejs) [YTFRONT-5300]#1438
Merged
ma-efremoff merged 12 commits intomainfrom Feb 17, 2026
Merged
fix(Navigation/AccessLog): proxify request through BFF(nodejs) [YTFRONT-5300]#1438ma-efremoff merged 12 commits intomainfrom
ma-efremoff merged 12 commits intomainfrom
Conversation
Reviewer's GuideThis PR routes Navigation/Access Log and Accounts/Detailed usage API calls through new Node.js proxy controllers that derive per-cluster base URLs from unified UI settings, adds availability checks that drive tab visibility in the UI, hardens error rendering, and introduces shared utilities for cluster-specific configuration merging and snake_case-to-camelCase conversion while updating related docs and dependencies. Sequence diagram for proxied Navigation/Access Log data requestsequenceDiagram
actor User
participant Browser as Browser_UI
participant AccessLogTab as AccessLogTab_fetchAccessLog
participant Axios as Axios_in_Browser
participant Node as Node_Server
participant Ctrl as ytAccessLogApi_controller
participant Config as Config_utils
participant AccessLogSvc as AccessLog_Service
User->>Browser: Open Navigation/Access_Log tab
Browser->>AccessLogTab: dispatch(fetchAccessLog)
AccessLogTab->>AccessLogTab: getAccessLogRequestParams(state)
AccessLogTab->>Axios: GET /api/access-log/:cluster/ready
AccessLogTab->>Axios: POST /api/access-log/:cluster/visible-time-range
AccessLogTab->>Axios: POST /api/access-log/:cluster/access_log
Axios->>Node: HTTP requests /api/access-log/:ytAuthCluster/:action
Node->>Ctrl: route to ytAccessLogApi
Ctrl->>Ctrl: validate action in ALLOWED_ACTIONS
Ctrl->>Config: getAccessLogBaseUrl(req)
Config->>Config: getBaseUrlFromConfiguration(req,{cluster,isDeveloper,uiSettingsFieldName})
Config->>Config: getClusterSpecificUiSettings(req,{cluster,isDeveloper})
Config->>Config: getPreloadedClusterUiConfig(cluster,ctx,isDeveloper)
Config->>Config: mergeUiSettings({uiSettings,uiConfig})
Config->>Config: getBaseUrlDetails(uiSettings,"accessLogBasePath")
Config-->>Ctrl: {baseUrl,testing,use_cors}
Ctrl->>Node: ServerFactory.getAuthHeaders(testing?accessLogTest:accessLogProd,req)
Ctrl->>AccessLogSvc: axios.request(baseUrl + "/" + action)
AccessLogSvc-->>Ctrl: streaming HTTP response
Ctrl->>Node: pipeAxiosResponse(ctx,res,response)
Node-->>Axios: proxied response stream
Axios-->>AccessLogTab: data for ready/visible-time-range/access_log
AccessLogTab-->>Browser: update Redux state and UI
Updated class diagram for UI settings, cluster config, and base URL utilitiesclassDiagram
class UISettings {
<<interface>>
+UiConfigBaseUrl accessLogBasePath
+UiConfigBaseUrl accountsUsageBasePath
+string docsBaseUrl
+string jupyterBasePath
}
class UiConfigBaseUrl {
<<type>>
+boolean @testing
+boolean @use_cors
+string $value
}
class ClusterUiConfig {
<<interface>>
+boolean enable_per_bundle_tablet_accounting
+boolean enable_per_account_tablet_accounting
+boolean enable_maintenance_api_nodes
+boolean enable_maintenance_api_proxies
+string chyt_controller_base_url
+string livy_controller_base_url
+Record~Stage,string~ query_tracker_default_aco
+any job_trace_url_template
+string operation_performance_url_template
+string tablet_errors_base_url
+string job_log_viewer_base_url
+string job_log_viewer_tvm_key
+Partial~UISettings~ ui_settings
}
class BaseUrlDetails {
+string baseUrl
+boolean testing
+boolean use_cors
}
class mergeUiSettings {
<<function>>
+UISettings mergeUiSettings(uiSettings, uiConfig)
}
class getBaseUrlDetails {
<<function>>
+BaseUrlDetails getBaseUrlDetails(uiSettings,key)
}
class getBaseUrlFromConfiguration {
<<function>>
+BaseUrlDetails getBaseUrlFromConfiguration(req,cluster,isDeveloper,uiSettingsFieldName)
}
class getClusterSpecificUiSettings {
<<function>>
+getClusterSpecificUiSettings(req,cluster,isDeveloper) UISettings
}
class snakeToCamel {
<<function>>
+snakeToCamel(str) string
}
class snakeToCamelObject {
<<function>>
+snakeToCamelObject(obj) object
}
class outputUiSettingsToCamelCase {
<<function>>
+outputUiSettingsToCamelCase(obj) obj_with_camelCase_ui_settings
}
class getMergedUiSettings {
<<selector>>
+getMergedUiSettings(state) UISettings
}
class getBaseUrlFromConfigurationDeps {
<<utility>>
+getPreloadedClusterUiConfig(cluster,ctx,isDeveloper)
}
UISettings <|.. UiConfigBaseUrl : uses
ClusterUiConfig --> UISettings : ui_settings
mergeUiSettings --> UISettings : returns
mergeUiSettings --> ClusterUiConfig : reads ui_settings
getBaseUrlDetails --> UISettings : reads base URL fields
getBaseUrlFromConfiguration --> getClusterSpecificUiSettings : calls
getBaseUrlFromConfiguration --> getBaseUrlDetails : calls
getClusterSpecificUiSettings --> mergeUiSettings : calls
getClusterSpecificUiSettings --> getBaseUrlFromConfigurationDeps : calls
snakeToCamelObject --> snakeToCamel : calls
outputUiSettingsToCamelCase --> snakeToCamelObject : transforms ui_settings
getMergedUiSettings --> mergeUiSettings : calls
getMergedUiSettings --> ClusterUiConfig : reads clusterUiConfig
getMergedUiSettings --> UISettings : merges global uiSettings
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
|
Storybook is ready. |
Contributor
|
Playwright components report is ready. |
Contributor
|
Statoscope report is ready. |
Contributor
|
E2E-local report is ready. |
ma-efremoff
force-pushed
the
ma-efremoff/accesslog
branch
6 times, most recently
from
428bff6 to
6dc6ff6
Compare
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 17 issues, and left some high level feedback:
- In the README changes, there are several typos and the options table is now split by blank lines, which likely breaks Markdown rendering; consider fixing spelling (e.g. 'optaions', 'optnos') and consolidating the rows back into a single properly formatted table, as well as correcting
uiSettings/acccess_log_base_pathto the right setting name. - In
NavigationStatetheis_access_log_availablefield is typed as the literalfalseinstead ofboolean, which will make it awkward or impossible to assigntruelater; update the type tobooleanto reflect actual usage. - Both
UpdateAccessLogAvailabilityandUpdateAccountsUsageAvailabilityuseclusteranduiConfiginsideuseEffectbut omitclusterfrom the dependency array (and only partially include the objects), which can lead to stale values when the cluster changes; consider includingclusterand using more stable dependencies (e.g. specific fields or serialized config) or moving the cookie update logic to a single central place (e.g. theApp-level helper you introduced).
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In the README changes, there are several typos and the options table is now split by blank lines, which likely breaks Markdown rendering; consider fixing spelling (e.g. 'optaions', 'optnos') and consolidating the rows back into a single properly formatted table, as well as correcting `uiSettings/acccess_log_base_path` to the right setting name.
- In `NavigationState` the `is_access_log_available` field is typed as the literal `false` instead of `boolean`, which will make it awkward or impossible to assign `true` later; update the type to `boolean` to reflect actual usage.
- Both `UpdateAccessLogAvailability` and `UpdateAccountsUsageAvailability` use `cluster` and `uiConfig` inside `useEffect` but omit `cluster` from the dependency array (and only partially include the objects), which can lead to stale values when the cluster changes; consider including `cluster` and using more stable dependencies (e.g. specific fields or serialized config) or moving the cookie update logic to a single central place (e.g. the `App`-level helper you introduced).
## Individual Comments
### Comment 1
<location> `packages/ui/src/ui/pages/accounts/tabs/detailed-usage/UpdateAccountsUsageAvaiability/UpdateAccountsUsageAvailability.tsx:18-27` </location>
<code_context>
+function UpdateUiConfigModeCookie() {
+ const isAdmin = useSelector(isDeveloper);
+ React.useEffect(() => {
+ updateUiConfigModeCookie(isAdmin);
+ }, [isAdmin]);
</code_context>
<issue_to_address>
**issue (bug_risk):** Cluster change will not trigger availability re-check because `cluster` is missing from deps.
This effect calls `/api/accounts-usage/${cluster}/check-available`, but `cluster` is not in the dependency array, so changing cluster without a full reload will keep using the old cluster when checking availability and updating state. Add `cluster` to the dependencies so the availability flag updates when the cluster changes.
</issue_to_address>
### Comment 2
<location> `packages/ui/src/ui/pages/navigation/tabs/AccessLog/UpdateAccessLogAvailability/UpdateAccessLogAvailability.tsx:18-27` </location>
<code_context>
+function UpdateUiConfigModeCookie() {
+ const isAdmin = useSelector(isDeveloper);
+ React.useEffect(() => {
+ updateUiConfigModeCookie(isAdmin);
+ }, [isAdmin]);
</code_context>
<issue_to_address>
**issue (bug_risk):** Missing `cluster` in the effect dependencies can desynchronize access-log availability on cluster change.
This effect derives values from `cluster`, but the dependency array only includes `isAdmin`, `uiConfig`, and `dispatch`. If the cluster changes without a reload, the effect won’t re-run and `is_access_log_available` can stay tied to the previous cluster. Include `cluster` in the dependency array to keep it in sync.
</issue_to_address>
### Comment 3
<location> `packages/ui/src/ui/store/reducers/navigation/navigation.tsx:42-44` </location>
<code_context>
sidePanelMode: 'qt' | 'yqlkit' | undefined;
originatingQueuePath: string | undefined;
+
+ is_access_log_available: false;
};
</code_context>
<issue_to_address>
**issue:** `is_access_log_available` is typed as `false` instead of `boolean` in `NavigationState`.
This declaration makes `is_access_log_available` a `false` literal type rather than `boolean`, while other code treats it as a boolean. Please change the type to `boolean` and keep the initial value `false` in `ephemeralState` to avoid type mismatch and unintended narrowing.
</issue_to_address>
### Comment 4
<location> `packages/ui/src/ui/store/selectors/navigation/navigation.ts:35-36` </location>
<code_context>
export const getNavigationCheckPermissionsError = (state: RootState) =>
state.navigation.navigation.checkPermissionsError as YTError | undefined;
export const getMode = (state: RootState) => state.navigation.navigation.mode;
+export const getNaviagationAccessLogAvailable = (state: RootState) =>
+ state.navigation.navigation.is_access_log_available;
export const getNavigationPathAccount = createSelector(
</code_context>
<issue_to_address>
**nitpick (typo):** Selector name contains a typo (`getNaviagationAccessLogAvailable`).
Please rename this selector to `getNavigationAccessLogAvailable` to fix the spelling and avoid spreading the typo to call sites.
Suggested implementation:
```typescript
export const getMode = (state: RootState) => state.navigation.navigation.mode;
export const getNavigationAccessLogAvailable = (state: RootState) =>
state.navigation.navigation.is_access_log_available;
```
```typescript
export const getNavigationPathAccount = createSelector(
[getNavigationPathAttributes],
getTableMountConfigHasData,
getTabletErrorsBackgroundCount,
getNavigationOriginatingQueuePath,
getNavigationAccessLogAvailable,
],
```
</issue_to_address>
### Comment 5
<location> `packages/ui/src/ui/pages/accounts/tabs/detailed-usage/UpdateAccountsUsageAvaiability/i18n/index.ts:6` </location>
<code_context>
+import en from './en.json';
+import ru from './ru.json';
+
+export default addI18Keysets('yt:update-accounts-usage-availablility', {en, ru});
</code_context>
<issue_to_address>
**nitpick (typo):** Multiple spellings of "availability" are inconsistent/misspelled in identifiers and paths.
The directory name (`UpdateAccountsUsageAvaiability`) and i18n keys (`yt:update-accounts-usage-availablility`, `yt:update-access-log-availablility`) all misspell “availability”. Please standardize these to the correct spelling (e.g., `UpdateAccountsUsageAvailability`, `...-availability`) to keep identifiers and paths consistent and searchable.
Suggested implementation:
```typescript
export default addI18Keysets('yt:update-accounts-usage-availability', {en, ru});
```
To fully apply your naming suggestion across the codebase, the following should also be done:
1. Rename the directory:
- From: `packages/ui/src/ui/pages/accounts/tabs/detailed-usage/UpdateAccountsUsageAvaiability/`
- To: `packages/ui/src/ui/pages/accounts/tabs/detailed-usage/UpdateAccountsUsageAvailability/`
2. Update all imports that reference `UpdateAccountsUsageAvaiability` to `UpdateAccountsUsageAvailability`.
3. Standardize any other i18n keys with the same typo, e.g.:
- `yt:update-access-log-availablility` → `yt:update-access-log-availability`
and update all usages of these keys in the codebase and translation JSON files (`en.json`, `ru.json` and any other locales).
</issue_to_address>
### Comment 6
<location> `packages/ui/README.md:102` </location>
<code_context>
-There is yt-api command get_supported_feature and it is a good place to describe some features.
-But some cases require ability to turn on/off a feature manually on a cluster. Such feature flags are placed placed in:
+There is yt-api command get_supported_feature and it is a good place to describe API features.
+But some cases require ability to turn on/off a feature manually on a specific cluster. Such cluster specific optinos are placed placed in:
- `//sys/@ui_config` (values affects all users)
</code_context>
<issue_to_address>
**suggestion (typo):** Fix typos and wording in the cluster-specific options sentence.
Suggest rephrasing to: “But some cases require the ability to turn on/off a feature manually on a specific cluster. Such cluster-specific options are placed in:” (fixing the typos, removing the duplicated “placed”, and adding “the” before “ability”).
```suggestion
But some cases require the ability to turn on/off a feature manually on a specific cluster. Such cluster-specific options are placed in:
```
</issue_to_address>
### Comment 7
<location> `packages/ui/README.md:104-105` </location>
<code_context>
+There is yt-api command get_supported_feature and it is a good place to describe API features.
+But some cases require ability to turn on/off a feature manually on a specific cluster. Such cluster specific optinos are placed placed in:
- `//sys/@ui_config` (values affects all users)
-- `//sys/@ui_config_dev_overrides` (values affects only developers)
+- `//sys/@ui_config_dev_overrides` (values affects only admins)
(see more detail in [YTFRONT-2804](https://nda.ya.ru/t/bgh9NWJ16fPRp4))
</code_context>
<issue_to_address>
**suggestion (typo):** Use correct subject–verb agreement in the bullet descriptions.
Because "values" is plural, both bullets should use "values affect", e.g. "(values affect all users)" and "(values affect only admins)".
Suggested implementation:
```
- `//sys/@ui_config` (values affect all users)
```
```
- `//sys/@ui_config_dev_overrides` (values affect only admins)
```
</issue_to_address>
### Comment 8
<location> `packages/ui/README.md:111` </location>
<code_context>
-| tablet_errors_base_url | **null**, url as string | Base URL for tablet errors service to override uiSettings.tabletErrorsBaseUrl |
+UI determines a user as an admin on the cluster if he has `write` access to `admins` group.
+
+Available optaions (**default values** are highlighted in bold):
+
+| Option name | Allowed values | Description |
</code_context>
<issue_to_address>
**issue (typo):** Fix typo in "optaions".
In the “Available options…” line, replace “optaions” with “options”.
```suggestion
Available options (**default values** are highlighted in bold):
```
</issue_to_address>
### Comment 9
<location> `packages/ui/README.md:121` </location>
<code_context>
+
+| per_bundle_accounting_help_link | **null**, url as string | Help link for resources of tablets to display from AccountEditorDialog about moving the resources to bundles [YTFRONT-2851](https://nda.ya.ru/t/xnLq-3Dm6fPYPo) |
+
+| enable_maintenance_api_nodes | **null**, boolean | Allows to use `add_maintenance`/`remove_maintenance` commands from `Comopnents/Nodes` page [YTFRONT-3792](https://nda.ya.ru/t/RvueJLzN6fWx3h) |
+
+| enable_maintenance_api_proxies | **null**, boolean | Allows to use `add_maintenance`/`remove_maintenance` commands from `Components/HTTP Proxies` and `Components/RPC Proxies` pages [YTFRONT-3792](https://nda.ya.ru/t/RvueJLzN6fWx3h) |
</code_context>
<issue_to_address>
**issue (typo):** Fix the spelling of "Comopnents".
Please update this entry to say "Components/Nodes" instead of "Comopnents/Nodes".
```suggestion
| enable_maintenance_api_nodes | **null**, boolean | Allows to use `add_maintenance`/`remove_maintenance` commands from `Components/Nodes` page [YTFRONT-3792](https://nda.ya.ru/t/RvueJLzN6fWx3h) |
```
</issue_to_address>
### Comment 10
<location> `packages/ui/README.md:128` </location>
<code_context>
+| chyt_controller_base_url | **null**, url as string | Base url for chyt-controller |
+
+| livy_controller_base_url | **null**, url as string | Base url for spyt-controller |
+| job_trace_url_template | **null**, `{title: string; url_template: string; enforce_for_trees?: Array<string>}` | If defined adds `Job trace` item to meta-table on `Job/Details` page for a job with `archive_features/has_trace == true` and for jobs from a tree in `enforce_for_trees`, example: `{title: 'Open im MyProfiling', url_template: 'https://my.profiling.service/{cluster}/{operationId}/{jobId}', enforce_for_trees: ['tree-with-traces'] }` |
+
+| query_tracker_default_aco | **null**, `{stage1: string; stage2: string; }` | Sets the default ACO in Query Tracker requests for each stage |
</code_context>
<issue_to_address>
**issue (typo):** Fix the typo "Open im MyProfiling" in the example string.
In the example configuration object for `job_trace_url_template`, update the `title` value to "Open in MyProfiling".
```suggestion
| job_trace_url_template | **null**, `{title: string; url_template: string; enforce_for_trees?: Array<string>}` | If defined adds `Job trace` item to meta-table on `Job/Details` page for a job with `archive_features/has_trace == true` and for jobs from a tree in `enforce_for_trees`, example: `{title: 'Open in MyProfiling', url_template: 'https://my.profiling.service/{cluster}/{operationId}/{jobId}', enforce_for_trees: ['tree-with-traces'] }` |
```
</issue_to_address>
### Comment 11
<location> `packages/ui/README.md:138` </location>
<code_context>
+
+| tablet_errors_base_url | **null**, url as string | Base URL for tablet errors service to override `uiSettings.tabletErrorsBaseUrl` |
+
+| access_log_base_url | **null**, url as string | Base URL for `Navigation/Access Log` API endpoint, the option overrides `uiSettings/acccess_log_base_path` |
### Configuration
</code_context>
<issue_to_address>
**question (typo):** Double-check the spelling of the `uiSettings/acccess_log_base_path` key.
If the actual setting key is `access_log_base_path`, please update this reference to match and avoid configuration confusion.
```suggestion
| access_log_base_url | **null**, url as string | Base URL for `Navigation/Access Log` API endpoint, the option overrides `uiSettings/access_log_base_path` |
```
</issue_to_address>
### Comment 12
<location> `packages/ui/src/server/controllers/yt-access-log-api.ts:63` </location>
<code_context>
+ const {baseUrl, testing} = await getAccountsUsageBaseUrl(req);
+
+ if (!baseUrl) {
+ throw new ErrorWithCode(
+ 404,
+ 'The UI installation is not configured to display "Accounts/Detailed usage" tab for current cluster. Please check your configuration: config.uiSettings.accountsUsageBasePath, //sys/@ui_config/resource_usage_base_url',
</code_context>
<issue_to_address>
**suggestion (review_instructions):** The error message string is slightly ungrammatical; consider using “for the current cluster” instead of “on current cluster.”
The message
"The UI installation is not configured to display \"Navigation/Access log\" tab on current cluster."
would read more naturally as
"The UI installation is not configured to display \"Navigation/Access log\" tab for the current cluster."
<details>
<summary>Review instructions:</summary>
**Path patterns:** `packages/ui/**/*.js,packages/ui/**/*.ts,packages/ui/**/*.tsx,packages/ui/**/*.jsx`
**Instructions:**
Do not pass grammatical issues
</details>
</issue_to_address>
### Comment 13
<location> `packages/ui/src/ui/pages/accounts/Accounts/Accounts.tsx:42` </location>
<code_context>
import './Accounts.scss';
import UIFactory from '../../../UIFactory';
import {UI_TAB_SIZE} from '../../../constants/global';
+import {UpdateAccountsUsageAvailability} from '../tabs/detailed-usage/UpdateAccountsUsageAvaiability/UpdateAccountsUsageAvailability';
const b = block('accounts');
</code_context>
<issue_to_address>
**suggestion (review_instructions):** There is a spelling mistake (“Avaiability”) in the imported path segment, which should be “Availability.”
The folder segment `UpdateAccountsUsageAvaiability` is misspelled; it should be `UpdateAccountsUsageAvailability`. Even though this isn’t user-facing text, the instruction asks us not to pass grammatical/spelling issues. Renaming the directory and updating imports accordingly would improve clarity.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `packages/ui/**/*.js,packages/ui/**/*.ts,packages/ui/**/*.tsx,packages/ui/**/*.jsx`
**Instructions:**
Do not pass grammatical issues
</details>
</issue_to_address>
### Comment 14
<location> `packages/ui/src/ui/store/selectors/navigation/navigation.ts:35` </location>
<code_context>
export const getNavigationCheckPermissionsError = (state: RootState) =>
state.navigation.navigation.checkPermissionsError as YTError | undefined;
export const getMode = (state: RootState) => state.navigation.navigation.mode;
+export const getNaviagationAccessLogAvailable = (state: RootState) =>
+ state.navigation.navigation.is_access_log_available;
</code_context>
<issue_to_address>
**suggestion (review_instructions):** The selector name contains a spelling mistake (“Naviagation” instead of “Navigation”).
The function name `getNaviagationAccessLogAvailable` has a typo; it should be `getNavigationAccessLogAvailable`. Renaming the selector and updating its usages will keep naming consistent and avoid confusion.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `packages/ui/**/*.js,packages/ui/**/*.ts,packages/ui/**/*.tsx,packages/ui/**/*.jsx`
**Instructions:**
Do not pass grammatical issues
</details>
</issue_to_address>
### Comment 15
<location> `packages/ui/src/server/controllers/yt-access-log-api.ts:26` </location>
<code_context>
+ }
+}
+
+export async function ytAccesLogCheckAvailable(req: Request, res: Response) {
+ try {
+ const {baseUrl} = await getAccessLogBaseUrl(req);
</code_context>
<issue_to_address>
**suggestion (review_instructions):** The function name is missing an “s” in “Access” (`ytAccesLogCheckAvailable`).
`ytAccesLogCheckAvailable` is missing an “s” in “Access”. Renaming it to `ytAccessLogCheckAvailable` (and updating imports/usages) would fix the spelling issue and improve readability.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `packages/ui/**/*.js,packages/ui/**/*.ts,packages/ui/**/*.tsx,packages/ui/**/*.jsx`
**Instructions:**
Do not pass grammatical issues
</details>
</issue_to_address>
### Comment 16
<location> `packages/ui/src/ui/pages/accounts/tabs/detailed-usage/UpdateAccountsUsageAvaiability/i18n/index.ts:6` </location>
<code_context>
+import en from './en.json';
+import ru from './ru.json';
+
+export default addI18Keysets('yt:update-accounts-usage-availablility', {en, ru});
--- /dev/null
+++ b/packages/ui/src/ui/pages/navigation/tabs/AccessLog/UpdateAccessLogAvailability/i18n/index.ts
</code_context>
<issue_to_address>
**suggestion (review_instructions):** The i18n key contains a spelling mistake (“availablility” instead of “availability”).
The namespace string `'yt:update-accounts-usage-availablility'` has “availablility” instead of “availability”. While it’s an internal key, it’s still a spelling issue per the review instructions; consider correcting it (and corresponding usages) to avoid confusion.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `packages/ui/**/*.js,packages/ui/**/*.ts,packages/ui/**/*.tsx,packages/ui/**/*.jsx`
**Instructions:**
Do not pass grammatical issues
</details>
</issue_to_address>
### Comment 17
<location> `packages/ui/src/ui/pages/navigation/tabs/AccessLog/UpdateAccessLogAvailability/i18n/index.ts:6` </location>
<code_context>
+import en from './en.json';
+import ru from './ru.json';
+
+export default addI18Keysets('yt:update-access-log-availablility', {en, ru});
--- /dev/null
+++ b/packages/ui/src/ui/pages/accounts/tabs/detailed-usage/UpdateAccountsUsageAvaiability/i18n/en.json
</code_context>
<issue_to_address>
**suggestion (review_instructions):** The i18n key contains a spelling mistake (“availablility” instead of “availability”).
The key `'yt:update-access-log-availablility'` misspells “availability” as “availablility”. Even though it’s not user-facing, this is still a spelling issue; consider renaming it and updating references to keep things consistent.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `packages/ui/**/*.js,packages/ui/**/*.ts,packages/ui/**/*.tsx,packages/ui/**/*.jsx`
**Instructions:**
Do not pass grammatical issues
</details>
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
…BFF [YTFRONT-5366]
ma-efremoff
force-pushed
the
ma-efremoff/accesslog
branch
from
6dc6ff6 to
ab6a720
Compare
ma-efremoff
force-pushed
the
ma-efremoff/accesslog
branch
from
9e74e9c to
ab048dd
Compare
ma-efremoff
force-pushed
the
ma-efremoff/accesslog
branch
from
9fa585c to
986d840
Compare
There was a problem hiding this comment.
Hey - I've found 4 issues, and left some high level feedback:
- The new
calcBaseUrl/calcAccountsUsageBaseUrlhelpers rely onurl.split('/').pop()which will misbehave for URLs with trailing slashes or query strings; consider passing the action segment explicitly or using a more robust URL builder. - The
updateUiConfigModeCookiecall is now triggered fromApp,UpdateAccessLogAvailability,UpdateAccountsUsageAvailability, andinitClusterParams; this looks redundant and could be centralized to a single place to avoid extra cookie writes and make the logic easier to follow. - The updated "Cluster specific features" table in
README.mdis split into multiple small tables by blank lines between|rows, which likely breaks GitHub markdown rendering; it would be better to keep it as a single table and fix typos likeoptinos.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `calcBaseUrl`/`calcAccountsUsageBaseUrl` helpers rely on `url.split('/').pop()` which will misbehave for URLs with trailing slashes or query strings; consider passing the action segment explicitly or using a more robust URL builder.
- The `updateUiConfigModeCookie` call is now triggered from `App`, `UpdateAccessLogAvailability`, `UpdateAccountsUsageAvailability`, and `initClusterParams`; this looks redundant and could be centralized to a single place to avoid extra cookie writes and make the logic easier to follow.
- The updated "Cluster specific features" table in `README.md` is split into multiple small tables by blank lines between `|` rows, which likely breaks GitHub markdown rendering; it would be better to keep it as a single table and fix typos like `optinos`.
## Individual Comments
### Comment 1
<location> `packages/ui/src/ui/pages/accounts/tabs/detailed-usage/UpdateAccountsUsageAvailability/UpdateAccountsUsageAvailability.tsx:16-17` </location>
<code_context>
+function UpdateUiConfigModeCookie() {
+ const isAdmin = useSelector(isDeveloper);
+ React.useEffect(() => {
+ updateUiConfigModeCookie(isAdmin);
+ }, [isAdmin]);
+
</code_context>
<issue_to_address>
**suggestion:** Avoid updating the `ui_config_mode` cookie from multiple places to reduce duplication and potential drift.
`ui_config_mode` is now written in multiple places: here, in `App` via `UpdateUiConfigModeCookie`, and in `initClusterParams`. Multiple writers make it hard to reason about when the cookie is set or cleared and increase the risk of inconsistencies. Centralize this side effect (e.g., in a single top‑level effect keyed on `isDeveloper`) and remove the redundant updates in lower‑level components.
Suggested implementation:
```typescript
```
```typescript
const dispatch = useDispatch();
const cluster = useSelector(getCluster);
```
```typescript
React.useEffect(() => {
wrapApiPromiseByToaster(
axios
.get<{
```
```typescript
}, []);
```
1. Ensure that `ui_config_mode` is updated in a single, centralized place (e.g. a top-level `UpdateUiConfigModeCookie` component or effect in `App` keyed on `isDeveloper`), and remove any remaining direct writes to `ui_config_mode` in `initClusterParams` or other lower-level modules.
2. You may want to refine the `useEffect` dependency array (`[]` above) to whatever actually drives the usage-availability fetch (for example, `cluster` or `cluster.id`) to better reflect when the data should be refreshed.
</issue_to_address>
### Comment 2
<location> `packages/ui/README.md:99` </location>
<code_context>
- Scheduling pool: `scheduler-pool`
-### Feature flags
+### Cluster specific features
-There is yt-api command get_supported_feature and it is a good place to describe some features.
</code_context>
<issue_to_address>
**suggestion (typo):** Consider hyphenating “Cluster-specific” in the heading.
As a compound adjective before “features”, this should be written as “Cluster-specific features” for grammatical correctness.
```suggestion
### Cluster-specific features
```
</issue_to_address>
### Comment 3
<location> `packages/ui/README.md:104-105` </location>
<code_context>
- `//sys/@ui_config` (values affects all users)
-- `//sys/@ui_config_dev_overrides` (values affects only developers)
+- `//sys/@ui_config_dev_overrides` (values affects only admins)
(see more detail in [YTFRONT-2804](https://nda.ya.ru/t/bgh9NWJ16fPRp4))
</code_context>
<issue_to_address>
**issue (typo):** Correct subject–verb agreement in “values affects only admins”.
Use “values affect only admins” to match the plural subject “values.”
```suggestion
- `//sys/@ui_config` (values affect all users)
- `//sys/@ui_config_dev_overrides` (values affect only admins)
```
</issue_to_address>
### Comment 4
<location> `packages/ui/src/shared/ui-settings.ts:7` </location>
<code_context>
+export type UiConfigBaseUrl = CypressNodeRaw<{testing: boolean; use_cors: boolean}, string>;
+
+/**
+ * It is possible to provide cluster specific overrides through `//sys/@ui_config/ui_settings`.
+ * All `snake_case`-fieldnames of `//sys/@ui_config/ui_settings` will be converted to `camelCase`.
+ */
</code_context>
<issue_to_address>
**issue (review_instructions):** The doc comment has minor grammar issues: 'cluster specific' should be hyphenated and 'fieldnames' should be written as 'field names'.
To fix the grammar, consider rephrasing the comment to something like:
`* It is possible to provide cluster-specific overrides through \\sys/@ui_config/ui_settings.`
`* All \"`snake_case\"` field names in \\sys/@ui_config/ui_settings will be converted to \"camelCase\".`
<details>
<summary>Review instructions:</summary>
**Path patterns:** `packages/ui/**/*.js,packages/ui/**/*.ts,packages/ui/**/*.tsx,packages/ui/**/*.jsx`
**Instructions:**
- Do not pass grammatical issues
</details>
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
SimbiozizV
reviewed
Feb 17, 2026
SimbiozizV
previously approved these changes
Feb 17, 2026
…nfig/ui_settings' for cluster specific overrides [YTFRONT-5300,YTFRONT-5366]
ma-efremoff
force-pushed
the
ma-efremoff/accesslog
branch
from
986d840 to
f8376f2
Compare
SimbiozizV
approved these changes
Feb 17, 2026
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.
https://nda.ya.ru/t/akvyyZcv7ThFSR
## Summary by SourceryProxy navigation access log and accounts usage requests through the Node.js backend using configurable per-cluster UI settings and expose their availability in the UI.
New Features:
Bug Fixes:
Enhancements: