Skip to content
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

portalicious: fix request caching #6007

Merged
merged 1 commit into from
Oct 31, 2024
Merged

Conversation

aberonni
Copy link
Contributor

@aberonni aberonni commented Oct 31, 2024

AB#31036

Describe your changes

On some requests in the export functionality, the same request + http parameters were being sent to the backend, even when the http parameters being used to make the request in the service were different.

I started by changing the generateQueryOptions function to have an exhaustive list of query keys, thinking that the lack of requestOptions in this array was the culprit.

It was not.

So then I refactored the HttpWrapper.performRequest to only accept HttpParam objects instead of HttpParam class instances. My thinking here was that tanstack-query was struggling to serialize class instances as opposed to objects.

This was partially true, but did not solve the problem entirely.

I ultimately added the paginateQuery option to the generateQueryOptions, because I realised that the issue was that the call to the Signal paginateQuery needed to happen inside the queryFn in order to allow the "signals to signal".

I'm not sure any of my thought process actually makes any sense, but hopefully you find the resulting code cleaner either way, given that now we don't have to worry about creating HttpParams anywhere anymore, and we just deal with objects.

Testing

There is nothing to test in the UI - as in, everything should work like before because the original buggy behaviour was only encountered in #6000

Checklist before requesting a review

  • I have performed a self-review of my code
  • I have added tests wherever relevant
  • I have made sure that all automated checks pass before requesting a review
  • I do not need any deviation from our PR guidelines

AB#31036

On some requests in the export functionality, the same request + http parameters were being sent to the backend, even when the http parameters being used to make the request in the service were different.

I started by changing the `generateQueryOptions` function to have an exhaustive list of query keys, thinking that the lack of `requestOptions` in this array was the culprit.

It was not.

So then I refactored the `HttpWrapper.performRequest` to only accept HttpParam _objects_ instead of HttpParam _class instances_. My thinking here was that tanstack-query was struggling to serialize class instances as opposed to objects.

This was partially true, but did not solve the problem entirely.

I ultimately added the `paginateQuery` option to the `generateQueryOptions`, because I realised that the issue was that the call to the Signal `paginateQuery` needed to happen _inside_ the `queryFn` in order to allow the "signals to signal".

I'm not sure any of my thought process actually makes any sense, but hopefully you find the resulting code cleaner either way, given that now we don't have to worry about creating HttpParams anywhere anymore, and we just deal with objects.
@aberonni aberonni merged commit 7a81043 into main Oct 31, 2024
5 checks passed
@aberonni aberonni deleted the aberonni/portalicious-caching branch October 31, 2024 11:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
portalicious Changes related to the portalicious release
Development

Successfully merging this pull request may close these issues.

2 participants