fix(core): 18520 auth fixes and refactors #819
Conversation
WalkthroughThe recent changes enhance the application's structure and functionality, focusing on improved linting practices, updated dependencies, and clearer code. Key updates include adjusting ESLint rules for TypeScript and React, upgrading various library versions, and refining the API client for better token management. These modifications collectively aim to bolster maintainability, performance, and the overall developer experience. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ApiClient
participant AuthService
User->>ApiClient: Make API Request
ApiClient->>AuthService: Get Access Token
AuthService-->>ApiClient: Return Token
ApiClient->>User: Provide Response
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
|
Bundle size diff
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Outside diff range comments (1)
src/core/apiClientInstance.ts (1)
[!TIP]
Codebase VerificationInconsistent Error Handling Across Clients
The error handling in
reportsClientuses a simplified approach by defaulting to a generic error notification. However, the search results indicate that error handling strategies vary across the codebase. Some instances log errors to the console, while others handle errors within asynchronous resource management or use specific error messages. This inconsistency may lead to unpredictable user experiences or overlooked error cases.
File:
src/widgets/FocusedGeometryEditor/index.ts
- Uses
console.errorfor logging errors.File:
src/features/advanced_analytics_panel/atoms/advancedAnalyticsWorldResource.ts
- Handles errors within an async resource context.
File:
src/core/apiClientInstance.ts
- Uses a switch statement to handle errors, similar to
reportsClient.Consider reviewing the error handling strategies across different clients to ensure consistency and adherence to the application's error handling policies.
Analysis chain
Line range hint
18-30:
Ensure consistent error handling across clients.The error handling strategy is now simplified. Verify that this approach is consistent with the application's error handling policies and doesn't overlook specific error cases.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify error handling consistency across clients. # Test: Search for error handling strategies in the codebase. Expect: Consistency with the new approach. rg --type typescript -A 5 $'error:'Length of output: 69
Script:
#!/bin/bash # Description: Verify error handling consistency across clients. # Test: Search for error handling strategies in TypeScript files. Expect: Consistency with the new approach. rg --glob '*.ts' -A 5 $'error:'Length of output: 29895
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
Files selected for processing (43)
- package.json (8 hunks)
- src/boot.ts (5 hunks)
- src/components/ConnectedMap/map-libre-adapter/index.tsx (4 hunks)
- src/core/api/features.ts (1 hunks)
- src/core/api/reports.ts (1 hunks)
- src/core/apiClientInstance.ts (2 hunks)
- src/core/api_client/apiClient.ts (8 hunks)
- src/core/api_client/tests/_clientTestsContext.ts (2 hunks)
- src/core/api_client/tests/authorization.test.ts (2 hunks)
- src/core/api_client/types.ts (1 hunks)
- src/core/api_client/utils.ts (1 hunks)
- src/core/app/authHooks.ts (1 hunks)
- src/core/app/postAppInit.ts (1 hunks)
- src/core/app/types.ts (1 hunks)
- src/core/app/userWasLanded.ts (1 hunks)
- src/core/auth/OidcSimpleClient.ts (1 hunks)
- src/core/auth/index.ts (1 hunks)
- src/core/authClientInstance.ts (1 hunks)
- src/core/config/index.ts (3 hunks)
- src/core/config/loaders/stageConfigLoader.ts (1 hunks)
- src/core/config/types.ts (1 hunks)
- src/core/metrics/app-metrics.ts (1 hunks)
- src/core/metrics/constants.ts (1 hunks)
- src/core/metrics/types.ts (1 hunks)
- src/core/router/components/Router.tsx (4 hunks)
- src/core/router/routes.tsx (1 hunks)
- src/core/router/types.ts (1 hunks)
- src/core/shared_state/featureFlags.ts (1 hunks)
- src/features/analytics_panel/atoms/analyticsResource.ts (1 hunks)
- src/features/current_event/atoms/currentEventGeometry.ts (1 hunks)
- src/features/events_list/atoms/eventListResource.ts (1 hunks)
- src/features/events_list/components/EventsPanel/EventsPanel.tsx (1 hunks)
- src/features/llm_analytics/atoms/llmAnalyticsResource.ts (1 hunks)
- src/utils/map/mapCSSToMapBoxPropertiesConverter/getRequirements.ts (1 hunks)
- src/views/About/About.tsx (1 hunks)
- src/views/CommonView.tsx (2 hunks)
- src/views/Map/Layouts/Desktop/Desktop.tsx (2 hunks)
- src/views/Map/Layouts/Laptop/Laptop.tsx (1 hunks)
- src/views/Map/Layouts/Layout.tsx (2 hunks)
- src/views/Map/Layouts/Mobile/Mobile.tsx (1 hunks)
- src/views/Map/Map.tsx (6 hunks)
- src/views/Map/SmartColumn/SmartColumn.tsx (2 hunks)
- src/vite-env.d.ts (1 hunks)
Additional comments not posted (86)
src/core/authClientInstance.ts (1)
1-2: VerifyOidcSimpleClientfunctionality.Ensure that
OidcSimpleClientprovides all the necessary methods and properties that were previously used fromAuthClient.src/core/app/postAppInit.ts (1)
7-9: Verify implications of removing authentication check.The removal of the authentication check could have implications on how the application handles user sessions. Ensure that this change does not lead to unexpected behavior.
src/views/About/About.tsx (1)
16-16: Address TypeScript type-checking suppression.The
// @ts-expect-errordirective is used to suppress type-checking errors. This should be a temporary measure, and the underlying type issues should be resolved to maintain type safety. Consider refactoring the code to align with TypeScript's type-checking rules.src/core/api_client/utils.ts (1)
3-21: LGTM! Effective response handling.The
autoParseBodyfunction correctly handles different response types and status codes. The use ofconsole.debugfor logging is appropriate for development and debugging.src/vite-env.d.ts (1)
3-19: Enhancement of type safety with environment variables.The introduction of the
ImportMetaEnvinterface and the modification of theImportMetainterface to include theenvproperty improve type safety and clarity when accessing environment variables. This structured approach enhances maintainability and readability.src/core/metrics/types.ts (1)
1-1: Refactor import path forAppFeatureType.Changing the import path for
AppFeatureTypeto~core/app/typesaligns it with application-level metrics, indicating a more appropriate categorization. This enhances the logical organization of the codebase.src/utils/map/mapCSSToMapBoxPropertiesConverter/getRequirements.ts (1)
1-2: Modularization oflocalStorageaccess.The change to initialize
DEBUG_MAPCSSusinglocalStoragefrom a utility module enhances modularity and testability. This abstraction allows for easier mocking or replacement in different environments, improving maintainability.src/core/app/userWasLanded.ts (1)
7-7: VerifyconfigRepoinitialization and usage.Ensure that
configRepois correctly initialized and accessible throughout the application. This change relies onconfigRepofor configuration management, which is crucial for the function's reliability.Verification successful
configRepois correctly initialized and used throughout the application.
configRepois initialized withexport const configRepo = new ConfigRepository();, confirming its proper setup.- It is consistently imported and used across various files, indicating its accessibility and integration for configuration management.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the initialization and usage of `configRepo`. # Test: Search for the initialization of `configRepo`. Expect: Proper initialization and usage. rg --type typescript -A 5 $'configRepo'Length of output: 73
Script:
#!/bin/bash # Description: Verify the initialization and usage of `configRepo`. # Find all TypeScript files and search for `configRepo` to check its initialization and usage. fd --extension ts --exec rg 'configRepo'Length of output: 4628
src/core/apiClientInstance.ts (1)
Line range hint
4-16:
Verify the impact of removingwithSetupCheck.The removal of
withSetupCheckmay affect setup checks or middleware functionality. Ensure that this change doesn't introduce regressions in error handling or application behavior.src/core/app/authHooks.ts (1)
6-11: Verify new state management approach for user and feature flags.The removal of dispatches to
currentUserAtomandfeatureFlagsAtomindicates a shift in state management. Ensure that user state and feature flags are correctly managed without these dispatches.src/core/shared_state/featureFlags.ts (1)
9-9: Simplified state initialization is a positive change.The removal of the
setfunction and direct initialization of the state with default values fromconfigRepo.get().featuressimplifies the control flow and reduces complexity. This change enhances maintainability and clarity.src/views/Map/Layouts/Mobile/Mobile.tsx (1)
12-15: Increased flexibility withReactNodeprop types.Changing the prop types from
JSX.ElementtoReactNodeallows theMobileLayoutcomponent to accept a wider range of React elements and types, enhancing its usability and versatility.src/views/Map/Layouts/Laptop/Laptop.tsx (1)
12-15: Increased flexibility withReactNodeprop types.Changing the prop types from
JSX.ElementtoReactNodeallows theLaptopLayoutcomponent to accept a wider range of React elements and types, enhancing its usability and versatility.src/views/Map/Layouts/Desktop/Desktop.tsx (1)
Line range hint
4-17: Good enhancement: Use ofReactNodefor props.Changing the prop types from
JSX.ElementtoReactNodeincreases flexibility, allowing a broader range of content types to be passed as children. This enhances the component's adaptability and usability.src/views/Map/SmartColumn/SmartColumn.tsx (1)
Line range hint
2-11: Good enhancement: Use ofReactNodefor children prop.Changing the
childrenprop type fromJSX.ElementtoReactNodeincreases flexibility, allowing a broader range of content types to be passed as children. This enhances the component's adaptability and usability.src/core/api/features.ts (1)
3-3: Rational import path change forAppFeature.Changing the import path for
AppFeatureto~core/app/typeslikely reflects a reorganization to align type definitions with the broader application context, improving maintainability and clarity.src/core/router/types.ts (1)
1-1: Import path update aligns with improved modularity.The change in the import path for
AppFeatureTypereflects a thoughtful reorganization of the module structure, enhancing the maintainability and clarity of the codebase.src/features/analytics_panel/atoms/analyticsResource.ts (1)
5-5: Import path update enhances feature categorization.The update of the import path for
AppFeaturealigns with a broader application context, supporting improved modularity and feature categorization.src/features/current_event/atoms/currentEventGeometry.ts (1)
4-4: Import path update supports improved modularity.The change in the import path for
AppFeaturealigns with a broader application context, enhancing the modularity and maintainability of the codebase.src/core/metrics/constants.ts (2)
1-1: Import source update is appropriate.The change in the import path for
AppFeaturealigns with its new context in the application, as described in the PR summary.
1-1: Removal of unused import is correct.The
AppFeatureTypeimport has been removed, indicating it is no longer necessary for this module.src/core/config/index.ts (1)
Line range hint
12-22:
Enhancements to configuration management are well-implemented.The addition of
initialUrlDataand changinginitialUrlto astringimprove the flexibility and usability of theConfigRepository.However, ensure that all usages of
initialUrlandinitialUrlDatain the codebase are updated to reflect these changes.src/views/Map/Layouts/Layout.tsx (1)
22-30: Prop type update enhances flexibility.Changing the prop types to
ReactNodeallows theLayoutcomponent to accept a wider range of children, improving its usability.src/core/app/types.ts (1)
Line range hint
1-66:
Verify the impact of removingUserStateStatusandUserStateType.The removal of
UserStateStatusandUserStateTypeindicates a shift in user state management. Ensure that their removal does not affect other parts of the codebase that might have relied on these types.Verification successful
No Impact from Removing
UserStateStatusandUserStateTypeThe search across the codebase found no remaining references to
UserStateStatusandUserStateType. This indicates that their removal does not affect other parts of the codebase.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining references to `UserStateStatus` and `UserStateType` in the codebase. # Test: Search for references to `UserStateStatus` and `UserStateType`. rg --type typescript 'UserStateStatus|UserStateType'Length of output: 89
Script:
#!/bin/bash # Description: Correctly check for any remaining references to `UserStateStatus` and `UserStateType` in the codebase. # Test: Search for references to `UserStateStatus` and `UserStateType` in .ts files. rg 'UserStateStatus|UserStateType' --glob '*.ts'Length of output: 50
src/features/events_list/atoms/eventListResource.ts (1)
7-7: LGTM! The import path change forAppFeatureis appropriate.The change in import path aligns with the refactoring to centralize feature types. Ensure that all usages of
AppFeatureare consistent with its intended purpose.src/core/config/loaders/stageConfigLoader.ts (1)
2-2: LGTM! The import path change forAppFeatureTypeis appropriate.The change in import path aligns with the refactoring to centralize feature types. Ensure that all usages of
AppFeatureTypeare consistent with its intended purpose.src/features/llm_analytics/atoms/llmAnalyticsResource.ts (1)
6-6: Import source update forAppFeature.The import source for
AppFeaturehas been updated to~core/app/types, reflecting a restructuring of the codebase. Ensure that all references toAppFeatureare updated accordingly throughout the project.src/core/config/types.ts (1)
8-9: Type modification and addition inConfig.The
initialUrlproperty type has been changed fromUrlDatatostring, andinitialUrlDatahas been added. This change simplifies the handling ofinitialUrlwhile retaining detailed URL information throughinitialUrlData. Ensure that the rest of the application correctly handles these changes, especially whereinitialUrlwas previously used asUrlData.Verification successful
Ensure correct handling of
initialUrlandinitialUrlDatachanges.The change from
UrlDatatostringforinitialUrland the addition ofinitialUrlDataappear to be correctly handled in the codebase. The usage insrc/core/app/userWasLanded.tstreatsinitialUrlas a string, andinitialUrlDatais utilized insrc/core/app/postAppInit.ts. Verify that no other parts of the application still expectinitialUrlto be of typeUrlData.
- Check
src/core/app/userWasLanded.tsfor correct usage ofinitialUrl.- Check
src/core/app/postAppInit.tsfor correct usage ofinitialUrlData.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify usage of `initialUrl` and `initialUrlData` in the codebase. # Test: Check for usage of `initialUrl` and `initialUrlData` to ensure correct handling. rg --type ts 'initialUrl' -A 3 rg --type ts 'initialUrlData' -A 3Length of output: 3397
src/views/CommonView.tsx (1)
52-52: Improved loading feedback withSuspense.The addition of
SuspensewithFullScreenLoaderaroundchildrenenhances the user experience by providing a loading indicator during asynchronous operations. This change ensures users receive immediate feedback when components are loading.src/core/api_client/tests/_clientTestsContext.ts (3)
49-52: Modularize the authentication logic.The instantiation of
ApiClientandOidcSimpleClientis well-structured, separating concerns between API interactions and authentication. This enhances maintainability.
3-3: Ensure theOidcSimpleClientimport path is correct.The import path for
OidcSimpleClientshould be verified to ensure it matches the project's directory structure.Verification successful
Import path for
OidcSimpleClientis correct.The file
OidcSimpleClient.tsis located at./src/core/auth/, matching the import path in the code. No changes are needed.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the import path for `OidcSimpleClient`. # Test: Check if the file exists at the specified path. fd 'OidcSimpleClient.ts' --exec echo "File found: {}"Length of output: 101
55-55: Verify the Keycloak realm URL.Ensure that the Keycloak realm URL is correctly configured and matches the expected format for your environment.
Verification successful
Keycloak Realm URL Configuration Verified
The Keycloak realm URL is dynamically constructed using the
keycloakRealmvariable in the code. The format appears correct as per the implementation insrc/core/api_client/tests/_clientTestsContext.ts. Ensure that thekeycloakRealmvariable is set to the expected value elsewhere in your codebase.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the Keycloak realm URL configuration. # Test: Check for the expected URL pattern in the codebase. rg --type ts 'realms/keycloak_mock_realm'Length of output: 41
Script:
#!/bin/bash # Description: Verify the Keycloak realm URL configuration using the pattern from the code snippet. # Test: Check for the expected URL pattern in the codebase. rg --type ts 'realms/\$\{keycloakRealm\}'Length of output: 169
src/core/router/components/Router.tsx (2)
52-52: MakeinitRouterprivate for internal use.The change to make
initRouterprivate aligns with encapsulating routing logic within the module. Ensure no external dependencies rely on this function.
12-12: Verify the import path foruserWasLanded.Ensure the updated import path for
userWasLandedis correct and reflects the intended module reorganization.Verification successful
Import path and exports verified successfully.
The import path for
userWasLandedinRouter.tsxis correct, and bothlandUseranduserWasLandedare properly exported fromuserWasLanded.ts.
- Import statement in
src/core/router/components/Router.tsxis correct.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the import path for `userWasLanded`. # Test: Check if the file exists at the specified path. fd 'userWasLanded.ts' --exec echo "File found: {}"Length of output: 94
Script:
#!/bin/bash # Description: Verify the exports from `userWasLanded.ts`. # Test: Check if `landUser` and `userWasLanded` are exported from the file. rg 'export .*landUser' ./src/core/app/userWasLanded.ts -A 1 rg 'export .*userWasLanded' ./src/core/app/userWasLanded.ts -A 1Length of output: 292
src/core/api_client/types.ts (1)
22-22: SimplifyApiClientConfigtype.The simplification of
ApiClientConfigto only includebaseUrlreduces complexity. Ensure that authentication details are managed elsewhere if needed.src/core/router/routes.tsx (1)
12-12: Import path update forAppFeature.The import path for
AppFeaturehas been updated from~core/auth/typesto~core/app/types. This change reflects a reorganization, likely to better align with the application's architecture. Ensure that this change is consistent across the codebase whereAppFeatureis used.Verification successful
Import path update for
AppFeatureis consistent across the codebase.The import path for
AppFeaturehas been consistently updated to~core/app/typesin all relevant files. No instances of the old path~core/auth/typeswere found. This confirms the reorganization is correctly reflected throughout the codebase.
- Verified files include:
src/features/llm_analytics/atoms/llmAnalyticsResource.tssrc/features/events_list/atoms/eventListResource.tssrc/features/current_event/atoms/currentEventGeometry.tssrc/features/analytics_panel/atoms/analyticsResource.tssrc/core/shared_state/featureFlags.tssrc/core/router/routes.tsxsrc/core/metrics/constants.tssrc/core/metrics/app-metrics.tssrc/core/api/features.tsScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify consistency of `AppFeature` import path across the codebase. # Test: Search for `AppFeature` import statements. Expect: All paths should be `~core/app/types`. rg --type tsx --type ts --type js --type jsx --pattern "import { AppFeature } from"Length of output: 497
Script:
#!/bin/bash # Description: Verify consistency of `AppFeature` import path across the codebase. # Test: Search for `AppFeature` import statements. Expect: All paths should be `~core/app/types`. rg "import { AppFeature } from" --type tsx --type ts --type js --type jsxLength of output: 193
Script:
#!/bin/bash # Description: Verify consistency of `AppFeature` import path across the codebase. # Test: Search for `AppFeature` import statements. Expect: All paths should be `~core/app/types`. rg "import \{ AppFeature \} from" --type tsx --type ts --type js --type jsxLength of output: 105
Script:
#!/bin/bash # Description: Verify consistency of `AppFeature` import path across the codebase. # Test: Search for `AppFeature` import statements. Expect: All paths should be `~core/app/types`. rg "import \{ AppFeature \} from" --type ts --type js --type jsxLength of output: 94
Script:
#!/bin/bash # Description: Verify consistency of `AppFeature` import path across the codebase. # Test: Search for `AppFeature` import statements. Expect: All paths should be `~core/app/types`. rg "import \{ AppFeature \} from" --type ts --type jsLength of output: 843
src/features/events_list/components/EventsPanel/EventsPanel.tsx (1)
59-59: EnsurehasTimelineis a boolean.The use of
!!ensures thathasTimelineis always a boolean. This change enhances the robustness of the component by preventing unexpected behavior due to improper prop types.src/boot.ts (6)
2-2: Introduction ofauthClientInstance.The addition of
authClientInstancesignifies a more structured approach to authentication. This change enhances security and token management within the application.
27-30: Initialization ofauthClientInstance.The
authClientInstanceis initialized with parameters fromstageConfig. This centralizes authentication logic and may improve maintainability.
38-38: Transition fromsetuptoinitforreportsClient.Aligning
reportsClientwith the new initialization structure enhances consistency and maintainability.
Line range hint
50-61:
Refinement of user data handling.The logic for handling user data has been simplified, reducing complexity and focusing on defaults for anonymous sessions.
82-82: Update toconfigRepo.setwithinitialUrlData.The change to
initialUrlDatareflects a more descriptive naming convention, enhancing clarity regarding the purpose of the stored data.
34-34: Transition fromsetuptoinitforapiClient.The change from
setuptoinitsuggests a more flexible API client initialization process. Ensure that this change is consistent across the codebase.Verification successful
Transition from
setuptoinitforapiClientis consistent.The change from
setuptoinithas been applied consistently across the codebase. There are no remaining usages ofapiClient.setup, ensuring a uniform initialization process withapiClient.init.
apiClient.initis used in:
src/boot.tssrc/core/api_client/tests/_clientTestsContext.tsScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify consistency of `apiClient.init` usage across the codebase. # Test: Search for `apiClient.init` usage. Expect: All usages should align with the new method signature. rg --type ts --type tsx --pattern "apiClient.init"Length of output: 462
Script:
#!/bin/bash # Description: Verify consistency of `apiClient.init` and check for any remaining `apiClient.setup` usage across the codebase. # Search for usage of `apiClient.init` rg "apiClient.init" --type ts --type tsx # Search for any remaining usage of `apiClient.setup` rg "apiClient.setup" --type ts --type tsxLength of output: 138
Script:
#!/bin/bash # Description: Verify consistency of `apiClient.init` and check for any remaining `apiClient.setup` usage across the codebase. # Search for usage of `apiClient.init` in TypeScript files rg "apiClient.init" --glob "*.ts" # Search for any remaining usage of `apiClient.setup` in TypeScript files rg "apiClient.setup" --glob "*.ts"Length of output: 179
src/core/api_client/tests/authorization.test.ts (2)
25-25: Refactor to useauthClientfor login.The change from
ctx.apiClient.logintoctx.authClient.loginreflects the refactoring to separate authentication logic. This aligns with the PR objectives of consolidating token management withinOidcSimpleClient.
206-206: Refactor to useauthClientfor login.The change from
ctx.apiClient.logintoctx.authClient.loginis consistent with the refactoring effort to centralize authentication logic. This improves code clarity and aligns with the PR objectives.src/core/api_client/apiClient.ts (6)
31-35: Simplify constructor by removingstorage.The constructor no longer requires
storage, simplifying the initialization. Ensure that any previous dependencies onstorageare adequately addressed elsewhere in the code.
37-43: Addinitmethod for base URL configuration.The new
initmethod replaces thesetupmethod, focusing on configuring the base URL. This change streamlines the initialization process. Ensure that this method is called appropriately during client setup.
Line range hint
58-98:
Updatecallmethod for authentication handling.The
callmethod now usesauthServicefor token management, simplifying the authorization process. The use of a catcher for 401 errors to refresh tokens is a robust approach. Ensure that this logic is thoroughly tested.
118-129: Handle unauthorized errors with token refresh.The updated logic attempts to refresh the token upon receiving a 401 error, improving user experience. Ensure that the
authServicehandles logout scenarios correctly.
172-174: DefaultuseAuthtofalsein request methods.The default value for
useAuthis nowfalse, reflecting a more explicit approach to authorization. This change aligns with the goal of reducing unnecessary authentication checks. Verify that all necessary requests explicitly setuseAuthtotruewhere needed.Also applies to: 181-183, 190-192, 199-201, 207-209
19-19: EnsureOidcSimpleClientis correctly implemented.The integration of
OidcSimpleClientfor authentication centralizes token management. Verify that this client is correctly implemented and configured to handle all necessary authentication tasks.Verification successful
OidcSimpleClientis correctly implemented.The
OidcSimpleClientclass insrc/core/auth/OidcSimpleClient.tsis well-implemented, covering essential authentication functionalities such as token management, refreshing tokens, and session handling. It effectively centralizes these tasks, ensuring robust authentication management.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of `OidcSimpleClient`. # Test: Search for the implementation of `OidcSimpleClient`. ast-grep --lang typescript --pattern $'class OidcSimpleClient { $$$ }'Length of output: 17851
src/core/metrics/app-metrics.ts (1)
3-3: Update import path forAppFeature.The import path for
AppFeaturehas been updated to reflect its broader application context. This change improves the organization of type imports, aligning them with their functional domains.src/views/Map/Map.tsx (4)
Line range hint
40-109: Verify feature flags handling and dynamic imports.Ensure that all feature flags are correctly handled and initialized. Verify that dynamic imports based on feature flags are efficient and do not introduce performance bottlenecks.
Verification successful
Feature Flags and Dynamic Imports Verified
The feature flags in
src/views/Map/Map.tsxare correctly handled, and the dynamic imports are efficiently managed based on these flags. This approach optimizes performance by loading only the necessary modules. No issues were found with the current implementation.
- The feature flags are extensively used to conditionally import modules.
- Dynamic imports are efficiently triggered based on these flags.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify handling of feature flags and dynamic imports. # Test: Search for feature flags usage and ensure all are defined and used correctly. rg --type javascript --type typescript 'FeatureFlag\.[A-Z_]+' # Test: Check for dynamic imports and ensure they are efficient. rg --type javascript --type typescript 'import\('Length of output: 185
Script:
#!/bin/bash # Description: Verify handling of feature flags and dynamic imports. # Test: Search for feature flags usage and ensure all are defined and used correctly. rg --type js --type ts 'FeatureFlag\.[A-Z_]+' # Test: Check for dynamic imports and ensure they are efficient. rg --type js --type ts 'import\('Length of output: 9758
Line range hint
177-197: Verify feature flag usage inAnalytics.Ensure that the feature flags used within the
Analyticscomponent are correctly accessed and utilized. Confirm that the logic aligns with the intended functionality.Verification successful
Feature flag usage in
Analyticscomponent is correct.The feature flag
FeatureFlag.ANALYTICS_PANELis correctly accessed and utilized within theAnalyticscomponent to determine theisAnalyticsOnstate and theinitialStateof theFullAndShortStatesPanelWidget. This aligns with the intended functionality.
src/views/Map/Map.tsx:
const isAnalyticsOn = !!featureFlags[FeatureFlag.ANALYTICS_PANEL];initialState={featureFlags[FeatureFlag.ANALYTICS_PANEL] ? 'short' : null}Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify feature flag usage in `Analytics` component. # Test: Search for feature flag usage within `Analytics` and ensure correctness. rg --type javascript --type typescript 'featureFlags\[FeatureFlag\.ANALYTICS_PANEL\]'Length of output: 122
Script:
#!/bin/bash # Description: Verify feature flag usage in `Analytics` component. # Test: Search for feature flag usage within `Analytics` and ensure correctness. rg --type js --type ts 'featureFlags\[FeatureFlag\.ANALYTICS_PANEL\]'Length of output: 264
Line range hint
204-213: Verify feature flag usage inLayersAndLegends.Ensure that the feature flags used within the
LayersAndLegendscomponent are correctly accessed and utilized. Confirm that the logic aligns with the intended functionality.Verification successful
Feature flag usage in
LayersAndLegendsis correct.The feature flag
FeatureFlag.MAP_LAYERS_PANELis correctly used in theLayersAndLegendscomponent to conditionally render thelayersPanel. This aligns with the intended functionality of toggling UI components based on feature flags.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify feature flag usage in `LayersAndLegends` component. # Test: Search for feature flag usage within `LayersAndLegends` and ensure correctness. rg --type javascript --type typescript 'featureFlags\[FeatureFlag\.MAP_LAYERS_PANEL\]'Length of output: 123
Script:
#!/bin/bash # Description: Verify feature flag usage in `LayersAndLegends` component. # Test: Search for feature flag usage within `LayersAndLegends` and ensure correctness. rg --type typescript --type tsx 'featureFlags\[FeatureFlag\.MAP_LAYERS_PANEL\]'Length of output: 116
Script:
#!/bin/bash # Description: Verify feature flag usage in `LayersAndLegends` component. # Test: Search for feature flag usage within the `src/views/Map` directory. rg 'featureFlags\[FeatureFlag\.MAP_LAYERS_PANEL\]' src/views/MapLength of output: 159
22-22: Verify initialization ofconfigRepo.get().features.Ensure that
configRepo.get().featuresis correctly initialized and available at the time of access to prevent potential runtime errors.src/components/ConnectedMap/map-libre-adapter/index.tsx (1)
Line range hint
43-245: Verify refactoring inMapboxMap.Ensure that the refactoring of the
MapboxMapcomponent does not introduce regressions or affect its functionality. Confirm that the focus on map styles and layers aligns with the intended design.package.json (12)
119-119: Verify update of@types/nodeto^20.14.11.Ensure that the update from
^20.14.9to^20.14.11does not introduce breaking changes or affect the codebase.
201-201: Verify update oftypescriptto5.5.3.Ensure that the update from
5.4.5to5.5.3does not introduce issues or affect the codebase.
103-103: Verify update of@sentry/reactto^8.18.0.Ensure that the update from
^7.113.0to^8.18.0does not introduce breaking changes or affect the codebase.
97-97: Verify update of@reatom/coreto^3.9.0.Ensure that the update from
^3.8.3to^3.9.0does not introduce breaking changes or affect the codebase.
206-206: Verify update ofviteto~5.3.4.Ensure that the update from
~5.3.3to~5.3.4does not introduce issues or affect the codebase.
104-104: Verify update of@slack/web-apito^7.3.1.Ensure that the update from
^7.0.4to^7.3.1does not introduce breaking changes or affect the codebase.
149-149: Verify update ofeslint-plugin-reactto^7.34.4.Ensure that the update from
^7.34.3to^7.34.4does not introduce issues or affect the codebase.
223-223: Verify update ofsemverto^7.6.3.Ensure that the update from
^7.6.0to^7.6.3does not introduce issues or affect the codebase.Verification successful
No direct usage of
semverfound in the codebase.The
semverpackage is not directly used in JavaScript or TypeScript files, and its update to^7.6.3aligns with the versions inpnpm-lock.yaml. This suggests the update is unlikely to introduce issues. Further manual verification is not necessary unless indirect usage is suspected.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify update of `semver` to `^7.6.3`. # Test: Check for usage of `semver` in the codebase and ensure compatibility with the new version. rg --type javascript --type typescript 'semver'Length of output: 82
Script:
#!/bin/bash # Description: Verify update of `semver` to `^7.6.3`. # Test: Check for usage of `semver` in the codebase and ensure compatibility with the new version. rg --type js --type ts 'semver'Length of output: 31
Script:
#!/bin/bash # Description: Check for usage of `semver` in other file types or configurations. # Test: Search in JSON, YAML, and shell script files for references to `semver`. rg --type json --type yaml --type sh 'semver'Length of output: 1125
132-132: Verify update of@vitest/coverage-v8to^2.0.3.Ensure that the update from
^1.6.0to^2.0.3does not introduce breaking changes or affect existing tests.
106-106: Verify update of@testing-library/reactto^16.0.0.Ensure that the update from
^14.2.1to^16.0.0does not introduce breaking changes or affect existing tests.
210-210: Verify update ofvitestto^2.0.3.Ensure that the update from
^1.6.0to^2.0.3does not introduce breaking changes or affect existing tests.
105-105: Verify addition of@testing-library/domat^10.3.2.Ensure that the addition of
@testing-library/domdoes not introduce conflicts and is compatible with existing tests.src/core/auth/OidcSimpleClient.ts (17)
1-10: Imports look good.The imported modules and types are relevant to the functionality implemented in the file.
12-13: Constants are well-defined.The constants
LOCALSTORAGE_AUTH_KEYandTIME_TO_REFRESH_MSare appropriately named and used.
15-31: Class declaration and constructor are clear.The
OidcSimpleClientclass is well-structured, and the constructor initializes the storage appropriately.
33-52:initmethod is well-structured.The method initializes necessary fields and performs token checks effectively. The use of environment checks is appropriate.
58-82:storeTokensmethod is effective.The method efficiently stores tokens and handles errors using
parseToken.
88-118:checkLocalAuthTokenmethod is well-implemented.The method effectively checks and validates tokens from local storage.
120-126:resetAuthmethod is straightforward.The method effectively clears authentication tokens and resets the state.
128-138:setAuthmethod is clear.The method effectively sets tokens and their expiration dates.
140-154:_tokenRefreshFlowmethod is well-implemented.The method effectively checks and refreshes tokens as needed.
161-168:getAccessTokenmethod is clear.The method effectively retrieves and refreshes the access token as needed.
170-174:isRefreshTokenExpiredmethod is straightforward.The method effectively checks if the refresh token is expired.
179-202:refreshAuthTokenmethod is well-implemented.The method effectively refreshes tokens and handles errors appropriately.
207-223:requestTokenOrThrowmethod is clear.The method effectively requests tokens and handles errors.
243-254:authenticatemethod is clear.The method effectively handles user authentication and page reload.
256-272:endSessionmethod is straightforward.The method effectively ends the session and resets authentication.
278-283:logoutmethod is clear.The method effectively handles user logout and page reload.
286-299:parseTokenfunction is well-implemented.The function effectively decodes JWTs and checks for expiration.
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
Files selected for processing (5)
- .eslintrc.json (1 hunks)
- package.json (6 hunks)
- src/core/draw_tools/atoms/temporaryGeometryAtom.ts (1 hunks)
- src/core/store/store.ts (1 hunks)
- src/types/entry.ts (2 hunks)
Additional comments not posted (20)
src/types/entry.ts (3)
9-10: Update ESLint directive for empty object types.The change from
@typescript-eslint/ban-typesto@typescript-eslint/no-empty-object-typealigns with updated linting rules for better handling of empty object types.
19-24: Ensure consistent handling of empty object types.The update to the ESLint directive for the
Entrytype maintains consistency across the codebase and aligns with best practices.
26-28: Adhere to updated linting standards.The update to the ESLint directive for the
typedObjectEntriesfunction improves maintainability and aligns with current linting practices.src/core/draw_tools/atoms/temporaryGeometryAtom.ts (1)
20-23: Enhance immutability and clarity in feature modification.The updated logic ensures immutability by always creating a new
propertiesobject, improving code clarity and robustness..eslintrc.json (3)
44-44: Encourage cleanup of unused variables.Changing
@typescript-eslint/no-unused-varsfrom "off" to "warn" encourages developers to clean up unused variables, enhancing code quality.
45-45: Promote clearer code with unused expression warnings.Changing
no-unused-expressionsfrom "off" to "warn" helps promote clearer and more intentional code usage.
46-46: Emphasize meaningful expressions in TypeScript.Adding
@typescript-eslint/no-unused-expressionswith a severity of "warn" aligns with best practices by emphasizing the importance of meaningful expressions.src/core/store/store.ts (2)
11-11: Simplified store initialization is beneficial.The direct instantiation and export of the store improve clarity and reduce complexity.
Line range hint
16-56:
Ensure logging is disabled in production environments.The logger function is detailed and useful for development, but verify that it does not run in production to avoid performance impacts.
package.json (11)
104-104: Minor version update for@slack/web-apiis likely safe.The update to version
^7.3.1should not introduce breaking changes.
119-119: Patch update for@types/nodeis likely safe.The update to version
^20.14.11should not introduce breaking changes.
149-149: Minor version update foreslint-plugin-reactis likely safe.The update to version
^7.35.0should not introduce breaking changes.
205-205: Patch update forviteis likely safe.The update to version
~5.4.0should not introduce breaking changes.
132-132: Verify compatibility with@vitest/coverage-v8version^2.0.5.Major version updates might introduce breaking changes. Ensure that all necessary migration steps are followed.
103-103: Verify compatibility with@sentry/reactversion^8.18.0.Major version updates might introduce breaking changes. Ensure that all necessary migration steps are followed.
208-208: Verify compatibility withvite-tsconfig-pathsversion^5.0.1.Major version updates might introduce breaking changes. Ensure that all necessary migration steps are followed.
209-209: Verify compatibility withvitestversion^2.0.5.Major version updates might introduce breaking changes. Ensure that all necessary migration steps are followed.
97-97: Verify compatibility with@reatom/coreversion^3.9.0.Ensure that the minor version update does not introduce breaking changes.
106-106: Verify compatibility with@testing-library/reactversion^16.0.0.Major version updates might introduce breaking changes. Ensure that all necessary migration steps are followed.
200-200: Verify compatibility withtypescriptversion5.5.3.Ensure that the minor version update does not introduce breaking changes.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
Files selected for processing (2)
- package.json (7 hunks)
- src/core/api_client/apiClient.ts (8 hunks)
Additional comments not posted (9)
src/core/api_client/apiClient.ts (4)
35-41:initmethod addition approved.The
initmethod simplifies the client setup and aligns with the PR objectives of reducing complexity and potential errors during initialization.
Line range hint
170-209:
Method signature changes approved.The default
useAuth = falsealigns with the updated authentication strategy, ensuring that requests are not authenticated by default unless specified.
29-33: Constructor changes approved, but verify impact.The removal of the
storageparameter aligns with the new token management strategy usingOidcSimpleClient. Ensure that this change does not affect other parts of the codebase that might have relied on this parameter.Verification successful
No issues found with the removal of the
storageparameter fromApiClient.The instances of
ApiClientin the codebase do not rely on thestorageparameter, confirming that its removal does not impact other parts of the codebase. The change aligns with the new token management strategy.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the impact of removing the `storage` parameter from the `ApiClient` constructor. # Test: Search for instances where `ApiClient` is instantiated. Expect: No reliance on the `storage` parameter. rg --type ts -A 5 $'new ApiClient'Length of output: 5789
Line range hint
56-127:
callmethod changes approved, but verify token refresh logic.The integration with
OidcSimpleClientstreamlines authentication handling and improves user experience by attempting to refresh tokens on 401 errors. Ensure that the token refresh logic functions as expected in all scenarios.Verification successful
Token refresh logic verified and functions as expected.
The
OidcSimpleClientclass effectively handles token refresh operations. ThegetAccessTokenmethod ensures tokens are refreshed when nearing expiration, and therefreshAuthTokenmethod manages the refresh process, logging out the user if the refresh token is expired. This aligns with the expected behavior.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the token refresh logic in the `call` method. # Test: Search for the `getAccessToken` method usage in `OidcSimpleClient`. Expect: Correct handling of token refresh. ast-grep --lang typescript --pattern $'class OidcSimpleClient { $$$ getAccessToken() { $$$ } $$$ }'Length of output: 17892
package.json (5)
200-200: Verify compatibility oftypescriptversion upgrade.The version upgrade from
5.4.5to5.5.3may introduce new features or improvements. Ensure compatibility with the existing TypeScript code.
205-205: Verify compatibility ofviteversion upgrade.The version upgrade from
~5.3.3to~5.4.0may introduce improvements or bug fixes. Ensure compatibility with the existing build process.
103-103: Verify compatibility of@sentry/reactversion upgrade.The version upgrade from
^7.113.0to^8.18.0may introduce breaking changes or new functionalities. Ensure compatibility with the existing codebase.
97-97: Verify compatibility of@reatom/coreversion upgrade.The version upgrade from
^3.8.3to^3.9.0may introduce new features or bug fixes. Ensure compatibility with the existing codebase.
106-106: Verify compatibility of@testing-library/reactversion upgrade.The version upgrade from
^14.2.1to^16.0.0suggests potential breaking changes or new functionalities. Ensure compatibility with the existing test suite.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
Files selected for processing (3)
- package.json (7 hunks)
- src/core/pages/index.tsx (2 hunks)
- src/core/url_store/readInitialUrl.ts (1 hunks)
Additional comments not posted (4)
src/core/url_store/readInitialUrl.ts (1)
5-7: Verify the impact of removing thelayersmodification.The removal of the line modifying
layerssimplifies the function, but ensure that any downstream processes depending on the alteredlayersare updated accordingly.Verification successful
No impact from removing
layersmodification inreadInitialUrl.The removal of the
layersmodification inreadInitialUrlappears to be isolated and does not affect other parts of the codebase. The function's usage insrc/boot.tsdoes not indicate any dependency on the modifiedlayers. Therefore, no further updates are necessary.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the impact of removing the `layers` modification in `readInitialUrl`. # Test: Search for usage of `readInitialUrl` and check for dependencies on `layers` modification. rg --type ts -A 5 $'readInitialUrl'Length of output: 999
Script:
#!/bin/bash # Search for references to `layers` in the codebase to determine if its modification in `readInitialUrl` had any downstream effects. rg --type ts 'layers'Length of output: 100731
src/core/pages/index.tsx (2)
80-107: Encapsulate link behavior inCustomLink.The
CustomLinkcomponent effectively separates link handling logic, improving modularity. Ensure that all link types are correctly handled, especially any custom link behaviors.
68-78: Transition tomarkdown-to-jsxfor Markdown rendering.The transition from
ReactMarkdownwithremark-gfmtomarkdown-to-jsxsimplifies the component but verify that all markdown features previously supported are still correctly rendered.package.json (1)
Line range hint
97-209:
Verify integration of updated dependencies.Ensure that all updated dependencies, especially those with potential breaking changes, are correctly integrated and tested within the codebase.
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
Files selected for processing (2)
- package.json (7 hunks)
- src/features/events_list/components/EventCard/EventCard.tsx (2 hunks)
Additional comments not posted (2)
src/features/events_list/components/EventCard/EventCard.tsx (1)
42-42: Verify the format ofevent.updatedAt.The change to
new Date(event.updatedAt)assumes thatevent.updatedAtis in a format directly compatible with the JavaScript Date constructor. Ensure that the input format is consistent and will not lead to unexpected results.package.json (1)
105-105: Consider removing@testing-library/domif not directly used.The
@testing-library/dompackage is not directly referenced in the codebase. Since@testing-library/reactis used, which already depends on@testing-library/dom, you might not need to include it separately unless specific functionalities from@testing-library/domare required.
vkozio
deleted the
18520-log-out-user-on-kontur-app-if-a-session-is-expired
branch
implemented end session flow via endpoint
automatic logout only on 401 from dedicated request to token endpoint
refresh token during authClient init, all requests will use fresh token from app start
Refactor:
extracted token management from apiClient to OidcSimpleClient,
authClientInstance is OidcSimpleClient now, AuthClient and irrelevant code removed, necessary methods are used directly
adjusted naming in auth-related code to align more with standarts and existing solutions
move ~core/auth/types to ~core/app/types as it has app-related types
simplify and streamline init flow, remove unecessary atoms dependencies, reduce early activation issues
reduce reactive usage of featureFlags
Addidtional fixes:
add Suspense with preloader in view hierarchy to keep sidebar visible during loading, fixes (19141) https://kontur.fibery.io/Tasks/Task/White-screen-fails-while-loading-application-19141
map adapter: removed heavy react-dom/server dependency (~50kb in bundle) and unused or irrelevant code
drop date-fns in favor of native function
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes