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

Reactions state management with react query #4244

Closed
jnaviask opened this issue Jun 14, 2023 · 4 comments · Fixed by #4426
Closed

Reactions state management with react query #4244

jnaviask opened this issue Jun 14, 2023 · 4 comments · Fixed by #4426
Assignees
Labels
2 Half day task 3 Full day task cleanup

Comments

@jnaviask
Copy link
Collaborator

Description

Follow-up for #4186. We should use react-query to store + mutate the state of reactions, rather than relying on raw HTTP requests + event emitters. See client/scripts/state/api/topics as a reference.

Engineering Requirements

  • Logic for reactions encapsulated with react-query.
  • New state folder for reactions that adds react-specific hooks.
  • Use react-specific hooks for reactions rather than existing business logic.

Acceptance Criteria

No product-facing changes or regressions.

@jnaviask jnaviask added this to Cycle 4 Jun 14, 2023
@jnaviask jnaviask moved this to Teed Up 🏌️ in Cycle 4 Jun 14, 2023
@kurtisassad kurtisassad moved this from Teed Up 🏌️ to In Progress ⛹️‍♂️ in Cycle 4 Jun 14, 2023
@CowMuon
Copy link
Contributor

CowMuon commented Jun 15, 2023

Looks like this is set to in progress but has no point estimate? @kurtisassad
Please see #4258 (literally the first item on the sub-task list.)

@kurtisassad kurtisassad added the 2 Half day task label Jun 15, 2023
@CowMuon CowMuon removed this from Cycle 4 Jul 5, 2023
@CowMuon CowMuon assigned mzparacha and unassigned kurtisassad Jul 5, 2023
@mzparacha
Copy link
Contributor

Did some work and apart from the reaction thread controller we have some API calls of reactions endpoints in the threads and some controllers. These are all the files that have some /reactions related API calls

what I am concerned about is threads.ts. Should we move the reaction-related API calls from threads.ts to the new react query /reactions state/folder for better organization

cc: @jnaviask @CowMuon

@jnaviask
Copy link
Collaborator Author

jnaviask commented Jul 6, 2023

@mzparacha feel free to move the reactions functions around as needed -- if it makes sense to move them from threads.ts, then that's fine (see the old TODO anyway!).

@CowMuon
Copy link
Contributor

CowMuon commented Jul 10, 2023

This is under review and should be mergeable once @masvelio marks his comments as resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 Half day task 3 Full day task cleanup
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants