Add support for SLAS DNT (do not track) parameter #167
Conversation
| @@ -47,6 +47,7 @@ const parameters = { | |||
| refreshToken: 'refresh_token', | |||
| usid: 'usid', | |||
| hint: 'hint', | |||
| dnt: 'true', | |||
There was a problem hiding this comment.
well, the API spec says this parameter is string.. which is kinda weird to me, shouldn't it be a boolean? I'm gonna ask
There was a problem hiding this comment.
Agreed that it is a bit weird, but if it changes in the RAML spec from type string to type boolean it'll be a breaking change :(
There was a problem hiding this comment.
Good call. I'm going to make the SLAS helpers to accepts booleans and do a .toString() to convert it before calling the API client.
| @@ -166,6 +166,7 @@ export async function authorize( | |||
| * @param credentials.clientSecret - secret associated with client ID | |||
| * @param parameters - parameters to pass in the API calls. | |||
| * @param parameters.usid? - Unique Shopper Identifier to enable personalization. | |||
| * @param parameters.dnt? - Optional parameter to enable Do Not Track (DNT) for the user. | |||
There was a problem hiding this comment.
Since it is boolean, does it make sense to set default value instead of making the parameter optional?dnt=true
There was a problem hiding this comment.
the default is controlled at the API level, if this parameter is not present, SLAS default to dnt=false.
There was a problem hiding this comment.
One thing we should be weary of is if this dnt field will be subject to change in the future. If it gets updated to accept other string values besides 'true' and 'false' this implementation will break
If we expect this to be updated in the future we should probably revert to the previous implementation. Given the nature of the field I can't imagine any other options, but better to be safe than sorry, let's check with the SLAS team first if we want to use this approach
This PR adds a new optional parameter
dntto SLAS helpers:loginGuestUserPrivate,loginGuestUserandloginRegisteredUserB2C