School info in admin, Sentry message on school error during lead creation #1236
Conversation
This reverts commit ff9e4a8.
There was a problem hiding this comment.
I'm thinking instead of this entire PR all you need is maybe another check in update_user_contact_info.rb for this additional case:
if !school.nil? && !sf_school.nil?
In this case you want to check that school.salesforce_id == sf_school.id and if not, then update the user's school?
| updated_school.save! | ||
|
|
||
| merged_school.users.update_all(school: updated_school) if merged_school.users.any? | ||
| merged_school.destroy! |
There was a problem hiding this comment.
I'd call reload before destroy!, just in case... otherwise it could try to dissociate the users.
| @@ -121,6 +121,8 @@ def call | |||
| user.grant_tutor_access = sf_contact.grant_tutor_access | |||
|
|
|||
| if school.nil? && !sf_school.nil? | |||
| # Add the possible updated school id to the record, we check for this in UpdateSchoolSalesforceInfo | |||
| user.school.updated_salesforce_id = sf_school.id | |||
There was a problem hiding this comment.
user.school is maybe not always set?
There was a problem hiding this comment.
Also this code would only be relevant when the school is in SF but not in Accounts, which hopefully only happens sometimes in the first sync after CS adds a school in SF.
|
Nvm, what you actually what is compare |
|
Alternatively, change the |
|
You certainly helped me see that I was on the wrong path in trying to fix the error. The error is popping up when the lead gets created first, which is where it should be caught - then update the users affected so it doesn't impact future syncs with user/school combo. See if you think I still need to modify the foreign key to accomplish what I am trying to do. There might be another place in the application that I should check if the school is 'stale'. |
|
@Dantemss decided to just throw a message to Sentry for now, this also adds some admin info that will be helpful. |
mwvolo
changed the title
There was a problem hiding this comment.
I think we can still have the case where sf_contact.school_id != user.school.salesforce_id. If user.school is supposed to reflect what's in Salesforce, I think we just need to set user.school in app/routines/update_user_contact_info.rb to match whatever is in the contact? Basically where it says school = just change to user.school =
| @@ -121,31 +121,19 @@ def exec(user:) | |||
| event_data: { lead_id: lead.id.to_s } | |||
| ) | |||
| else | |||
| if lead.errors.messages.inspect.include? == 'INSUFFICIENT_ACCESS_ON_CROSS_REFERENCE_ENTITY' | |||
| Sentry.capture_message("Invalid school (#{user.school.salesforce_id}) for user (#{user.id})") | |||
There was a problem hiding this comment.
It's not clear to me that's what the message means but if you are sure from what you've seen in SF go for it.
There was a problem hiding this comment.
This took a bit of command line testing to figure out what SF is spitting out as an error - but yeah, this is the one's that need catching with the ID of that school so it can at least be manually cleaned up (maybe this can move to the admin or some automated process in the future).
|
Nvm, I see this line https://github.com/openstax/accounts/blob/main/app/routines/update_user_contact_info.rb#L132 is supposed to do that |
Ah, this might be where I need to put something to update all the users with that old school id! |
We have a few users that are signing up and getting associated with a school that has likely been merged in Salesforce, or somehow had it's salesforce_id updated. Because there are users associated with it, it doesn't delete when updating schools from Salesforce.
This code will throw a Sentry message for more investigation into what's going on.
I've also added some administrative abilities to see the school associated with the user.