Conversation
acarrera42
left a comment
acarrera42
left a comment
There was a problem hiding this comment.
Let's also run the full unit test suite on this PR as well to verify the unit tests have reduced on our CI boxes.
| @organization = blp_org | ||
| else | ||
| subdomain = CartoDB.subdomainless_urls? ? request.host.to_s.gsub(".#{CartoDB.session_domain}", '') : CartoDB.subdomain_from_request(request) | ||
| @organization = ::Organization.where(name: subdomain).first if subdomain |
There was a problem hiding this comment.
What will happen if @organization is null (ie: if subdomain is null?)
There was a problem hiding this comment.
It's an invalid scenario for us, since we always need to have the global scenario, the other flow is for carto. @organization is setup in both flows, bloomberg vs non-bloomberg, so it should always be there.
| @@ -168,7 +171,14 @@ def load_organization | |||
| #subdomain = CartoDB.subdomainless_urls? ? request.host.to_s.gsub(".#{CartoDB.session_domain}", '') : CartoDB.subdomain_from_request(request) | |||
| #@organization = ::Organization.where(name: subdomain).first if subdomain | |||
There was a problem hiding this comment.
Should we remove these comments since they are active below now?
|
|
||
| blp_user_info = build_blp_user_info | ||
| blp_user_info.save | ||
| blp_user_info.save if blp_user_info.uuid |
There was a problem hiding this comment.
What's the impact if blp_user_info is not saved to the db?
There was a problem hiding this comment.
If the user creation doesn't have the record during the creation, the user creation fails for bloomberg flow. From the tests, the carto flow should be good.
There was a problem hiding this comment.
Should we add additional unit tests for the Bloomberg flow?
There was a problem hiding this comment.
short answer is 'yes'. I will take it up as a next filler item.
2 participants
We use the username via http headers