-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat: add-optional-query-params-to-start #7
Conversation
🦋 Changeset detectedLatest commit: 6ce29c4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
View your CI Pipeline Execution ↗ for commit 6ce29c4.
☁️ Nx Cloud last updated this comment at |
07c344f
to
6959be7
Compare
clientId: '724ec718-c41c-4d51-98b0-84a583f450f9', | ||
redirectUri: window.location.href, | ||
redirectUri: window.location.origin + '/', |
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.
because i added query params to the testing, i just appended the '/' since it's stripped when using origin
.
@@ -178,7 +179,21 @@ const config = { | |||
console.log('Event emitted from store:', node); | |||
}); | |||
|
|||
const node = await davinciClient.start(); | |||
const qs = window.location.search; |
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.
logic so that we can pull query params dynamically in tests
e2e/davinci-app/tsconfig.app.json
Outdated
"exclude": ["src/**/*.spec.ts", "src/**/*.test.ts"], | ||
"include": ["src/**/*.ts"] | ||
"exclude": ["**/*.spec.ts", "**/*.test.ts"], | ||
"include": ["./main.ts", "components/**/*.ts"] |
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.
was causing issues with module resolution not using Bundler since there is no src
dir here
@@ -1,6 +1,7 @@ | |||
{ | |||
"extends": "./tsconfig.json", | |||
"compilerOptions": { | |||
"moduleResolution": "NodeNext", |
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.
i think this was me debugging but should be a noop since this affects test files.
@@ -26,3 +26,45 @@ test('Test happy paths on test page', async ({ page }) => { | |||
const accessToken = await page.locator('#accessTokenValue').innerText(); | |||
await expect(accessToken).toBeTruthy(); | |||
}); | |||
test('ensure query params passed to start are sent off in authorize call', async ({ page }) => { |
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.
copy pasted above test, passed in a query param and then also added a few other query params from authorize specifically to make sure thats what we are testing
6959be7
to
239d016
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7 +/- ##
==========================================
+ Coverage 45.73% 54.12% +8.39%
==========================================
Files 18 13 -5
Lines 2003 1637 -366
Branches 123 111 -12
==========================================
- Hits 916 886 -30
+ Misses 1087 751 -336 ☔ View full report in Codecov by Sentry. |
239d016
to
dc96747
Compare
adds ability to add query parameters to the start call
dc96747
to
6ce29c4
Compare
JIRA Ticket
https://pingidentity.atlassian.net/browse/SDKS-3647
Description
adds ability to add query parameters to the start call