fix(server): Fix case of duplicate connectionId with different providers for /connections/:id #3198
Conversation
| connectionConfig?: any; | ||
| } | ||
| ): Promise<NangoConnection> => { | ||
| const name = rest?.connectionId ? rest.connectionId : Math.random().toString(36).substring(7); |
There was a problem hiding this comment.
🍖 make connectionId configurable through rest
There was a problem hiding this comment.
not a huge fan of this pattern. I am ok going with it because this change is time sensitive as long as we follow up immediately with a PR to use a object as parameter
There was a problem hiding this comment.
Its not a pattern its just like that for historical reason, changing to object will update a lot of tests so it was easier like this (we cant fix everything all the time unfortunately)
Happy to see it changed to object 👌🏻
| }); | ||
| }); | ||
|
|
||
| it('should get a connection despite another connection with same name on a different provider', async () => { |
There was a problem hiding this comment.
🍖 Add test with multiple providers. I can remove the prior test if we want but it was helpful to have a case I could run with the regression and the fix not applied yet.
|
|
||
| // We get connection one last time to get endUser, errors | ||
| // This is very unoptimized unfortunately | ||
| const finalConnections = await connectionService.listConnections({ environmentId: environment.id, connectionId, integrationIds: [providerConfigKey] }); |
| connectionConfig?: any; | ||
| } | ||
| ): Promise<NangoConnection> => { | ||
| const name = rest?.connectionId ? rest.connectionId : Math.random().toString(36).substring(7); |
There was a problem hiding this comment.
not a huge fan of this pattern. I am ok going with it because this change is time sensitive as long as we follow up immediately with a PR to use a object as parameter
Reverts the previous revert and adds test for that test case.