Added Alexa Contact, Device, ISP and List APIs #149
Conversation
docs/inSkillPurchases.rst
Outdated
|
|
||
| .. code-block:: javascript | ||
|
|
||
| skill.onIntent('BuyIntent', async (voxaEvent) => { |
There was a problem hiding this comment.
I know ISP is an Alexa feature, but shouldn't this be app instead of skill?
There was a problem hiding this comment.
Same for all other skill statements
There was a problem hiding this comment.
Yeah, I think it makes sense. I was just following the same structure of the docs. If you look at other docs' examples, it uses skill as well. Also, in the headspace action and skill, the guys are still using skill for their intents/states:
https://github.com/mediarain/headspace-skill/blob/V3-staging/app/states/alexa.states.js
@armonge thoughts on this change ?? If we go with @lvelasquezm suggestion, I'd have to change all the docs (and probably the tests?)
I also think is a matter of the developer's preference. I really like skill since I'm coming from the voxa2 version and I'm used to that. But open to any guidance on this topic.
There was a problem hiding this comment.
We should call them app, in Voxa3 most of the code is part of the VoxaApp, the skill is just the actual wrapper. Also, if you see the docs calling stuff skill instead of app go ahead and change that, wherever it makes sens
docs/index.rst
Outdated
| @@ -133,6 +133,11 @@ Links | |||
| statemachine-skill | |||
| request-flow | |||
| i18n | |||
| customerContact | |||
There was a problem hiding this comment.
we need a way to signal the user from the menu that this is alexa only
| @@ -61,6 +61,7 @@ | |||
| "@types/chai": "^4.0.10", | |||
| "@types/chai-as-promised": "^7.1.0", | |||
| "@types/mocha": "^2.2.45", | |||
| "@types/nock": "^9.3.0", | |||
There was a problem hiding this comment.
i believe you also need to add nock, not only @types/nock
There was a problem hiding this comment.
Hmm I didn't have any issues in my tests nor lint, nor in Travis, do you still think is worth it to include it ?
There was a problem hiding this comment.
yes, most likely nock was included as a dependency from another package, however the @types/nock package does not include it as a dependency. Add nock in here to have a specific version and not depend on side effects from other packages
src/VoxaEvent.ts
Outdated
| @@ -24,6 +24,8 @@ export abstract class IVoxaEvent { | |||
| public requestToIntent: ITypeMap = {}; | |||
| public requestToRequest: ITypeMap = {}; | |||
| public platform!: string; | |||
| public alexa: any; | |||
There was a problem hiding this comment.
actually, this seems like keys that should only be present in the AlexaEvent and DialogFlowEvent subclasses
There was a problem hiding this comment.
Also, they're not any we know which keys are present, go ahead and define a type
There was a problem hiding this comment.
Ok, I'll remove the google one because I'm not using it. I'll just move out the alexa key
src/platforms/alexa/AlexaEvent.ts
Outdated
| @@ -50,6 +57,14 @@ export class AlexaEvent extends IVoxaEvent { | |||
| } | |||
|
|
|||
| this.platform = "alexa"; | |||
|
|
|||
| this.alexa = { | |||
| customerContact: new CustomerContact(event), | |||
There was a problem hiding this comment.
instead of event better pass the rawEvent
src/platforms/alexa/apis/ApiBase.ts
Outdated
| export class ApiBase { | ||
| public errorCodeSafeToIgnore: number = 0; // the code error to ignore on checkError function | ||
| public tag: string = ""; // the class reference for error logging | ||
| public voxaEvent: any; // the event as sent by the service |
There was a problem hiding this comment.
instead of using a voxaEvent perhaps it's better to use the rawEvent which has a type of RequestEnvelope if i'm not mistaken. In any case, let's try and not use any when possible
src/platforms/alexa/apis/ApiBase.ts
Outdated
| this.voxaEvent = _.cloneDeep(event); | ||
| } | ||
|
|
||
| protected getResult(path = "", method = "GET", body = {}): any { |
There was a problem hiding this comment.
the type of each api response can be found in the ask-sdk-model package, for example
services.deviceAddress.Address
services.deviceAddress.ShortAddress
Let's use those, seems like a great oportunitty for generics
| public voxaEvent: any; // the event as sent by the service | ||
|
|
||
| constructor(event: RequestEnvelope) { | ||
| this.voxaEvent = _.cloneDeep(event); |
There was a problem hiding this comment.
check this, you're passing a voxaEvent as any in the calling AlexaEvent class, however this class takes a RequestEnvelope in the constructor
|
Also, seems like we should update https://github.com/mediarain/voxa/blob/v3/docs/voxa-event.rst |
|
@armonge done, just added bullets to group Alexa APIs in a single item |
Features added: