-
Notifications
You must be signed in to change notification settings - Fork 3.3k
internal: (studio) persist telemetry session ID in URL #32170
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
Changes from 8 commits
06244f9
43f93f1
73f5fa2
968d648
866c723
e1c7828
3b4b0a0
af44616
292fc6f
c514827
cf5994f
a7db5ac
9bafaeb
41db260
d81adb1
5cbf4b2
83db37a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -664,21 +664,21 @@ describe('studio functionality', () => { | |
it('updates the url with the testId and studio parameters when entering studio with a test', () => { | ||
launchStudio() | ||
|
||
cy.location().its('hash').should('contain', 'testId=r3').and('contain', 'studio=') | ||
cy.location().its('hash').should('contain', 'testId=r3').and('contain', 'studio=').and('contain', 'sessionId=') | ||
}) | ||
|
||
it('update the url with the suiteId and studio parameters when entering studio with a suite', () => { | ||
launchStudio({ createNewTestFromSuite: true }) | ||
|
||
cy.location().its('hash').should('contain', 'suiteId=r2').and('contain', 'studio=') | ||
cy.location().its('hash').should('contain', 'suiteId=r2').and('contain', 'studio=').and('contain', 'sessionId=') | ||
}) | ||
|
||
it('updates the studio url parameters and displays the single test view after creating a new test', () => { | ||
loadProjectAndRunSpec() | ||
|
||
// open the studio panel to create a new test in the root suite | ||
cy.findByTestId('studio-button').click() | ||
cy.location().its('hash').should('contain', 'suiteId=r1').and('contain', 'studio=') | ||
cy.location().its('hash').should('contain', 'suiteId=r1').and('contain', 'studio=').and('contain', 'sessionId=') | ||
|
||
// create a new test in the root suite | ||
inputNewTestName() | ||
|
@@ -694,7 +694,7 @@ describe('studio functionality', () => { | |
it('does not remove the studio url parameters when saving test changes', () => { | ||
launchStudio() | ||
|
||
cy.location().its('hash').should('contain', 'testId=r3').and('contain', 'studio=') | ||
cy.location().its('hash').should('contain', 'testId=r3').and('contain', 'studio=').and('contain', 'sessionId=') | ||
cursor[bot] marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: URL Assertion Omission in Studio ModeMultiple Additional Locations (2) |
||
|
||
cy.findByTestId('record-button-recording').should('be.visible') | ||
|
||
|
@@ -706,7 +706,7 @@ describe('studio functionality', () => { | |
|
||
cy.findByTestId('studio-save-button').click() | ||
|
||
cy.location().its('hash').should('contain', 'testId=r3').and('contain', 'studio=') | ||
cy.location().its('hash').should('contain', 'testId=r3').and('contain', 'studio=').and('contain', 'sessionId=') | ||
}) | ||
|
||
it('does not remove the studio url parameters if saving fails', () => { | ||
|
@@ -716,7 +716,7 @@ describe('studio functionality', () => { | |
|
||
incrementCounter(0) | ||
|
||
cy.location().its('hash').should('contain', 'testId=r3').and('contain', 'studio=') | ||
cy.location().its('hash').should('contain', 'testId=r3').and('contain', 'studio=').and('contain', 'sessionId=') | ||
|
||
// update the spec on the file system by changing the | ||
// test name which will cause the save to fail since | ||
|
@@ -757,7 +757,7 @@ describe('studio functionality', () => { | |
|
||
cy.findByTestId('studio-header-studio-button').click() | ||
|
||
cy.location().its('hash').and('not.contain', 'testId=').and('not.contain', 'studio=') | ||
cy.location().its('hash').and('not.contain', 'testId=').and('not.contain', 'studio=').and('not.contain', 'sessionId=') | ||
}) | ||
|
||
it('does not prompt for a URL until studio is active', () => { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -108,13 +108,30 @@ interface StudioRecorderState { | |
canAccessStudioAI: boolean | ||
showUrlPrompt: boolean | ||
cloudStudioRequested: boolean | ||
cloudStudioSessionId?: string | ||
sessionId?: string | ||
_isStudioCreatedTest: boolean | ||
newTestLineNumber?: number | ||
} | ||
|
||
function getUrlParams () { | ||
const url = new URL(window.location.href) | ||
const hashParams = new URLSearchParams(url.hash) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Hash Parsing Error in URLSearchParamsThe Additional Locations (1) |
||
|
||
const testId = hashParams.get('testId') | ||
const suiteId = hashParams.get('suiteId') | ||
const visitUrl = hashParams.get('url') | ||
const newTestLineNumber = hashParams.get('newTestLineNumber') ? Number(hashParams.get('newTestLineNumber')) : undefined | ||
const cloudStudioSessionId = hashParams.get('sessionId') | ||
|
||
return { testId, suiteId, url: visitUrl, newTestLineNumber, cloudStudioSessionId } | ||
astone123 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
export const useStudioStore = defineStore('studioRecorder', { | ||
state: (): StudioRecorderState => { | ||
// try to restore cloudStudioSessionId from URL parameters | ||
const urlParams = getUrlParams() | ||
const persistedSessionId = urlParams.cloudStudioSessionId || undefined | ||
astone123 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
return { | ||
saveModalIsOpen: false, | ||
instructionModalIsOpen: false, | ||
|
@@ -128,7 +145,7 @@ export const useStudioStore = defineStore('studioRecorder', { | |
canAccessStudioAI: false, | ||
showUrlPrompt: true, | ||
cloudStudioRequested: false, | ||
cloudStudioSessionId: undefined, | ||
sessionId: persistedSessionId, | ||
newTestLineNumber: undefined, | ||
_isStudioCreatedTest: false, | ||
} | ||
|
@@ -161,7 +178,13 @@ export const useStudioStore = defineStore('studioRecorder', { | |
}, | ||
|
||
setCloudStudioSessionId (cloudStudioSessionId: string) { | ||
astone123 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
this.cloudStudioSessionId = cloudStudioSessionId | ||
this.sessionId = cloudStudioSessionId | ||
astone123 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
this._updateUrlParams(['sessionId']) | ||
}, | ||
|
||
clearCloudStudioSessionId () { | ||
astone123 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
this.sessionId = undefined | ||
this._removeUrlParams(['sessionId']) | ||
}, | ||
|
||
setNewTestLineNumber (newTestLineNumber: number) { | ||
|
@@ -213,7 +236,7 @@ export const useStudioStore = defineStore('studioRecorder', { | |
}, | ||
|
||
setup (config) { | ||
const studio = this._getUrlParams() | ||
const studio = this.getUrlParams() | ||
|
||
if (studio.newTestLineNumber) { | ||
this.setNewTestLineNumber(studio.newTestLineNumber) | ||
|
@@ -227,6 +250,10 @@ export const useStudioStore = defineStore('studioRecorder', { | |
this._initialUrl = studio.url | ||
} | ||
|
||
if (studio.cloudStudioSessionId) { | ||
this.sessionId = studio.cloudStudioSessionId | ||
astone123 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Inconsistent URL Parameter HandlingThe |
||
|
||
// if we have an existing test or are creating a new test, we need to start loading | ||
// otherwise if we have a suite, we can just set the studio active | ||
if (this.testId || studio.newTestLineNumber) { | ||
|
@@ -308,6 +335,7 @@ export const useStudioStore = defineStore('studioRecorder', { | |
this.clearRunnableIds() | ||
this._removeUrlParams() | ||
this._initialUrl = undefined | ||
this.clearCloudStudioSessionId() | ||
astone123 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}, | ||
|
||
startSave () { | ||
|
@@ -436,21 +464,11 @@ export const useStudioStore = defineStore('studioRecorder', { | |
} | ||
}, | ||
|
||
_getUrlParams () { | ||
const url = new URL(window.location.href) | ||
const hashParams = new URLSearchParams(url.hash) | ||
|
||
const testId = hashParams.get('testId') | ||
const suiteId = hashParams.get('suiteId') | ||
const visitUrl = hashParams.get('url') | ||
const newTestLineNumber = hashParams.get('newTestLineNumber') ? Number(hashParams.get('newTestLineNumber')) : undefined | ||
|
||
return { testId, suiteId, url: visitUrl, newTestLineNumber } | ||
}, | ||
getUrlParams, | ||
|
||
_updateUrlParams (filter: string[] = ['testId', 'suiteId', 'url', 'newTestLineNumber']) { | ||
_updateUrlParams (filter: string[] = ['testId', 'suiteId', 'url', 'newTestLineNumber', 'sessionId']) { | ||
// if we don't have studio params, we don't need to update them | ||
if (!this.testId && !this.suiteId && !this.url && !this.newTestLineNumber) return | ||
if (!this.testId && !this.suiteId && !this.url && !this.newTestLineNumber && !this.sessionId) return | ||
|
||
// if we have studio params, we need to remove them before adding them back | ||
this._removeUrlParams(filter) | ||
|
@@ -469,7 +487,7 @@ export const useStudioStore = defineStore('studioRecorder', { | |
window.history.replaceState({}, '', url.toString()) | ||
}, | ||
|
||
_removeUrlParams (filter: string[] = ['testId', 'suiteId', 'url', 'newTestLineNumber']) { | ||
_removeUrlParams (filter: string[] = ['testId', 'suiteId', 'url', 'newTestLineNumber', 'sessionId']) { | ||
cursor[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
const url = new URL(window.location.href) | ||
const hashParams = new URLSearchParams(url.hash) | ||
|
||
|
@@ -482,7 +500,7 @@ export const useStudioStore = defineStore('studioRecorder', { | |
}) | ||
|
||
// if there are no studio specific params left, we can also remove the studio param | ||
if (!hashParams.has('testId') && !hashParams.has('suiteId') && !hashParams.has('url') && !hashParams.has('newTestLineNumber')) { | ||
if (!hashParams.has('testId') && !hashParams.has('suiteId') && !hashParams.has('url') && !hashParams.has('newTestLineNumber') && !hashParams.has('sessionId')) { | ||
hashParams.delete('studio') | ||
} | ||
|
||
|
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.
Suggest updating the url tests in
studio.cy.ts
to also test for thesessionId
.