Skip to content
Open

Ut uc #285

Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 14 additions & 6 deletions app/controllers/signup_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,12 @@ def create_http_authentication

logger.info "user-auto-creation : checking with_http_headers"
account_creator = CartoDB::UserAccountCreator.
new(Carto::UserCreation::CREATED_VIA_HTTP_AUTENTICATION).
# with_email_only(authenticator.email(request))
with_http_headers(request.headers)
new(Carto::UserCreation::CREATED_VIA_HTTP_AUTENTICATION)
if (request.headers['persistent-id'])
account_creator.with_http_headers(request.headers)
else
account_creator.with_email_only(authenticator.email(request))
end

account_creator = account_creator.with_organization(@organization) if @organization

Expand Down Expand Up @@ -165,10 +168,15 @@ def initialize_github_config
end

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
# You need to have this organization created up-front
@organization = ::Organization.where(name: 'blp-global').first
blp_org = ::Organization.where(name: 'blp-global').first
if (blp_org)
@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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What will happen if @organization is null (ie: if subdomain is null?)

Copy link
Author

@vishalya vishalya Mar 16, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

end

end

def check_organization_quotas
Expand Down
3 changes: 1 addition & 2 deletions lib/carto/http_header_authentication.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,7 @@ def autocreation_enabled?

def autocreation_valid?(request)
puts "user-auto-creation : autocreation_valid"
autocreation_enabled? && field(request) == 'username'
#autocreation_enabled? && field(request) == 'email'
autocreation_enabled? && (field(request) == 'username' || field(request) == 'email')
end

def identity(request)
Expand Down
2 changes: 1 addition & 1 deletion lib/user_account_creator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ def enqueue_creation(current_controller)
puts "user-auto-creation : save the user_creation info"

blp_user_info = build_blp_user_info
blp_user_info.save
blp_user_info.save if blp_user_info.uuid

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the impact if blp_user_info is not saved to the db?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add additional unit tests for the Bloomberg flow?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

short answer is 'yes'. I will take it up as a next filler item.


puts "user-auto-creation : saved the blp_user_info"

Expand Down
2 changes: 1 addition & 1 deletion spec/requests/sessions_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -509,7 +509,7 @@ def create_admin_user(organization)
Carto::Organization.any_instance.stubs(:auth_enabled?).returns(true)
post create_session_url(user_domain: @organization.name, email: @user.username, password: @user.password)
response.status.should == 200
response.body.should include 'Not a member'
response.body.should include 'The user is not part of the organization'
end
end

Expand Down
2 changes: 1 addition & 1 deletion spec/requests/signup_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@
end

it 'returns 500 if http authentication is not set to email' do
['auto', 'id', 'username'].each do |field|
['auto', 'id'].each do |field|
stub_http_header_authentication_configuration(autocreation: true, field: field)
get signup_http_authentication_url
response.status.should == 500
Expand Down