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

fix: url params should not propagate across pages #5417

Merged
merged 16 commits into from
Jul 18, 2024

Conversation

vikrantgupta25
Copy link
Collaborator

@vikrantgupta25 vikrantgupta25 commented Jul 3, 2024

Summary

Issue 1:

  • dashboards list url params gets initialised with the wrong params since the DashboardsProvider is wrapped at the very top of the application.
  • moved the initialisation to pick values from the url only when present on the Dashboards List page.
  • added extra checks in place to default to updatedAt when the key is mismatched and order to descend

Issue 2:

  • when on alerts list page if the order is set to undefined then after that navigating to the old logs explorer the order is retained.
  • why? because it is being initialised in the reducer on load of the app. so moved the condition to only pick from the url on load if the user is present at the old logs explorer page else default it to desc as happening before.
  • added extra checks in place to default to desc when the key is mismatched

Issue 3:

  • the performance of rendering dashboard list page was very poor as the component was re-rendering around a 100 times.
  • refactored the component for better performance.
  • the search string was not getting updated in the URL.
  • because of this re-renders we were unable to write proper unit test for the same.

Started adding test cases for the dashboards list page

Related Issues / PR's

Fixes :- #5394

Fixes :- #5448

Screenshots

NA

Affected Areas and Manually Tested Areas

  • tested dashboard list page ( various use cases / set different sorts and goto details page and navigate back etc.)
  • tested the url isolation issue with logs and dashboards

@github-actions github-actions bot added the bug Something isn't working label Jul 3, 2024
Copy link

github-actions bot commented Jul 3, 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 fix: dashboards list url query params isolation fix: url params should not propagate across pages Jul 3, 2024
Copy link

github-actions bot commented Jul 3, 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 fix: url params should not propagate across pages fix: url params should not propagate across pages ( WIP ) Jul 3, 2024
Copy link

github-actions bot commented Jul 3, 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 Jul 3, 2024

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

Copy link

github-actions bot commented Jul 3, 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 fix: url params should not propagate across pages ( WIP ) fix: url params should not propagate across pages Jul 3, 2024
@vikrantgupta25 vikrantgupta25 marked this pull request as ready for review July 4, 2024 06:31
SagarRajput-7
SagarRajput-7 previously approved these changes Jul 8, 2024
ahmadshaheer
ahmadshaheer previously approved these changes Jul 8, 2024
@vikrantgupta25 vikrantgupta25 merged commit adfe20e into develop Jul 18, 2024
13 of 14 checks passed
@vikrantgupta25 vikrantgupta25 deleted the url-state-isolation branch July 18, 2024 06:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants