OAuth BigQuery Connections #16191
OAuth BigQuery Connections #16191
Conversation
|
This pull request has been linked to Clubhouse Story #136326: Implement OAuth connection for BigQuery. |
jgoizueta
force-pushed
the
feature/ch136326/implement-oauth-connection-for-bigquery
branch
from
73b231a
to
56613ad
Compare
…ent-with-tileset-list Fix invalid query for listing no tilesets
…y' of github.com:CartoDB/cartodb into feature/ch136326/implement-oauth-connection-for-bigquery
For some reason (some middleware?) responses with status 401 are altered to content-type jaon with empty body
|
@jgoizueta do you mind updating your branch with master? chances are it fixes some things related to broker behavior and it helps me test without kidnapping staging :) |
| @@ -35,6 +36,21 @@ def errors | |||
| errors << 'Parameter access_token not supported through connections; use import API' | |||
| end | |||
| end | |||
| if @connection.connection_type == Carto::Connection::TYPE_OAUTH_SERVICE | |||
| unless incomplete? || @connection.parameters['billing_project'].present? | |||
There was a problem hiding this comment.
This should check if incomplete? right?
There was a problem hiding this comment.
it's an unless incomplete? because we don't wan't to validate the presence of the billing project if the connection is incomplete: and incomplete connection must be saved without errors in an incomplete state when the refresh token is saved in it by the oauth callback.
🤔 I guess that unless ... || ... doesn't help to understand this at first sight, maybe it should be rewritten as
if complete? && @connection.parameters['billing_project'].blank?There was a problem hiding this comment.
ahh oki doki. I think that no change is needed, I was my fault understanding the case
There was a problem hiding this comment.
(I'd rather use the straight if rather than the negative logic version, turns out more readable and less prone to misinterpretation... unless you don't agree with me)
(No puedo negar que no esté en desacuerdo contigo -- Groucho)
…n-for-bigquery # Conflicts: # NEWS.md # spec/models/carto/connection_spec.rb
complete? validates, so checks for error, so it shouldn't be used in errors!
To make evident the former incomplete? method is not the negation of compliete?
Otherwise they won't appear in the Dashboard and the user won't be able to delete them.
…n-for-bigquery # Conflicts: # NEWS.md
|
This has been merged into master through #16218 |
See https://app.clubhouse.io/cartoteam/story/136326