Skip to content

chore: Use updater in saveUser file #32993

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

Merged
merged 12 commits into from
Jan 2, 2025
Merged

Conversation

matheusbsilva137
Copy link
Contributor

@matheusbsilva137 matheusbsilva137 commented Aug 6, 2024

Proposed changes (including videos or screenshots)

  • Use our updater in the saveUser file to avoid multiple DB trips;
  • Fix IUserSettings type to have an optional profile, as set through the API;
  • Support settings and language params in the users.update endpoint (those were already being handled by the saveUser function, but were not accepted by the endpoint -- so they could only be updated using the Meteor method);
  • Support settings param in the users.create endpoint (which was already being handled by the saveUser function, but was not accepted by the endpoint -- so it could only be created using the Meteor method).

Issue(s)

Steps to test or reproduce

Further comments

CORE-734

Copy link
Contributor

dionisio-bot bot commented Aug 6, 2024

Looks like this PR is not ready to merge, because of the following issues:

  • This PR is targeting the wrong base branch. It should target 7.3.0, but it targets 7.2.0

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

Copy link

changeset-bot bot commented Aug 6, 2024

⚠️ No Changeset found

Latest commit: 3245d24

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link

codecov bot commented Aug 6, 2024

Codecov Report

Attention: Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 59.19%. Comparing base (5506c40) to head (3245d24).
Report is 1 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##           develop   #32993   +/-   ##
========================================
  Coverage    59.19%   59.19%           
========================================
  Files         2821     2821           
  Lines        67633    67633           
  Branches     15048    15048           
========================================
  Hits         40033    40033           
  Misses       24784    24784           
  Partials      2816     2816           
Flag Coverage Δ
unit 74.99% <75.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

@matheusbsilva137 matheusbsilva137 marked this pull request as ready for review August 6, 2024 23:46
@matheusbsilva137 matheusbsilva137 requested review from a team as code owners August 6, 2024 23:46
@MarcosSpessatto MarcosSpessatto added this to the 6.12 milestone Aug 8, 2024
@KevLehman
Copy link
Member

@matheusbsilva137 the task linked doesn't seem to relate that much with what you're doing here. Maybe this needs its own task?

@matheusbsilva137 matheusbsilva137 removed this from the 6.12 milestone Aug 21, 2024
@matheusbsilva137 matheusbsilva137 changed the title chore: Convert saveUser file to TS chore: Use updater in saveUser file Dec 27, 2024
Copy link
Contributor

github-actions bot commented Dec 27, 2024

PR Preview Action v1.4.8
🚀 Deployed preview to https://RocketChat.github.io/Rocket.Chat/pr-preview/pr-32993/
on branch gh-pages at 2025-01-02 19:46 UTC

@matheusbsilva137
Copy link
Contributor Author

matheusbsilva137 commented Dec 27, 2024

We converted saveUser file to TS in another PR (#33774). However, I decided to keep my PR so that we don't lose any of the typing refinements suggested here that weren't applied to the conversion PR.
Instead of converting the file to TS, this PR now aims to improve type-checking in this file, while using our new updater :)

Feel free to review it again, everyone! (Please check the PR description for details on what I'm changing in this PR)

@MarcosSpessatto MarcosSpessatto added this to the 7.3.0 milestone Dec 30, 2024
@matheusbsilva137 matheusbsilva137 added the stat: QA assured Means it has been tested and approved by a company insider label Jan 2, 2025
@dionisio-bot dionisio-bot bot added the stat: ready to merge PR tested and approved waiting for merge label Jan 2, 2025
@kodiakhq kodiakhq bot merged commit b04342c into develop Jan 2, 2025
49 checks passed
@kodiakhq kodiakhq bot deleted the chore/save-user-ts-conversion branch January 2, 2025 21:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stat: QA assured Means it has been tested and approved by a company insider stat: ready to merge PR tested and approved waiting for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants