You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
"Add email" button showing incorrectly on a private domain's workspace #5285
changed the title [-]"Add email" button showing incorrectly on a private domain[/-][+]"Add email" button showing incorrectly on a private domain's workspace[/+]on Sep 15, 2021
👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open StagingDeployCash deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:
Identify the pull request that introduced this issue and revert it.
This is probably related to this PR #5037. We probably don't have the domain in the common domain list or bedrock cache. That means we are actually going to Clearbit for certainty and meanwhile we treat the domain as public.
Do we require certainty that the domain is private in this case?
Just to follow up here. I'm still a bit unsure why the PRs we are reverting will fix anything and did not really get very far in reproducing this issue beyond the realization that email validation for non obviously public domains seems broken on dev (I used the clitools account validator and it was broken). So I'd guess (but did not confirm) that some users may have experienced issues validating accounts during the short time this code was live.
Once I locally reverted the lines here I was able to validate a non-public domain just fine without throwing any errors and logged in OK in New Expensify on web.
After creating and navigating to a workspace I still did not see an Add Email button like Kevin shared.
So TL;DR revert seems like a good call. But whether it fixed this issue I'm not too sure.
@luacmartins If you are up for it at some point (tomorrow maybe since it's late 😅 ) it would be good to provide a summary on this issue to explain why @kevinksullivan experienced this if it's obvious?
Following up on this issue. I didn't manage to reproduce the issue @kevinksullivan had, but it seems that reverting the PRs resolved the issue, so I think we can close this issue 🎉
The Clearbit exception being thrown here, as pointed out by @marcaaron, seems to be the problem in this case. However, I'm still unsure why new domains did not experience that behavior before these PRs, because AFAIK we triggered that same Clearbit exception while calling isFromPublicDomain. I'll try to investigate this more as part of this comment.
Activity
MelvinBot commentedon Sep 15, 2021
Triggered auto assignment to @mountiny (
Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.[-]"Add email" button showing incorrectly on a private domain[/-][+]"Add email" button showing incorrectly on a private domain's workspace[/+]OSBotify commentedon Sep 15, 2021
👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open
StagingDeployCashdeploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:luacmartins commentedon Sep 16, 2021
This is probably related to this PR #5037. We probably don't have the domain in the common domain list or bedrock cache. That means we are actually going to Clearbit for certainty and meanwhile we treat the domain as public.
Do we require certainty that the domain is private in this case?
marcaaron commentedon Sep 16, 2021
Just to follow up here. I'm still a bit unsure why the PRs we are reverting will fix anything and did not really get very far in reproducing this issue beyond the realization that email validation for non obviously public domains seems broken on dev (I used the clitools account validator and it was broken). So I'd guess (but did not confirm) that some users may have experienced issues validating accounts during the short time this code was live.
The error I saw is here.
Once I locally reverted the lines here I was able to validate a non-public domain just fine without throwing any errors and logged in OK in New Expensify on web.
https://github.com/Expensify/Web-Expensify/blob/576c72e2325caf9bfb6f8a318ac9d82add88fc9e/lib/UserAPI.php#L1818-L1823
After creating and navigating to a workspace I still did not see an
Add Emailbutton like Kevin shared.So TL;DR revert seems like a good call. But whether it fixed this issue I'm not too sure.
@luacmartins If you are up for it at some point (tomorrow maybe since it's late 😅 ) it would be good to provide a summary on this issue to explain why @kevinksullivan experienced this if it's obvious?
luacmartins commentedon Sep 16, 2021
Following up on this issue. I didn't manage to reproduce the issue @kevinksullivan had, but it seems that reverting the PRs resolved the issue, so I think we can close this issue 🎉
The Clearbit exception being thrown here, as pointed out by @marcaaron, seems to be the problem in this case. However, I'm still unsure why new domains did not experience that behavior before these PRs, because AFAIK we triggered that same Clearbit exception while calling
isFromPublicDomain. I'll try to investigate this more as part of this comment.