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

refactor(routing): Updates Advisor Routes to use global schema + Remove unused packages #1285

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

adonispuente
Copy link
Contributor

@adonispuente adonispuente commented Sep 25, 2024

This is mainly a routing refactor so that advisor uses the same schema as other apps. Along with some unused package clean ups.

@adonispuente adonispuente marked this pull request as ready for review November 26, 2024 17:28
@adonispuente adonispuente requested a review from a team as a code owner November 26, 2024 17:28
Copy link

codecov bot commented Nov 26, 2024

Codecov Report

Attention: Patch coverage is 6.45161% with 29 lines in your changes missing coverage. Please review.

Project coverage is 21.61%. Comparing base (51f47b4) to head (ad5ffd8).

Files with missing lines Patch % Lines
src/Routes.js 6.45% 29 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1285      +/-   ##
==========================================
- Coverage   22.23%   21.61%   -0.62%     
==========================================
  Files          96       95       -1     
  Lines        2559     2549      -10     
  Branches      811      809       -2     
==========================================
- Hits          569      551      -18     
- Misses       1990     1998       +8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@adonispuente adonispuente changed the title Refactor refactor(routing): Updates Advisor Routes to use global schema Dec 2, 2024
@adonispuente adonispuente changed the title refactor(routing): Updates Advisor Routes to use global schema refactor(routing): Updates Advisor Routes to use global schema + Remove unused packages Dec 2, 2024
@adonispuente
Copy link
Contributor Author

/retest

@bastilian
Copy link
Member

Nice refactor! ✨

There seem to be some minor issues, for example, the "Pathways" tab and the "Immutable ..." tab on the details both lead to broken pages. It might be that we need to updated the paths of the links/navigate there to fix these issues.

Another issue is that the "Zero State" flash up, even if the account has connected systems.

Other than that everything appears to work as before.

@adonispuente
Copy link
Contributor Author

@bastilian Thanks for taking a look! I adding in a loading const to avoid that flash, and it seems I had actually removed some routes on accident. I changed the way we mapped through routes so that it could handle arrays, and also added routes that render the same component into the same object to tidy things up. In the original we have 16 routes, this refactor has 16 as well.

@adonispuente
Copy link
Contributor Author

/retest

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants