Stop updating user.username when a user renames or deletes their github account#14072
Stop updating user.username when a user renames or deletes their github account#14072moskirathe wants to merge 3 commits into
Conversation
moskirathe
changed the title
moskirathe
force-pushed
the
stop-updating-username-when-user-deletes-gh-ac
branch
2 times, most recently
from
2f822b7 to
815f295
Compare
…r updates or deletes gh account
moskirathe
force-pushed
the
stop-updating-username-when-user-deletes-gh-ac
branch
from
815f295 to
f4d2a97
Compare
|
sorry, but we can't do this yet without offering users a way to rename their accounts themselves and a way to disambiguate |
The |
| // For now, we want to update the `User` record too | ||
| let user_after_update = User::find(&conn, user_id).await?; | ||
| assert_eq!(user_after_update.gh_login, expected_username); | ||
| assert_eq!(user_after_update.username, expected_username); |
There was a problem hiding this comment.
Rather than removing this assertion, I think it should change to assert that user_after_update.username still equals existing_gh_user.login.
| .do_update() | ||
| .set(( | ||
| users::gh_login.eq(excluded(users::gh_login)), | ||
| users::username.eq(excluded(users::username)), |
There was a problem hiding this comment.
We should probably have a test for this change in behavior; I'm a little surprised that we don't have one that failed.
There was a problem hiding this comment.
Aha, this test would have failed, but it's checking users.gh_login rather than users.username. That test should be changed to username, which should pass without this change, then fail with the change, then the assertion should be changed to assert that users.username stays the same but that the oauth_github.login changes.
thanks for the review @carols10cents! addressed in 2a601ce |
@carols10cents yes, i'd love to tackle #13769. I'll get started on it this weekend. |
4 participants
fixes #13761
Stop updating a user's username when a user updates or deletes their github account. This update currently happens on background job added in #13143 and on login.