earthdata-search icon indicating copy to clipboard operation
earthdata-search copied to clipboard

EDSC-4031: Always route to saved Projects when selected.

Open Bccorb opened this issue 9 months ago • 4 comments

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:

  1. Logging in to EDSC
  2. Selecting the save project icon in the right hand tool bar
  3. Entering a project name
  4. Adding a collection to the project via the plus button on a collection
  5. Clicking the user icon in the right hand tool bar and selecting "saved projects"
  6. 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

Bccorb avatar May 07 '24 03:05 Bccorb

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

Files Patch % Lines
...tic/src/js/components/AccessMethod/AccessMethod.js 38.24% 147 Missing and 8 partials :warning:
...tatic/src/js/routes/SavedProjects/SavedProjects.js 33.33% 2 Missing :warning:
static/src/js/components/EDSCIcon/EDSCIcon.js 50.00% 1 Missing :warning:
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.

codecov-commenter avatar May 08 '24 01:05 codecov-commenter

/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.

macrouch avatar May 08 '24 13:05 macrouch

/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.

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

Bccorb avatar May 08 '24 15:05 Bccorb

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

eudoroolivares2016 avatar Jun 08 '24 21:06 eudoroolivares2016

Closing PR likely will use a different solution otherwise we'll reference this one but, its gotten quite behind the HEAD

eudoroolivares2016 avatar Aug 13 '24 14:08 eudoroolivares2016