feat: route to the onboarding flow when clicking the sending logs in case of no logs #4600
Conversation
vikrantgupta25
changed the title
|
Warning Rate Limit Exceeded@Vikrant2520 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 15 minutes and 19 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. WalkthroughThis update enhances the onboarding experience for users by introducing new routes and permissions for application monitoring, logs management, and infrastructure monitoring. It also includes UI adjustments to accommodate these new features and ensures a more guided and informative onboarding process. The changes streamline the next steps in user onboarding and improve the application's configurability for different user needs. Changes
Related issues
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 Configration File (
|
|
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files selected for processing (9)
- frontend/src/AppRoutes/routes.ts (1 hunks)
- frontend/src/constants/routes.ts (1 hunks)
- frontend/src/container/AppLayout/index.tsx (1 hunks)
- frontend/src/container/OnboardingContainer/OnboardingContainer.tsx (6 hunks)
- frontend/src/container/OnboardingContainer/utils/dataSourceUtils.ts (3 hunks)
- frontend/src/container/TopNav/DateTimeSelection/config.ts (1 hunks)
- frontend/src/container/TopNav/DateTimeSelectionV2/config.ts (1 hunks)
- frontend/src/utils/app.ts (1 hunks)
- frontend/src/utils/permission/index.ts (1 hunks)
Additional comments: 11
frontend/src/constants/routes.ts (1)
- 10-13: The addition of new routes for application monitoring, logs management, and infrastructure monitoring under the
GET_STARTEDsection is a positive enhancement. It aligns with the objectives to improve the onboarding flow and routing logic, enhancing the application's navigation and user experience.frontend/src/container/TopNav/DateTimeSelection/config.ts (1)
- 84-86: The addition of new routes for application monitoring, infrastructure monitoring, and logs management to the list of routes to skip in the top navigation bar configuration is logical. It enhances the user experience by not displaying irrelevant UI elements on onboarding or setup pages.
frontend/src/utils/permission/index.ts (1)
- 85-87: The addition of permissions for application monitoring, infrastructure monitoring, and logs management under the
GET_STARTEDsection is crucial for access control. It ensures that only authorized users can access these features, supporting the application's security and user management strategy.frontend/src/container/TopNav/DateTimeSelectionV2/config.ts (1)
- 120-122: The addition of new routes for application monitoring, infrastructure monitoring, and logs management to the list of routes to skip in the top navigation bar configuration for
DateTimeSelectionV2is consistent with improving user experience. It avoids displaying unnecessary UI components on specific pages.frontend/src/container/OnboardingContainer/utils/dataSourceUtils.ts (3)
- 6-10: The addition of the
ModulesMapenum is a positive change, providing a clear and maintainable way to reference different modules within the onboarding flow.- 91-131: Updating image URLs for various supported languages ensures that the correct assets are used, enhancing the UI's visual aspects. Please verify the correctness and accessibility of these URLs in the deployed environment.
- 288-293: The introduction of the
moduleRouteMapto map modules to specific routes supports the new routing logic, directly aligning with the PR's objectives to refine the onboarding flow and enhance navigation.frontend/src/AppRoutes/routes.ts (1)
- 60-60: Changing the
exactproperty fromtruetofalsefor theROUTES.GET_STARTEDroute allows for more flexible route matching. This change likely accommodates the new onboarding flow routes, enabling nested routing under theGET_STARTEDpath. Please verify its impact on the application's routing behavior to ensure it aligns with the intended user experience.frontend/src/container/OnboardingContainer/OnboardingContainer.tsx (2)
- 9-9: The addition of imports for
ROUTES,moduleRouteMap, andhistoryis appropriate for the enhancements made to the onboarding flow and routing logic. Ensure that these imports are used efficiently throughout the file to avoid any unnecessary overhead.- 205-243: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [187-212]
The introduction of the
handleNextStepfunction and modifications to thehandleNextfunction to incorporate route changes based on selected modules are significant improvements. However, ensure that theactiveStepandcurrentstate variables are managed correctly to prevent any off-by-one errors or unintended behavior during the onboarding process.frontend/src/container/AppLayout/index.tsx (1)
- 234-238: The expansion of the
renderFullScreencondition to include checks forGET_STARTED_APPLICATION_MONITORING,GET_STARTED_INFRASTRUCTURE_MONITORING, andGET_STARTED_LOGS_MANAGEMENTroutes is a logical enhancement that aligns with the objectives of improving the onboarding flow and user experience. Ensure that these route constants are correctly defined in theROUTESobject and that the logic correctly handles all scenarios where a full-screen layout is desired.
frontend/src/container/OnboardingContainer/OnboardingContainer.tsx
Outdated
Show resolved
Hide resolved
|
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (1)
- frontend/src/container/NoLogs/NoLogs.tsx (3 hunks)
Additional comments: 1
frontend/src/container/NoLogs/NoLogs.tsx (1)
- 39-39: The use of
onClick={handleLinkClick}within theTypography.Linkcomponent is a good practice for enhancing user experience through dynamic routing. However, ensure that thehrefattribute's value is still accessible to users with JavaScript disabled in their browsers. This ensures that the application remains accessible to all users.Consider verifying the accessibility of links with JavaScript disabled and ensure fallback URLs are provided if necessary.
|
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
1 similar comment
|
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
vikrantgupta25
force-pushed
the
chore/SIG-534
branch
2 times, most recently
from
75df8c4
to
f38436d
Compare
|
The simple re-route logic will be annoying because it could be possible that the user sets up the logging by following instructions but stops sending data for some reason or it could be that their ingest pattern is not continuous logging. At this point, they shouldn't be redirected because they already completed the onboarding workflow and sent some logs. The only time the user should be redirected is when they are on the platform for the first time and didn't send any logs/traces in the past. |
@srikanthccv we are not re-routing the user to the onboarding page , we render the logs screen itself and on click of |
|
The specifics of re-route are not important. It is annoying if you ask to send logs instead of showing no logs when they already completed the onboarding workflow and sent some logs. |
|
I don't know the details about the onboarding related things but we shouldn't show that screen if they already sent some logs in the past and completed the onboarding flow. |
vikrantgupta25
force-pushed
the
chore/SIG-534
branch
from
b818534
to
e63d515
Compare
vikrantgupta25
force-pushed
the
chore/SIG-534
branch
from
e63d515
to
2684173
Compare
Summary
modulein the URL params when selecting it.Related Issues / PR's
https://linear.app/signoz-io/issue/SIG-534/empty-logs-text-update
Screenshots
NA
Affected Areas and Manually Tested Areas
Summary by CodeRabbit