create or update a lead instead of trying to time creation #1211
Conversation
There was a problem hiding this comment.
There's a race condition regardless of you doing the suggestion below or not, as the record can change in between the find and the save. Ideally we'd use an upsert API from Salesforce, if possible. But if it doesn't exist then this should be mostly fine... or at least better than what is there now.
| ) | ||
| user.faculty_status = User::INCOMPLETE_SIGNUP unless user.is_profile_complete? | ||
|
|
||
| lead = OpenStax::Salesforce::Remote::Lead.find_by(email: user.best_email_address_for_salesforce) |
There was a problem hiding this comment.
You could probably do
lead = OpenStax::Salesforce::Remote::Lead.find_by(email: user.best_email_address_for_salesforce) || OpenStax::Salesforce::Remote::Lead.new(email: user.best_email_address_for_salesforce)
and then you don't need the if as the logic is the same after that
There was a problem hiding this comment.
Tried this, but unfortunately, it needs the school to create a new one, so it doesn't work with the ||.
Maybe I can add the school into that call... but let me think about the implications of that (like if the school gets updated).
There was a problem hiding this comment.
@Dantemss I think it's best (not a fan of the duplication) to keep the if statement. I changed it slightly to check if the user has a salesforce_lead_id if so, search for them by email, which I think will cover some of the lead merging that could happen on the salesforce side.
Let me know if you think otherwise -- ready for another look when you have a chance.
|
Will need to merge/release openstax/openstax_salesforce#74 before tests will pass |
|
Also of note: I removed the lead creation for students wanting the newsletter. This was failing because a lead needs a school, which we do not collect from students. |
|
Will rerecord cassettes for SF after we merge this: openstax/openstax_salesforce#75 |
|
|
||
|
|
||
| if user.salesforce_lead_id | ||
| lead = OpenStax::Salesforce::Remote::Lead.find_by(email: user.best_email_address_for_salesforce) |
There was a problem hiding this comment.
OK finding by email here because I remember you writing something about the lead IDs changing?
There was a problem hiding this comment.
yes, exactly - they could merge on the Salesforce side if they already have a lead and it'll change the id (then it updates it on the user later down the script)
We are noticing not all leads are making it into Salesforce and thinking they are getting caught up in a timing issue between the SheerID webhook response and them filling out their profile.
This will allow us to create and update the lead if necessary (upon hearing back from SheerID).
This also creates a lead for folks that did the verification step but still need to complete their profile, setting the lead FV status to 'Incomplete Signup', allowing us to market to them and try to get them back to fill out their profile.