Skip to content

fix: falsy ids omitted for search params#31

Merged
WilliamKelley merged 1 commit intomainfrom
william/fix-falsy-search-params
Mar 26, 2026
Merged

fix: falsy ids omitted for search params#31
WilliamKelley merged 1 commit intomainfrom
william/fix-falsy-search-params

Conversation

@WilliamKelley
Copy link
Copy Markdown
Contributor

@WilliamKelley WilliamKelley commented Mar 26, 2026

In interview, candidate encountered problem when changing page_size to 1 on the client. Their code was identical to the solution so something was up. Turns out the client api implementation has a bug where falsy parameters weren't serialized to the URL. Since the first project's id is 0, then the offset parameter wasn't transmitted.

Another invisible symptom of this bug is that the first user, "James Smith", would never work with the filter control since their userId was 0. But this was never apparent because they're coded to have zero projects anyways.

This bug is not present in the typescript version of the interview because there's not an actual server or parameter serialization. Using the same language for server and client means that parameters were just passed to the function invocation directly.

In interview, candidate encountered problem when changing `page_size` to `1` on the client. Their code was identical to the solution so something was up. Turns out the client api implementation has a bug where falsy parameters weren't serialized to the URL. Since the first project's `id` is `0`, then the offset parameter wasn't transmitted.

Another invisible symptom of this bug is that the first user, "James Smith", would never work with the filter control since their `userId` was `0`. But this was never apparent because they're coded to have zero projects anyways.
@WilliamKelley WilliamKelley requested a review from cpimhoff March 26, 2026 19:32
@WilliamKelley WilliamKelley merged commit ff4a41d into main Mar 26, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants