-
Notifications
You must be signed in to change notification settings - Fork 35
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
chore: add tests for query params #430
base: main
Are you sure you want to change the base?
Changes from 1 commit
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 |
---|---|---|
|
@@ -114,16 +114,17 @@ export function configLangfuseSDK(params?: LangfuseCoreOptions, secretRequired: | |
} | ||
|
||
export const encodeQueryParams = (params?: { [key: string]: any }): string => { | ||
const queryParams = new URLSearchParams(); | ||
Object.entries(params ?? {}).forEach(([key, value]) => { | ||
if (value !== undefined && value !== null) { | ||
// check for date | ||
if (value instanceof Date) { | ||
queryParams.append(key, value.toISOString()); | ||
} else { | ||
queryParams.append(key, value.toString()); | ||
const queryParams = Object.entries(params ?? {}) | ||
.map(([key, value]) => { | ||
if (value !== undefined && value !== null) { | ||
const encodedKey = encodeURIComponent(key); | ||
const encodedValue = | ||
value instanceof Date ? encodeURIComponent(value.toISOString()) : encodeURIComponent(value.toString()); | ||
return `${encodedKey}=${encodedValue}`; | ||
Comment on lines
+122
to
+123
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. logic: This doesn't handle array values correctly. Arrays will be converted to strings, potentially losing data. Consider adding specific array handling. |
||
} | ||
} | ||
}); | ||
return queryParams.toString(); | ||
return null; | ||
}) | ||
.filter((param) => param !== null) | ||
.join("&"); | ||
return queryParams; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,7 @@ import { | |
currentISOTime, | ||
currentTimestamp, | ||
configLangfuseSDK, | ||
encodeQueryParams, | ||
} from "../src/utils"; | ||
|
||
describe("utils", () => { | ||
|
@@ -42,6 +43,38 @@ describe("utils", () => { | |
expect(currentTimestamp()).toEqual(Date.now()); | ||
}); | ||
}); | ||
describe("encodeQueryParams", () => { | ||
it("should encode query parameters correctly", () => { | ||
const params = { | ||
name: "John Doe", | ||
age: 30, | ||
active: true, | ||
date: new Date("2022-01-01T00:00:00.000Z"), | ||
empty: null, | ||
undefinedValue: undefined, | ||
}; | ||
const expected = "name=John%20Doe&age=30&active=true&date=2022-01-01T00%3A00%3A00.000Z"; | ||
expect(encodeQueryParams(params)).toEqual(expected); | ||
}); | ||
Comment on lines
+46
to
+58
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. logic: Test case doesn't check handling of null and undefined values |
||
|
||
it("should encode query params with arrays correctly", () => { | ||
const params = { | ||
tags: ["tag-one", "tag-two"], | ||
}; | ||
const expected = "tags=tag-one%2Ctag-two"; | ||
expect(encodeQueryParams(params)).toEqual(expected); | ||
}); | ||
|
||
it("should handle special characters in keys and values", () => { | ||
const params = { | ||
"key with spaces": "value/with/slash", | ||
"key&with&special": "value?with=query", | ||
}; | ||
const expected = "key%20with%20spaces=value%2Fwith%2Fslash&key%26with%26special=value%3Fwith%3Dquery"; | ||
expect(encodeQueryParams(params)).toEqual(expected); | ||
}); | ||
}); | ||
|
||
describe("currentISOTime", () => { | ||
it("should get the iso time", () => { | ||
jest.setSystemTime(new Date("2022-01-01")); | ||
|
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.
Remove
it.only
to ensure all tests run.