Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: Apply credential overwrites recursively #5072

Merged
merged 2 commits into from
Jan 4, 2023
Merged

fix: Apply credential overwrites recursively #5072

merged 2 commits into from
Jan 4, 2023

Conversation

netroy
Copy link
Member

@netroy netroy commented Jan 2, 2023

This ensures that overwrites defined for a parent credential type also applies to all credentials extending it.

@netroy netroy requested a review from maspio January 2, 2023 19:07
@n8n-assistant n8n-assistant bot added core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team labels Jan 2, 2023
@maspio
Copy link
Contributor

maspio commented Jan 3, 2023

  • Overriding googleOAuth2Api and microsoftOAuth2Api both work for google/microsoft services.
  • Overriding oAuth2Api does not work but also doesn't make sense to override so I guess we can ignore it.

@netroy
fix: Apply credential overwrites recursively
1f42af8 
This ensures that overwrites defined for a parent credential type also applies to all credentials extending it.
@netroy netroy force-pushed the fix-parent-credential-overwrites branch from 1c9b4e2 to 1f42af8 Compare January 4, 2023 15:04
@netroy netroy marked this pull request as ready for review January 4, 2023 15:04
@maspio
Copy link
Contributor

maspio commented Jan 4, 2023

Overriding oAuth2Api does work now 🚀 Maybe not useful but proofs that recursive overwrite works. Great work, looks good to me.

@netroy netroy merged commit 5d746c4 into master Jan 4, 2023
@netroy netroy deleted the fix-parent-credential-overwrites branch January 4, 2023 17:16
@n8n-assistant n8n-assistant bot added the Upcoming Release Will be part of the upcoming release label Jan 4, 2023
@janober
Copy link
Member

janober commented Jan 5, 2023

Got released with n8n@0.210.0

@janober janober removed the Upcoming Release Will be part of the upcoming release label Jan 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maspio maspio maspio approved these changes

Assignees
No one assigned
Labels
core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@netroy @maspio @janober