earthdata-search
earthdata-search copied to clipboard
EDSC-4031: Always route to saved Projects when selected.
Overview
What is the feature?
Created a SavedProjects page and route to avoid the accidental appending of route params to the project page.
What is the Solution?
- New Component
- New Route
- Updated the link for saved projects to the new route
What areas of the application does this impact?
- Saved Projects
- Projects when missing all params
Testing
Reproduction steps
This is confirmable by:
- Logging in to EDSC
- Selecting the save project icon in the right hand tool bar
- Entering a project name
- Adding a collection to the project via the plus button on a collection
- Clicking the user icon in the right hand tool bar and selecting "saved projects"
- You should observe that you are routed to '/savedProjects' and it is in face your saved projects.
Attachments
Please include relevant screenshots or files that would be helpful in reviewing and verifying this change.
Checklist
- [x] I have added automated tests that prove my fix is effective or that my feature works
- [x] New and existing unit tests pass locally with my changes
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
Codecov Report
Attention: Patch coverage is 38.75969%
with 158 lines
in your changes are missing coverage. Please review.
Project coverage is 66.81%. Comparing base (
b796613
) to head (a6d3bc4
).
:exclamation: Current head a6d3bc4 differs from pull request most recent head b8f8c39. Consider uploading reports for the commit b8f8c39 to get more accurate results
Additional details and impacted files
@@ Coverage Diff @@
## main #1745 +/- ##
===========================================
- Coverage 91.93% 66.81% -25.12%
===========================================
Files 730 731 +1
Lines 19568 19528 -40
Branches 4647 4640 -7
===========================================
- Hits 17989 13048 -4941
- Misses 1441 5709 +4268
- Partials 138 771 +633
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
/savedProjects
is not a valid URL, it should be /saved-projects
to match every other URL in App.js
.
Instead of fixing the bug in the ticket you added a new feature and left the original buggy code abandoned in the codebase. The Project.js
route already has code with comments that explain how the behavior should work. This is occurring because ?projectId=...
is still in the URL when it switches to the /projects
endpoint. Either track down the bug and fix it, or remove the buggy code.
See CONTRIBUTING.md, all commits have to start with the ticket number.
/savedProjects
is not a valid URL, it should be/saved-projects
to match every other URL inApp.js
.Instead of fixing the bug in the ticket you added a new feature and left the original buggy code abandoned in the codebase. The
Project.js
route already has code with comments that explain how the behavior should work. This is occurring because?projectId=...
is still in the URL when it switches to the/projects
endpoint. Either track down the bug and fix it, or remove the buggy code.See CONTRIBUTING.md, all commits have to start with the ticket number.
All feedback incorporated.
- Removed saved project logic from the project url
- Added logic to correct an issue where a project url with no params could still be navigated to
- Added the back to search to the saved-projects url
- renamed the url endpoint
Update I was looking at this some because of a related ticket I have. It seems like the issue is that we can't really differentiate between an empty project and the saved project page. Its possible to fix the issue where it will go to saved project. The way that we are currently checking makes sense i.e. if there are search parameters then we go to the project page the issue is that somehow the saved projects page is carrying some search parameters when clicking on the saved projects page
Closing PR likely will use a different solution otherwise we'll reference this one but, its gotten quite behind the HEAD