-
Notifications
You must be signed in to change notification settings - Fork 119
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
Fixes for federation and shortcutting federations #596
Conversation
Codecov Report
@@ Coverage Diff @@
## master #596 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 95 96 +1
Lines 1407 1443 +36
Branches 248 253 +5
=====================================
+ Hits 1407 1443 +36
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work @patrickarlt!
Good thinking to break the logic into helper functions w/ that can be tested w/o fetch-mock. That should make long-term maintenance much easier. I also appreciate the new fetch-mock tests for the the use cases described in the bug.
/** | ||
* Used to test if a URL is production ArcGIS Online Portal | ||
*/ | ||
const arcgisOnlinePortalRegex = /^https?:\/\/(dev|devext|qa|qaext|www)\.arcgis\.com\/sharing\/rest+/; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are dev
and qa
really valid here?
/** | ||
* Used to test if a URL is an ArcGIS Online Organization Portal | ||
*/ | ||
const arcgisOnlineOrgPortalRegex = /^https?:\/\/(?:[a-z0-9-]+\.maps(dev|devext|qa|qaext)?)?.\arcgis\.com\/sharing\/rest/; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are dev
and qa
really valid here too?
return "qa"; | ||
} | ||
|
||
return "production"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Up to you, but I suggest using "prod" for consistency w/ "dev" and "qa"
Once this is merged, I'm glad to handle the release, just LMK. |
Fixes #583 by adding new utility functions (with a heavy amount of testing) for determining if we can short cut federation for all AGOL environments. This also performs additional normalization on URLs when checking if a given server and portal are federated to allow for possibly mismatched https/http in URLs and or some URLs including or not included a trailing slash.