-
Notifications
You must be signed in to change notification settings - Fork 4
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
[CM-1090] Allow resolving idcookie #304
Conversation
this.timeout | ||
) | ||
private buildUrl(params: [string, string][]): string { | ||
return `${this.url}/${this.source}/${this.publisherId}${toParams(this.filterParams(params))}` | ||
} | ||
|
||
getUrl(additionalParams: Record<string, string | string[]>): string { |
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.
Any idea why we expose the resolutionCallUrl
in LiveConnect at all getUrl
is used to compute resolutionCallUrl
?
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.
No idea. As you say, it's just an alias for getUrl resolutionCallUrl: resolver.getUrl.bind(resolver),
it('should resolve the idcookie when requested via additional attributes', (done) => { | ||
const value = 'foo' | ||
const identityResolver = new IdentityResolver({ resolvedIdCookie: value }, calls) | ||
const successCallback = (responseAsJson) => { | ||
expect(requestToComplete.url).to.eq('https://idx.liadm.com/idex/unknown/any') | ||
expect(errors).to.be.empty() | ||
expect(responseAsJson).to.be.eql({ idcookie: value }) | ||
done() | ||
} | ||
identityResolver.resolve(successCallback, () => {}, { resolve: ['idcookie'] }) | ||
requestToComplete.respond(200, { 'Content-Type': 'application/json' }, JSON.stringify({})) |
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.
👍
return state => { | ||
let resolvedIdCookie: string | null | ||
|
||
if (state.idCookie?.mode === 'provided' && state.idCookie?.strategy === 'cookie' && typeof state.idCookie?.name === 'string') { |
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.
We can make 'generated', 'provided', 'cookie', 'localStorage'
constants and use them, WDYT
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.
Not really any benefit when using typescript. It is already typed as "provided" | "generated"
and will complain if you try to compare it to any other string
CM-1090
Author Todo List:
Ready For Review
status