Use vue-query for offers #2560
Use vue-query for offers #2560
Conversation
To avoid having Object|Number prop for user... and ProfilePicture no longer handles retreiving the user
Codecov Report
@@ Coverage Diff @@
## master #2560 +/- ##
==========================================
- Coverage 52.47% 51.04% -1.43%
==========================================
Files 181 193 +12
Lines 4877 5244 +367
Branches 822 912 +90
==========================================
+ Hits 2559 2677 +118
- Misses 2318 2567 +249
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Nice, this is a serious change in direction for frontend data handling!
I wonder where to draw the line between local state (to be handled with vue-query) and global state, e.g. currentOffer seems to be somehow global as it also affects the topbar. Is it reasonable to keep this kind of state in vuex (maybe later pinia) instead of the topbar/breadcrumb component having to know about all kinds of queries across Karrot?
| import { useCurrentGroupIdRef } from '@/group/queries' | ||
|
|
||
| // Store the ids we are currently mutating, so we can better decide when to refresh from websocket | ||
| const mutatingOfferIds = new Set() |
There was a problem hiding this comment.
Not sure I fully understand - I guess it shouldn't invalidate the offer while the user is editing it? Or is it about some data race when clicking the "save"/"create" button?
There was a problem hiding this comment.
It's to avoid extra unneeded queries when receiving websocket updates. Without this logic the websocket update will invalidate the offers query and reload the data, and then the response to the mutation request will also do that, and two requests to the backend will go out. I think some situations like that would get debounced, but in this case it would make two separate requests.
Another option is to rely on the websocket update alone, but I tend to not like to rely on that approach, although I think currently we do in places and not others (depending on who wrote the code :), so maybe we should actually decide on a policy there).
I also looked at just updating the specific offer that is saved/edited/archived in the query (using setQueryData), but for the query it's not always obvious how a given edit might impact all the queries (e.g. when you archive it, it needs to be added to the status=archived query and removed from the status=active one), so I think just invalidating the lot is better.
a composable ( my sense is that a lot of the global data (from server) could be put into vue-query... maybe there is still some need for global local state (i.e. not from serve), if it's only a little bit maybe just standalone
at the moment something has to know how to get all the breadcrumb values from somewhere, currently in the breadcrumb datastore... another idea could be that page-level components set the breadcrumbs, so there stops being a single place that has to pull together all the sources, I think that might work quite nicely. |
Switch to using vue-query for offers, as a first scenario of using it for other modules.
Notable points:
<ProfilePicture>now accepts a user object or just a user id (so we can use non-enriched offer objects)getUserRefutility that can be used<script setup>- see offers/pages/*.vue files...useCurrentGroupId()thingTODO:
statusobject, but I only implemented thevalidationErrorsbit of it... need to decide whether to map the vue-query status to our existing form, or switch the code to use vue-query-style status... DONE: I mapped it to our existing status object, which was quite seamlessuseCurrentGroupIdfunction togroup/queries.jsuseCurrentUserId->useCurrentUserIdRefoffers/deletestore commit fromboot/socket.jsLinks to related issues
Checklist