Skip to content

Conversation

@joegoggins
Copy link
Contributor

@joegoggins joegoggins commented Jun 11, 2022

Overview

Introduces the ability to set a default auth token on the Particle API instance so you don't have to specify it to each REST related method. Can pass it via auth arg to constructor OR via new setDefaultAuth(auth) method on the instance.

Increases minimum required test coverage to where we currently stand at 91% rather than 50%.

See sc-76665 for more details.

How to test?

Observe the changes to unit tests.

Also, you can end to end test the changes via:

-1: git checkout sc-76665/setDefaultAuth and npm i
0. Run npm run compile

  1. Create a foo script like this in the root of the repo.
const ParticleAPI = require('.');
const auth = 'REDACTED TODO';

async function oldWay() {
    const api = new ParticleAPI();
    const devices = await api.listDevices({auth});
    console.log(devices)
}

async function newWay() {
    const api = new ParticleAPI();
    api.setDefaultAuth(auth);
    const devices = await api.listDevices({});
    console.log(devices)
}

async function newWayViaConstructor() {
    const api = new ParticleAPI({auth});
    const devices = await api.listDevices({});
    console.log(devices)
}

async function newWayViaConstructorWebhooks() {
    const api = new ParticleAPI({auth});
    const foo = await api.listWebhooks({});
    console.log(foo)
}

//oldWay();
//newWay();
//newWayViaConstructor();
newWayViaConstructorWebhooks();
  1. Populate the script with a valid access token
  2. Uncomment one of the invocations or play with others to validate all API methods honor a default auth token and that the old way still works.
  3. Run the script with node foo

@joegoggins joegoggins marked this pull request as ready for review June 14, 2022 19:53
@joegoggins joegoggins requested review from busticated and deleonn June 14, 2022 19:55
Joe Goggins and others added 6 commits June 14, 2022 16:06
@joegoggins
Copy link
Contributor Author

Thanks for the earlier review @busticated. Since then, while integrating this change into https://github.com/particle-iot/cli/pull/133 I discovered and a fixed a bug in 6408739 Also, I decided to fix this tiny bug Brett identified in 10cb93b as well.

Once you've approved these two commits, I'll merge, cut a minor release, and finish validating everything is cool in the Delorean/cli-land PR with API auth usage without the proxy using latest particle-api-js release (i.e. return to https://github.com/particle-iot/cli/pull/133 )

@busticated busticated self-requested a review June 14, 2022 22:42
Copy link
Contributor

@busticated busticated left a comment

Choose a reason for hiding this comment

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

looks good @joegoggins - nice work 🙏 👍

@joegoggins joegoggins merged commit 94b7192 into master Jun 14, 2022
@joegoggins joegoggins deleted the sc-76665/setDefaultAuth branch June 14, 2022 23:01
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.

3 participants