Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

Fix database inconsistent when admin change user email #17549

Merged
merged 10 commits into from
Nov 26, 2021
Merged

Fix database inconsistent when admin change user email #17549

merged 10 commits into from
Nov 26, 2021

Conversation

lunny
Copy link
Member

@lunny lunny commented Nov 4, 2021

Fix #17519

@lunny lunny added type/bug skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. labels Nov 4, 2021
zeripath
zeripath reviewed Nov 5, 2021
View reviewed changes
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Nov 5, 2021
@lunny lunny force-pushed the lunny/fix_admin_change_email branch from 3025b67 to ec62efb Compare November 5, 2021 06:14
@codecov-commenter
Copy link

codecov-commenter commented Nov 5, 2021 •
edited
Loading

Codecov Report

Merging #17549 (5e3d0d0) into main (af901a4) will decrease coverage by 0.00%.
The diff coverage is 39.21%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #17549      +/-   ##
==========================================
- Coverage   45.56%   45.55%   -0.01%     
==========================================
  Files         808      808              
  Lines       90010    90049      +39     
==========================================
+ Hits        41009    41021      +12     
- Misses      42453    42478      +25     
- Partials     6548     6550       +2
Impacted Files Coverage Δ
routers/api/v1/user/settings.go 0.00% <0.00%> (ø)
routers/web/org/org_labels.go 0.00% <0.00%> (ø)
routers/web/org/setting.go 0.00% <0.00%> (ø)
routers/web/admin/users.go 40.41% <16.66%> (-0.37%) ⬇️
routers/api/v1/admin/user.go 23.78% <33.33%> (-0.24%) ⬇️
models/user/user.go 61.95% <53.84%> (-0.48%) ⬇️
routers/web/web.go 90.67% <100.00%> (+0.02%) ⬆️
modules/queue/workerpool.go 48.85% <0.00%> (-1.15%) ⬇️
services/pull/pull.go 41.78% <0.00%> (ø)
modules/queue/queue_channel.go 96.66% <0.00%> (+1.66%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f23851f...5e3d0d0. Read the comment docs.

zeripath
zeripath approved these changes Nov 5, 2021
View reviewed changes
Copy link
Contributor

@zeripath zeripath left a comment

Choose a reason for hiding this comment

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

This suffers from the select for update problems we have elsewhere but I guess it's the same so lgtm

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Nov 5, 2021
@lunny lunny force-pushed the lunny/fix_admin_change_email branch from d7723b6 to 4e863b1 Compare November 8, 2021 07:38
@lunny lunny force-pushed the lunny/fix_admin_change_email branch from a1a568b to 1855130 Compare November 11, 2021 12:40
@lunny lunny mentioned this pull request Nov 17, 2021
Closed
@lunny lunny force-pushed the lunny/fix_admin_change_email branch from 1855130 to 1c4ba86 Compare November 17, 2021 11:47
@lunny
Copy link
Member Author

lunny commented Nov 17, 2021 •
edited
Loading

depend on #17688

@lunny lunny force-pushed the lunny/fix_admin_change_email branch from 65ddcde to ea1cbe5 Compare November 20, 2021 10:31
@lafriks
Copy link
Member

lafriks commented Nov 21, 2021

Please resolve conflicts

@lunny lunny force-pushed the lunny/fix_admin_change_email branch from ea1cbe5 to c3f14b3 Compare November 22, 2021 06:53
lunny
lunny commented Nov 23, 2021
View reviewed changes
routers/api/v1/admin/user.go
return
}

emailChanged = !strings.EqualFold(u.Email, email)
Copy link
Member Author

Choose a reason for hiding this comment

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

We assume ValidateEmail will remove all invisible characters.

@lunny lunny force-pushed the lunny/fix_admin_change_email branch from c3f14b3 to 719a6f3 Compare November 25, 2021 02:16
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Nov 25, 2021
@lunny lunny force-pushed the lunny/fix_admin_change_email branch from 719a6f3 to 47c63ff Compare November 25, 2021 13:15
@techknowlogick techknowlogick merged commit 371ebce into go-gitea:main Nov 26, 2021
@lunny lunny deleted the lunny/fix_admin_change_email branch November 26, 2021 03:12
@lunny lunny mentioned this pull request Nov 28, 2021
Merged
@lunny lunny added the backport/done All backports for this PR have been created label Nov 28, 2021
@pboguslawski pboguslawski mentioned this pull request Feb 4, 2022
Closed
Chianina pushed a commit to Chianina/gitea that referenced this pull request Mar 28, 2022
@lunny
Fix database inconsistent when admin change user email (go-gitea#17549)
64251a2
@go-gitea go-gitea locked and limited conversation to collaborators Apr 28, 2022
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Reviewers

@wxiaoguang wxiaoguang wxiaoguang approved these changes

@zeripath zeripath zeripath approved these changes

Assignees
No one assigned
Labels
backport/done All backports for this PR have been created lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. type/bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Changing email account doesn't release that email
7 participants
@lunny @codecov-commenter @lafriks @zeripath @wxiaoguang @techknowlogick @GiteaBot