feat: Create TOTP node #5901
feat: Create TOTP node #5901
Conversation
|
Great PR! Please pay attention to the following items before merging: Files matching
Files matching
Files matching
Make sure to check off this list before asking for review. |
n8n-assistant
bot
added
n8n team
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #5901 +/- ##
==========================================
+ Coverage 17.55% 17.57% +0.01%
==========================================
Files 2498 2500 +2
Lines 114358 114384 +26
Branches 17867 17870 +3
==========================================
+ Hits 20080 20098 +18
- Misses 93686 93694 +8
Partials 592 592
... and 2 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
| @@ -30,6 +30,24 @@ import path from 'path'; | |||
| import { tmpdir } from 'os'; | |||
| import { isEmpty } from 'lodash'; | |||
|
|
|||
| import { FAKE_CREDENTIALS_DATA } from './FakeCredentialsMap'; | |||
| if (!options.algorithm) options.algorithm = 'SHA1'; | ||
| if (!options.digits) options.digits = 6; | ||
| if (!options.period) options.period = 30; |
There was a problem hiding this comment.
Is there a cleaner way to specify default options? Adding default: { algorithm: 'SHA1', period: 30, digits: 6 } causes the options to start off revealed, which is not what we want, but otherwise options defaults to {}, which is also not what we want.
There was a problem hiding this comment.
something like this?
const options = {
algorithm: 'SHA1',
digits: 6,
period: 30,
...(this.getNodeParameter('options', 0, {}) as IDataObject)),
};
There was a problem hiding this comment.
Sorry I should've been clearer. By "cleaner" I meant to avoid repeating all these defaults that were already specified in the node params above, for maintainability.
There was a problem hiding this comment.
not that I'm aware of, we could use some const/enum defaultOptions to make it dryer
There was a problem hiding this comment.
Thanks! I think the ideal would be for getNodeParameter to return an object with every field set to its default value, so that making it dryer does not introduce too much indirection. Let's ignore for now then.
|
✅ All Cypress E2E specs passed |
|
Got released with |
* ✨ Create TOTP node * ♻️ Apply feedback * ♻️ Recreate `pnpm-lock.yaml` * ♻️ Apply Giulio's feedback * 🚧 WIP node tests * ✅ Finish node test setup * ⏪ Restore test command * ⚡ linter fixes, tweaks * ♻️ Address Michael's feedback --------- Co-authored-by: Michael Kret <[email protected]>
https://linear.app/n8n/issue/ADO-517/totp-node