Conversation
DrewProebstel
changed the title
|
i remember rotating the intercom keys to be a bit tricky when i did it for vita-min. i don’t see the intercom workspace when i log into intercom. did you have a chance to do that? |
Is rotating the key an important part of this work? Since we are using the FYST key can't it just rotate when we do our other rotations? |
|
that’s fine. you don’t have to rotate them. i just wanted to call out setting the keys. have you set the keys for PYA? FYST and PYA have different ways of setting environment variables. also, i don’t want to be a blocker, so feel free to slack me if you’re waiting on a response from me. |
| <%= javascript_importmap_tags "application" %> | ||
| <%= Sentry.get_trace_propagation_meta.html_safe %> | ||
|
|
||
| <%= render "layouts/intercom_jwt" %> |
There was a problem hiding this comment.
I noticed in the JIRA ticket it noted that it want the intercom widget to display on only these pages:
Year Selection: /year_select
Contact preference: /contact_preference
Entering email address: /email_address
Entering phone number: /phone_number
Entering verification code: /verification_code
Entering SSN/ITIN: /identification_number
Selecting mailing address: /mailing_address_validation
PDF download: /pdf
All offboarding & lockout pages
but would this display it on the whole site instead?
There was a problem hiding this comment.
that is the whole site actually.
| # == Enabled Environments | ||
| # Which environments is auto inclusion of the Javascript enabled for | ||
| # | ||
| config.enabled_environments = ["development", "production"] |
There was a problem hiding this comment.
should you add demo & staging here as well?
There was a problem hiding this comment.
our staging actually has production as the environment so we don't need to.
There was a problem hiding this comment.
Hmmm staging should not share the production creds 😬, that would mean if someone sent a message on the staging site it would go to the production intercom and confuse the client success folks right?
There was a problem hiding this comment.
The creds are stored and accessed using amazon secret key manager so the correct creds will be used here.
config/initializers/intercom.rb
Outdated
| # The method/variable that contains the logged in user in your controllers. | ||
| # If it is `current_user` or `@user`, then you can ignore this | ||
|
|
||
| config.user.current = proc {} |
There was a problem hiding this comment.
proc{} will return nil right? so this line can probably deleted if its the default current user other wise config.user.current = proc {other_name_for_current_user}
There was a problem hiding this comment.
unless you want all intercom chatters to look like logged out clients?
| IntercomRails.config do |config| | ||
| # == Intercom app_id | ||
| # | ||
| config.app_id = ENV["INTERCOM_APP_ID"] || "rtcpj4hf" |
There was a problem hiding this comment.
should we do config.app_id = ENV.fetch("INTERCOM_APP_ID", nil) instead so its more obvious when env is missing and then put "rtcpj4hf" for all the test envs?
There was a problem hiding this comment.
good idea I am moving this into doppler
app/services/intercom_service.rb
Outdated
| @@ -0,0 +1,13 @@ | |||
| class IntercomService | |||
| def self.generate_user_hash(user_id) | |||
There was a problem hiding this comment.
where is this getting called? and maybe we don't need it since we are doing JWT stuff for security instead?
| return nil unless intake | ||
|
|
||
| payload = { | ||
| user_id: intake.id.to_s, |
There was a problem hiding this comment.
user_id should be a consistent identifier for intercom, what happens if there is a returning client with a new intake id? i think we will run into conflicts here if a user has more than one intake
There was a problem hiding this comment.
We won't have a user with multiple intakes because StateFileArchivedIntake functions as the user.
There was a problem hiding this comment.
So if a client uses the site in 2025 and then again in 2026 wouldn't this create two StateFileArchivedIntakes? or would it use the same object in the model?
There was a problem hiding this comment.
PYA doesn't have anything like a user so I think each intake would need to have its own chat.
| @@ -0,0 +1,19 @@ | |||
| # app/services/intercom_jwt.rb | |||
| require "jwt" | |||
There was a problem hiding this comment.
i guess since we are using this we don't need to do the identity verification stuff i saw here that it says its deprecated so that we should use JSON Web Tokens instead, so nice work with this
| return nil unless intake | ||
|
|
||
| payload = { | ||
| user_id: intake.id.to_s, |
There was a problem hiding this comment.
So if a client uses the site in 2025 and then again in 2026 wouldn't this create two StateFileArchivedIntakes? or would it use the same object in the model?
| # == Enabled Environments | ||
| # Which environments is auto inclusion of the Javascript enabled for | ||
| # | ||
| config.enabled_environments = ["development", "production"] |
There was a problem hiding this comment.
Hmmm staging should not share the production creds 😬, that would mean if someone sent a message on the staging site it would go to the production intercom and confuse the client success folks right?
4 participants
Link to pivotal/JIRA issue
Is PM acceptance required? (delete one)
Reminder: merge main into this branch and get green tests before merging to main
What was done?
How to test?
Screenshots (for visual changes)