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

EDSC-3923: Add clientId to EDD startDownload link #1696

Merged
merged 9 commits into from
Dec 1, 2023

Conversation

rushgeo
Copy link
Contributor

@rushgeo rushgeo commented Nov 22, 2023

Overview

What is the feature?

Add the clientId parameter to the startDownload link to track which EDSC environment initializes the download

What is the Solution?

Use the env property from the Application config for the clientId parameter and place it in the EDD URL.

What areas of the application does this impact?

EDD link generation.

Testing

  • Select some granules to download
  • Choose the Direct Download access method
  • Click the Download Data button
  • Once the file URLs populate in the status window, click the Download Files button
  • Close the Earthdata Download application if it opens, or refuse your browser's request to open it
  • Right-click on the Open Earthdata Download button, and copy the URL
  • Paste the URL somewhere, and verify that the it contains &clientId=eed-edsc-dev-serverless-client after the mocked authToken

image

Checklist

  • I have added automated tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings

Copy link

codecov bot commented Nov 22, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (ed874ef) 91.87% compared to head (8b43fb5) 91.94%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1696      +/-   ##
==========================================
+ Coverage   91.87%   91.94%   +0.06%     
==========================================
  Files         725      725              
  Lines       19349    19349              
  Branches     4562     4562              
==========================================
+ Hits        17777    17790      +13     
+ Misses       1436     1423      -13     
  Partials      136      136              

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

@rushgeo rushgeo marked this pull request as ready for review November 24, 2023 13:53
@macrouch
Copy link
Contributor

The clientId needs to be more specific, just using the env doesn't tell us EDSC sent the user to EDD. I'd look into the clientIds that we send to CMR and see if we can reuse anything from that logic/config

@rushgeo
Copy link
Contributor Author

rushgeo commented Nov 29, 2023

The latest commit fixes the tests by using the correct defaultPortal value set in static.config.json. I was trying to use mocked functions to override the values used by getClientId(), which would be helpful if there was also a overrideStatic.config.json file present. That ended up not being worth the time investment, though, so I went with the simpler approach.

@rushgeo
Copy link
Contributor Author

rushgeo commented Nov 30, 2023

Mocking getClientId after all with the latest commit instead of relying on absence of overrideStatic.config.json on developer machines.

@rushgeo rushgeo merged commit 2b6b38b into nasa:main Dec 1, 2023
9 checks passed
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.

4 participants