Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -65,12 +66,15 @@ | |||
| function onSignIn(signup?: any) { | |||
There was a problem hiding this comment.
[maintainability]
The signup parameter is typed as any, which can lead to potential type-related issues. Consider using a more specific type, such as boolean | undefined, to improve type safety and maintainability.
| ].filter(Boolean).join('&') | ||
|
|
||
| // Append UTM parameters from cookie if they exist | ||
| signupUrl = appendUtmParamsToUrl(signupUrl); |
There was a problem hiding this comment.
[❗❗ correctness]
Ensure that the appendUtmParamsToUrl function correctly handles URLs that already contain query parameters, to avoid malformed URLs. This is crucial for maintaining the correctness of the URL redirection.
| const params: UtmParams = {}; | ||
|
|
||
| try { | ||
| const searchParams = new URLSearchParams(window.location.search); |
There was a problem hiding this comment.
[design]
Using window.location directly ties this function to a browser environment, which may limit testing or reuse in non-browser contexts. Consider passing the URL as a parameter to increase flexibility.
| domain = getCookieDomain(), | ||
| path = COOKIE_PATH, | ||
| sameSite = COOKIE_SAMESITE, | ||
| secure = true, |
There was a problem hiding this comment.
[security]
The secure option is hardcoded to true, which means this code will not work over HTTP. Consider making this configurable or documenting that this function requires HTTPS.
| */ | ||
| export function getUtmCookie(): UtmParams | null { | ||
| try { | ||
| const cookies = document.cookie.split(';'); |
There was a problem hiding this comment.
[💡 correctness]
Splitting document.cookie by ; assumes that cookies do not contain semicolons, which is generally safe but not guaranteed. Consider using a more robust cookie parsing library if this becomes an issue.
| } | ||
|
|
||
| try { | ||
| const urlObj = new URL(url, window.location.origin); |
There was a problem hiding this comment.
[design]
Using window.location.origin assumes the code is running in a browser environment. Consider passing the base URL as a parameter to make this function more versatile.
| * @param input - The string to sanitize | ||
| * @returns Sanitized string | ||
| */ | ||
| export declare function sanitize(input: string): string; |
There was a problem hiding this comment.
[correctness]
Consider specifying the behavior of the sanitize function when the input contains characters that are not A-Z, a-z, 0-9, hyphen (-), or underscore (_). This will improve the clarity of the function's contract.
| * Extracts UTM parameters from URL, sanitizes them, and persists to cookie | ||
| * Only sets the cookie if it doesn't already exist | ||
| */ | ||
| export declare function initializeUtmCookieHandler(): void; |
There was a problem hiding this comment.
[correctness]
The initializeUtmCookieHandler function should specify the expected behavior if the URL does not contain any UTM parameters. This will help ensure consistent handling of edge cases.
| * @param url - The base URL to append parameters to | ||
| * @returns URL with UTM parameters appended, or original URL if no cookie exists | ||
| */ | ||
| export declare function appendUtmParamsToUrl(url: string): string; |
There was a problem hiding this comment.
[correctness]
Clarify the behavior of appendUtmParamsToUrl when the url parameter already contains UTM parameters. This will help avoid potential conflicts or duplication of parameters.
PM-3267 - nasa referral
…a-referral Revert "PM-3267 - nasa referral"
…PM-3267_nasa-referral Revert "Revert "PM-3267 - nasa referral""
| url: getMarketingUrl('/talent'), | ||
| }, | ||
| referralProgram: { | ||
| label: 'Referral Programme', |
There was a problem hiding this comment.
[💡 readability]
Consider using consistent spelling for 'Programme' as 'Program' to match the rest of the codebase and ensure uniformity across the application.
2 participants
https://topcoder.atlassian.net/browse/PM-3201
https://topcoder.atlassian.net/browse/PM-3267