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

Refactor profiles in remaining queries referencing Profiles #8480

Merged
merged 18 commits into from
Jul 19, 2024

Conversation

Rotorsoft
Copy link
Contributor

Link to Issue

Closes: #8472

Description of Changes

  • Replaces Profiles with Users.profile JSONB in queries
  • A little bit of code cleanup and optimization
  • Creates query and comment schemas, shared by client and server

Test Plan

@Rotorsoft Rotorsoft requested a review from mzparacha July 16, 2024 15:00
@mzparacha mzparacha requested a review from masvelio July 16, 2024 16:12
Copy link
Collaborator

@timolegros timolegros left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally works/looks good on first pass but need to resolve conflicts and merge master before I can test more deeply the feeds.

@Rotorsoft Rotorsoft requested a review from timolegros July 18, 2024 13:14
@mzparacha
Copy link
Contributor

mzparacha commented Jul 18, 2024

I tried some profile/thread related user flows, found some issues

  1. When creating a thread, the username is always Anonymous, even when profile has username
Screen.Recording.2024-07-18.at.7.09.13.PM.mov
  1. New comments also have Anonymous names
image
  1. Not sure if its planned for this PR, but the edit thread route still returns User.Profiles
image

Will try address related user flows next.

@Rotorsoft
Copy link
Contributor Author

@mzparacha I had to merge other PRs and reconcile the UI model a little bit more to get this working... was planning to do this in the next step, but seems to be impossible in order to meet the full functionality. Please test all the flows again to make sure this is working as expected. Thanks!

@mzparacha
Copy link
Contributor

Per the changes done, I tested these user flows

  1. Thread CRUD
  2. Comments CRUD
  3. Thread/Comment Reactions CRUD
  4. User Profile CRUD
  5. User Addresses CRUD (address linking etc)
  6. Account creation flows (as we updated some passort.js logic)

and found this issue

  1. Can't use the edit thread API to edit/mark or unmark spam/update status/add or remove collaborators and other actions. The API fails with this error
{"status":400,"error":"Must provide body"}

the other things I tested worked for me.

cc: @Rotorsoft

@timolegros
Copy link
Collaborator

timolegros commented Jul 19, 2024

  1. Can't use the edit thread API to edit/mark or unmark spam/update status/add or remove collaborators and other actions. The API fails with this error
{"status":400,"error":"Must provide body"}

@kurtisassad might be caused by #8041? Specifically https://github.com/hicommonwealth/commonwealth/blob/master/packages/commonwealth/server/controllers/server_threads_methods/update_thread.ts#L110.

@Rotorsoft
Copy link
Contributor Author

  1. Can't use the edit thread API to edit/mark or unmark spam/update status/add or remove collaborators and other actions. The API fails with this error
{"status":400,"error":"Must provide body"}

@kurtisassad might be caused by #8041? Specifically https://github.com/hicommonwealth/commonwealth/blob/master/packages/commonwealth/server/controllers/server_threads_methods/update_thread.ts#L110.

@kurtisassad can you check this? ...seems related to the history PR

@kurtisassad kurtisassad force-pushed the rotorsoft/8472-refactor-server-routes-utils-profile branch from 75a9b82 to 60d187e Compare July 19, 2024 18:02
@Rotorsoft Rotorsoft merged commit 3b05f1b into master Jul 19, 2024
9 checks passed
@Rotorsoft Rotorsoft deleted the rotorsoft/8472-refactor-server-routes-utils-profile branch July 19, 2024 19:14
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.

Refactor misc routes and utils to use new Users.profile model instead of Profiles
5 participants