add adopter status to user as string #1207
Conversation
|
Thanks, @Dantemss! I knew that was a problem so I through the wip label on here... can you take one more quick glance at it to make sure it looks sane? |
There was a problem hiding this comment.
If this task is just for the initial migration and since the change is mainly happening to the local data, I'd reverse it: load 250 users from the local DB that have adopter_status: nil first, then look up their contacts only. This way if you need to stop and resume the process it'll look only at the users that are still missing their adopter_status.
| user.adopter_status = contact.adoption_status | ||
| user.save! | ||
| end | ||
| rescue StandardError => se |
There was a problem hiding this comment.
If this rescues an error, you'll skip up to 250 contacts (the entire batch) which is not great. May be less of a problem if we make this resumable. See above.
There was a problem hiding this comment.
I think you can remove the rescue now. Just let it fail if something unexpected happens. Otherwise it can get stuck in an infinite loop.
If it fails, you can always retry.
| user = user_by_salesforce_id[contact.id] | ||
| user.adopter_status = contact.adoption_status | ||
| user.save! | ||
| end |
There was a problem hiding this comment.
I'd wrap each batch inside a separate transaction. I think that will improve performance (rather than using autocommit to automatically create a transaction for each save!).
There was a problem hiding this comment.
I think this is how it's done but let me know if this doesn't look like the correct way to do this.
| :accounts_uuid | ||
| ) | ||
| .where(id: users.map(&:salesforce_contact_id)) | ||
| .index_by(&:id) |
There was a problem hiding this comment.
I'm a little fuzzy on if I did this correctly, will this work properly with line 22 to map the correct contact to the user?
There was a problem hiding this comment.
You need to remove .to_a in line 19 below if you want a hashmap.
| contacts = contacts.where("Id > '#{last_id}'") unless last_id.nil? | ||
| contacts = contacts.to_a | ||
| last_id = contacts.last&.id | ||
| users = User.where(adopter_status: nil).order(:id).limit(250) |
There was a problem hiding this comment.
I would say User.where(adopter_status: nil).where.not(salesforce_contact_id: nil).limit(250) here (order(:id) is also fine but optional in this case).
| contacts = contacts.to_a | ||
| last_id = contacts.last&.id | ||
| users = User.where(adopter_status: nil).order(:id).limit(250) | ||
| users = users.where("id > #{last_id}") unless last_id.nil? |
There was a problem hiding this comment.
With the above change you shouldn't need this anymore (or to track last_id). You just find 250 of them and update so adopter_status is not nil and they won't be found in later iterations.
| end | ||
|
|
||
| updated_users.transaction do | ||
| updated_users.save! |
There was a problem hiding this comment.
I think you need updated_users.each(&:save!)
| user.adopter_status = contact.adoption_status | ||
| user.save! | ||
| end | ||
| rescue StandardError => se |
There was a problem hiding this comment.
I think you can remove the rescue now. Just let it fail if something unexpected happens. Otherwise it can get stuck in an infinite loop.
If it fails, you can always retry.
| end | ||
| rescue StandardError => se | ||
| Sentry.capture_exception se | ||
| end |
There was a problem hiding this comment.
You still need to break out of the loop when there are less than 250 users.
There was a problem hiding this comment.
I was wondering why this was running for so long locally! Fixed
| :accounts_uuid | ||
| ) | ||
| .where(id: users.map(&:salesforce_contact_id)) | ||
| .index_by(&:id) |
There was a problem hiding this comment.
@Dantemss it looks like index_by is not supported by ActiveForce. I'll see what I can do to fix this.
There was a problem hiding this comment.
Try .to_a before .index_by and it should work, since index_by is an array method.
No description provided.