(feat) [NAN-677] enforce cap when calling nangoauth

[khaliqgant]

Describe your changes

Add check when creating a new connection and importing a connection to see if the account has reached the cap.

Issue ticket number and link

NAN-680

Checklist before requesting a review (skip if just adding/editing APIs & templates)

• I added tests, otherwise the reason is:
• I added observability, otherwise the reason is:
• I added analytics, otherwise the reason is:

[linear]

NAN-677 Enforce cap when calling `nango.auth`

[khaliqgant]

Happy to get another pair of eyes on this query to improve it.

[bodinsamuel]

I guess you only want connection with syncs? I feel like it's harder to explain to customers but whatever :D
Isn't something like this enough? I'm not sure why you selected that much stuff from the table

SELECT
	COUNT(1)
FROM
	_nango_connections
JOIN _nango_syncs ON _nango_syncs.nango_connection_id = _nango_connections.id AND _nango_syncs.deleted = FALSE
WHERE
	environment_id = 2
	AND provider_config_key = 'github-demo'
	AND _nango_syncs.deleted = FALSE;

and if it's only 3 connections ever

SELECT
	COUNT(1)
FROM
	_nango_connections
WHERE
	environment_id = 2
	AND provider_config_key = 'github-demo'
	AND _nango_syncs.deleted = FALSE;

[khaliqgant]

SELECT
	COUNT(1)
FROM
	_nango_connections
JOIN _nango_syncs ON _nango_syncs.nango_connection_id = _nango_connections.id AND _nango_syncs.deleted = FALSE
WHERE
	environment_id = 2
	AND provider_config_key = 'github-demo'
	AND _nango_syncs.deleted = FALSE;

Thanks, that query would work nicely but at some point I would like to move away from storing the provider_config_key in the connections table and instead rely on the config_id there which relates back to the _nango_configs table. As a result of the provider_config_key being stored in the connections table a user can't edit a provider_config_key if there are connections are attached to it. Happy to discuss that more if you want?

[bodinsamuel]

This give you all connections with active syncs or actions for a provider.

SELECT
	conn.id
FROM
	_nango_configs AS configs
	JOIN _nango_connections AS conn ON configs.id = conn.config_id
		AND conn.deleted = FALSE
	JOIN _nango_sync_configs AS syncs ON syncs.nango_config_id = configs.id
		AND syncs.deleted = FALSE
		AND syncs.active = TRUE
WHERE
	configs.environment_id = 2
        AND configs.provider_config_key = 'github-demo'
	AND configs.deleted = FALSE
GROUP BY
	conn.id
HAVING
	COUNT(syncs.id) > 0;

then res.length >= 3

[khaliqgant]

Thanks, this doesn't give me what I want either

SELECT
	conn.id
FROM
	nango._nango_configs AS configs
	JOIN nango._nango_connections AS conn ON configs.id = conn.config_id
		AND conn.deleted = FALSE
	JOIN nango._nango_sync_configs AS syncs ON syncs.nango_config_id = configs.id
		AND syncs.deleted = FALSE
		AND syncs.active = TRUE
WHERE
	configs.environment_id = 2
        AND configs.unique_key = 'unauthenticated'
	AND configs.deleted = FALSE
GROUP BY
	conn.id
HAVING
	COUNT(syncs.id) > 0;

gives me one row

[bodinsamuel]

Yeah indeed, I realized that _nango_sync_configs are per config not per connection and _nango_syncs are per connection but do not contain actions :/
I guess you got the right one then!

[khaliqgant]

Pending from Bastien.

[bodinsamuel]

small review, not tested

[bodinsamuel]

I guess you only want connection with syncs? I feel like it's harder to explain to customers but whatever :D
Isn't something like this enough? I'm not sure why you selected that much stuff from the table

SELECT
	COUNT(1)
FROM
	_nango_connections
JOIN _nango_syncs ON _nango_syncs.nango_connection_id = _nango_connections.id AND _nango_syncs.deleted = FALSE
WHERE
	environment_id = 2
	AND provider_config_key = 'github-demo'
	AND _nango_syncs.deleted = FALSE;

and if it's only 3 connections ever

SELECT
	COUNT(1)
FROM
	_nango_connections
WHERE
	environment_id = 2
	AND provider_config_key = 'github-demo'
	AND _nango_syncs.deleted = FALSE;

[TBonnin]

we are making more queries to the db on each request by adding a middleware which is doing something similar to the access.middleware, ie: fetching the account and environment from the db. Can we fetch the account.is_capped as part of the access.middleware and save it into the res.locals like we do with accountId and environmentId so it can be access down the line by controllers?

[khaliqgant]

I like the idea in theory, but the issue is that the value of is_capped can change via the api and then the value stored in the res.locals is stale and potentially inaccurate. Since the user changing the is_capped setting is different than the user who has the capping nor not.

[bodinsamuel]

res.locals is only stored for the current request if that's your worry

[khaliqgant]

Ah yes, but still this is making it more complicated than it needs since right now the only controller it goes through is the connection.controller when it is imported. If it went into the access.middleware instead of the capping.middleware then the logic to check the cap would need to go through:

• oauth.controller
• unauth.controller
• apiAuth.controller
• appStoreController

[TBonnin]

do we need a new function? Can we not call syncService.getSyncsByProviderConfigKey and count the connections?

[khaliqgant]

That method would be good, but we need both to check against syncs and actions.

[khaliqgant]

@bodinsamuel are you good with this one? A few other tasks rely on this one so is semi blocking but not a huge issue.

[bodinsamuel]

I'm sure we can improve it later 👌🏻