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

Extract user creation and move to backends package #4248

Closed
wants to merge 67 commits into from

Conversation

b1ron
Copy link
Contributor

@b1ron b1ron commented Apr 18, 2024

Description

Closes #4243.

Readiness checklist

  • I added/updated unit tests (and they pass).
  • I added/updated integration/compatibility tests (and they pass).
  • I added/updated comments and checked rendering.
  • I made spot refactorings.
  • I updated user documentation.
  • I ran task all, and it passed.
  • I ensured that PR title is good enough for the changelog.
  • (for maintainers only) I set Reviewers (@FerretDB/core), Milestone (Next), Labels, Project and project's Sprint fields.
  • I marked all done items in this checklist.

@b1ron b1ron added the code/chore Code maintenance improvements label Apr 18, 2024
@b1ron b1ron self-assigned this Apr 18, 2024
@b1ron b1ron added this to the v1.22.0 milestone Apr 18, 2024
Copy link

codecov bot commented Apr 18, 2024

Codecov Report

Attention: Patch coverage is 88.00000% with 15 lines in your changes are missing coverage. Please review.

Project coverage is 74.19%. Comparing base (23d1e94) to head (5e31a54).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4248      +/-   ##
==========================================
- Coverage   74.90%   74.19%   -0.72%     
==========================================
  Files         323      325       +2     
  Lines       22365    22414      +49     
==========================================
- Hits        16752    16629     -123     
- Misses       4385     4575     +190     
+ Partials     1228     1210      -18     
Files Coverage Δ
internal/handler/authenticate.go 76.47% <100.00%> (ø)
internal/util/password/password.go 100.00% <100.00%> (ø)
internal/handler/msg_createuser.go 75.22% <97.56%> (-0.31%) ⬇️
internal/handler/msg_updateuser.go 64.00% <92.00%> (+3.42%) ⬆️
internal/backends/user.go 77.77% <77.77%> (ø)

... and 15 files with indirect coverage changes

Flag Coverage Δ
filter-true 67.48% <88.00%> (-0.79%) ⬇️
hana-1 ?
integration 67.48% <88.00%> (-0.79%) ⬇️
mongodb-1 5.13% <0.00%> (-0.03%) ⬇️
postgresql-1 44.79% <66.40%> (+0.02%) ⬆️
postgresql-2 44.88% <48.00%> (-0.04%) ⬇️
postgresql-3 41.55% <46.40%> (-0.02%) ⬇️
postgresql-4 41.13% <46.40%> (-0.03%) ⬇️
postgresql-5 43.35% <69.60%> (-0.01%) ⬇️
sqlite-1 43.76% <66.40%> (-0.02%) ⬇️
sqlite-2 44.20% <48.00%> (+0.01%) ⬆️
sqlite-3 40.78% <46.40%> (-0.03%) ⬇️
sqlite-4 40.51% <46.40%> (-0.02%) ⬇️
sqlite-5 42.48% <69.60%> (+<0.01%) ⬆️
unit 33.22% <0.00%> (-0.08%) ⬇️

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

@b1ron
Copy link
Contributor Author

b1ron commented Apr 19, 2024

Tentatively sending for review for feedback on current implementation.

Still needs linter fixes and some manual testing.

@b1ron b1ron marked this pull request as ready for review April 19, 2024 13:32
@b1ron b1ron requested review from AlekSi and a team as code owners April 19, 2024 13:32
@b1ron b1ron requested review from rumyantseva, a team, chilagrow and noisersup April 19, 2024 13:32
internal/backends/backend.go Outdated Show resolved Hide resolved
@b1ron b1ron requested a review from chilagrow May 7, 2024 14:51
Copy link
Contributor

@chilagrow chilagrow left a comment

Choose a reason for hiding this comment

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

Tiny comment 🤗

internal/backends/backend.go Outdated Show resolved Hide resolved
internal/handler/msg_createuser.go Outdated Show resolved Hide resolved
@b1ron b1ron requested a review from chilagrow May 8, 2024 07:30
chilagrow
chilagrow previously approved these changes May 8, 2024
Copy link
Contributor

@chilagrow chilagrow left a comment

Choose a reason for hiding this comment

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

Great!

@AlekSi AlekSi added the not ready Issues that are not ready to be worked on; PRs that should skip CI label May 28, 2024
@AlekSi AlekSi assigned AlekSi and unassigned b1ron May 28, 2024
@AlekSi AlekSi disabled auto-merge May 28, 2024 10:36
@AlekSi AlekSi removed the not ready Issues that are not ready to be worked on; PRs that should skip CI label May 28, 2024

// Password wraps a password string.
//
// It exist mainly to avoid issues when multiple string parameters are used.
Copy link
Member

Choose a reason for hiding this comment

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

Another reason is to try to hide values of that type from logs in the future.

That's experimental. I will either update more code to use that type or remove it in future PRs.

@AlekSi AlekSi enabled auto-merge (squash) May 28, 2024 11:25
@AlekSi AlekSi requested review from AlekSi, chilagrow, noisersup and a team May 28, 2024 11:25
@b1ron b1ron closed this by deleting the head repository May 28, 2024
Copy link
Contributor

mergify bot commented May 29, 2024

⚠️ The sha of the head commit of this PR conflicts with #4311. Mergify cannot evaluate rules on this PR. ⚠️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code/chore Code maintenance improvements
Projects
Status: In progress
Development

Successfully merging this pull request may close these issues.

Implement ferretdb --setup flags
5 participants