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

update merge accounts script #9213

Merged
merged 1 commit into from
May 3, 2024
Merged

update merge accounts script #9213

merged 1 commit into from
May 3, 2024

Conversation

s-cheng
Copy link
Collaborator

@s-cheng s-cheng commented May 2, 2024

I noticed a few issues when I tried to merge two accounts, so here is my attempt to fix them:

  1. The post coauthorship values didn't get transferred to the new account
  2. The userId field in message (and other table) contents didn't get updated to the new account, although the message userId values did get properly updated (but this may cause issues if we normalize the contents field)
  3. Some newer/minor collections didn't get transferred (like election votes)
  4. I think there was an issue with unsetting services.resume, though I'm not 100% confident. In any case, after I updated compileUpdateField() to fix issue (2) above, the unsetting was definitely not working in my testing, so I've updated the function to properly handle unsetting services.resume.

┆Issue is synchronized with this Asana task by Unito

@s-cheng s-cheng requested a review from a team as a code owner May 2, 2024 19:57
@s-cheng s-cheng requested review from jpaddison3 and removed request for a team May 2, 2024 19:57
@jpaddison3 jpaddison3 self-assigned this May 2, 2024
[column, ` - '${fieldTokens[1]}'`],
);
} else {
throw new Error(`Unsetting a field past the first level of a JSON blob is not yet supported`)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As far as I can tell, we don't unset past the first level anywhere so I decided to just fix the easy case

@@ -189,6 +189,10 @@ Vulcan.mergeAccounts = async ({sourceUserId, targetUserId, dryRun}: {

// Transfer posts
await transferCollection({sourceUserId, targetUserId, collectionName: "Posts", dryRun})
// Transfer post co-authorship
if (!dryRun) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this the only check for dryRun?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There are some things in this script that already follow this pattern, like moveUserConversationsToNewUser(). We could add dryRun versions for those things (ideally this script would have those) but I felt like this PR had enough changes.

@jpaddison3 jpaddison3 assigned s-cheng and unassigned jpaddison3 May 3, 2024
@s-cheng s-cheng merged commit c66e514 into master May 3, 2024
18 checks passed
@s-cheng s-cheng deleted the update-merge-accounts branch May 3, 2024 15:18
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.

None yet

2 participants