Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: revamp the frontend architecture #6598

Merged
merged 44 commits into from
Dec 20, 2024
Merged

Conversation

vikrantgupta25
Copy link
Collaborator

@vikrantgupta25 vikrantgupta25 commented Dec 5, 2024

Refactor

This PR takes care of moving away from redux as much as possible and streamlining the routing , sync setup calls and making sure we have deterministic actions based on API results. All the test cases have been fixed in sync with the latest design architecture

Contributes to - #5621 , https://github.com/SigNoz/engineering-pod/issues/2068

@vikrantgupta25
Copy link
Collaborator Author

Copy link

github-actions bot commented Dec 5, 2024

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

@github-actions github-actions bot added the enhancement New feature or request label Dec 5, 2024
@vikrantgupta25 vikrantgupta25 changed the title feat: setup the app context to fetch users,licenses and feature flags feat: architectural changes for app routes and private routes [ deprecating redux ] Dec 5, 2024
@vikrantgupta25 vikrantgupta25 changed the title feat: architectural changes for app routes and private routes [ deprecating redux ] feat: architectural changes for app routes and private routes [deprecating redux] Dec 5, 2024
Copy link

github-actions bot commented Dec 5, 2024

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

1 similar comment
Copy link

github-actions bot commented Dec 5, 2024

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

@vikrantgupta25 vikrantgupta25 changed the title feat: architectural changes for app routes and private routes [deprecating redux] feat: revamp the frontend architecture Dec 5, 2024
Copy link

github-actions bot commented Dec 5, 2024

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

1 similar comment
Copy link

github-actions bot commented Dec 5, 2024

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

@vikrantgupta25 vikrantgupta25 changed the title feat: revamp the frontend architecture chore: revamp the frontend architecture Dec 5, 2024
Copy link

github-actions bot commented Dec 5, 2024

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

@github-actions github-actions bot added the chore label Dec 5, 2024
@vikrantgupta25 vikrantgupta25 removed the enhancement New feature or request label Dec 5, 2024
Copy link

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

@vikrantgupta25
Copy link
Collaborator Author

contributes to - #5621

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Looks good to me! Incremental review on a95d218 in 13 minutes and 33 seconds

More details
  • Looked at 109 lines of code in 4 files
  • Skipped 0 files when reviewing.
  • Skipped posting 4 drafted comments based on config settings.
1. frontend/src/container/LogsExplorerViews/index.tsx:292
  • Draft comment:
    Remove the unnecessary console.log statement to clean up the code.
  • Reason this comment was not posted:
    Comment looked like it was already resolved.
2. frontend/src/container/Login/index.tsx:86
  • Draft comment:
    Avoid using inline styles. Replace inline styles with CSS classes or styled components where applicable.
  • Reason this comment was not posted:
    Comment was not on a valid diff hunk.
3. frontend/src/container/Login/index.tsx:1
  • Draft comment:
    Avoid using index.tsx for component files. Use a more descriptive file name for better debugging and searching.
  • Reason this comment was not posted:
    Comment was on unchanged code.
4. frontend/src/container/LogsExplorerViews/index.tsx:292
  • Draft comment:
    Avoid using index.tsx for component files. Use a more descriptive file name for better debugging and searching.
  • Reason this comment was not posted:
    Marked as duplicate.

Workflow ID: wflow_tYinoYp2xQoGc5Vq


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

YounixM
YounixM previously approved these changes Dec 18, 2024
Copy link
Member

@YounixM YounixM left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor Comments, can be taken up in a separate PR.

ahmadshaheer
ahmadshaheer previously approved these changes Dec 19, 2024
@vikrantgupta25 vikrantgupta25 changed the base branch from develop to main December 19, 2024 10:19
@vikrantgupta25 vikrantgupta25 dismissed stale reviews from ahmadshaheer and YounixM December 19, 2024 10:19

The base branch was changed.

frontend/src/tests/test-utils.tsx Dismissed Show resolved Hide resolved
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Looks good to me! Incremental review on 420915e in 2 minutes and 8 seconds

More details
  • Looked at 49 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 3 drafted comments based on config settings.
1. frontend/src/AppRoutes/Private.tsx:106
  • Draft comment:
    Avoid using any type for route parameter. Define a specific type for route to improve type safety and code readability. This applies to navigateToWorkSpaceSuspended as well.
  • Reason this comment was not posted:
    Comment was on unchanged code.
2. frontend/src/AppRoutes/Private.tsx:123
  • Draft comment:
    Avoid using entire objects like mapRoutes in dependency arrays of useEffect. Use specific properties or values instead to prevent unnecessary re-renders. This applies to the useEffect on line 133 as well.
  • Reason this comment was not posted:
    Comment was on unchanged code.
3. frontend/src/AppRoutes/Private.tsx:62
  • Draft comment:
    Avoid using the component/index.tsx file structure approach, as it makes it difficult to debug and find components using global search tools like VS Code.
  • Reason this comment was not posted:
    Comment was on unchanged code.

Workflow ID: wflow_RSQj50oZqKn7eelE


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link
Member

@srikanthccv srikanthccv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am approving to unblock.

@vikrantgupta25 vikrantgupta25 merged commit 26fe5e4 into main Dec 20, 2024
17 of 18 checks passed
@vikrantgupta25 vikrantgupta25 deleted the cleanup-app-routes branch December 20, 2024 08:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants