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.

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

PDE-5323 feat(cli): invoke auth refresh subcommand #878

Merged
merged 6 commits into from
Oct 10, 2024

Conversation

eliangcs
Copy link
Member

@eliangcs eliangcs commented Oct 7, 2024

Changes:

  • add invoke auth refresh subcommand
  • add invoke command to smoke tests
    • while I was there, I had to fix an issue with the dynamic-dropdown project template, where https://swapi.dev/api/people can have empty species if the person is a human.
  • add --local-port flag for auth start subcommand so that the local port can be different from the one you specify in --redirect-uri, which is essential if you're doing port forwarding
  • fix an issue where --inputData @file.json didn't really work
    • turns out if you're using fs Promises API (require('node:fs/promises')), you can't use fs.createReadStream(); you need to open the file first with const fd = await fs.open() and then fd.createReadStream()
  • fix an issue where the rendered HTML command docs had redundant line breaks

The zapier invoke auth refresh command is available for session and oauth2 auth types.

For oauth2, it invokes authentication.oauth2Config.refreshAccessToken using the existing auth data in the .env file. It gets the new auth data and appends it to .env.

For session auth, it's the same except for it invokes authentication.sessionConfig.perform.

@eliangcs eliangcs changed the base branch from main to PDE-5319-invoke-auth-start October 7, 2024 08:32
@eliangcs eliangcs force-pushed the PDE-5323-invoke-auth-refresh branch from 8ec8058 to ce2afea Compare October 8, 2024 09:38
Base automatically changed from PDE-5319-invoke-auth-start to main October 9, 2024 03:56
@eliangcs eliangcs marked this pull request as ready for review October 9, 2024 07:42
@eliangcs eliangcs requested a review from a team as a code owner October 9, 2024 07:42
Copy link
Contributor

@standielpls standielpls left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't get a chance to test the refresh option, but the code looks ok! Default to approving, if you want a full manual test for all the options, happy to help conduct one as well!

Comment on lines +1028 to +1029
const fd = await fs.open(filePath);
inputStream = fd.createReadStream({ encoding: 'utf8' });
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting, is this an edge case? I definitely recalled testing the input file option and it worked ok

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed require('node:fs') to require('node:fs/promises') in the auth start PR, so it was working and then was broken by the auth start PR.

@eliangcs eliangcs merged commit c7278e1 into main Oct 10, 2024
14 checks passed
@eliangcs eliangcs deleted the PDE-5323-invoke-auth-refresh branch October 10, 2024 04:25
@eliangcs eliangcs mentioned this pull request Oct 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants