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

Possible fix for #77 #83

Closed
wants to merge 6 commits into from

Conversation

Projects
None yet
2 participants
@benedekh
Copy link
Contributor

commented Mar 24, 2017

Hello,

I implemented a possible fix for #77. It makes the application id queryable and replaces the template application id in the queries, when the server is not in production. Some tests are implemented also.

benedekh added some commits Mar 24, 2017

@@ -2,7 +2,7 @@

### 3.0.2 (Next)

* Your contribution here.

This comment has been minimized.

Copy link
@dblock

dblock Mar 24, 2017

Collaborator

Put this back please?

index.js Outdated
@@ -128,6 +128,10 @@ var appServer = function(config) {
});

var endpoint = path.posix.join(normalizedRoot, app.name);
self.express.route(endpoint+'/appid').get(function(req, res){

This comment has been minimized.

Copy link
@dblock

dblock Mar 24, 2017

Collaborator

Something wrong with tabs/spaces here at least.

Now do we really want this with every app server?

@dblock

This comment has been minimized.

Copy link
Collaborator

commented Mar 24, 2017

This seems strange. We rely on a request to figure out the application ID and then remember it? I am confused.

@benedekh

This comment has been minimized.

Copy link
Contributor Author

commented Mar 24, 2017

Yes, because the website uses a separate controller which does not have the app.id registered automatically. The app.id in index.js is not accessible from the views/test.ejs.

Maybe I get something wrong with the controller in test.ejs, but is there a better solution for this problem?

UPDATE: Solution is changed completely. The index.js and views/test.ejb files are restored to their original states and the examples are modified instead.

* different solution for the issue: application id is not exposed on …
…a rest endpoint, but is directly read from the alexa app -> examples are changed accordingly, core functionality is unchanged.
@@ -1,10 +1,14 @@
var alexa = require('alexa-app');
var pkg = require('./package.json');
var applicationId = (typeof pkg.alexa == 'object' ? pkg.alexa.applicationId : "");

This comment has been minimized.

Copy link
@dblock

dblock Mar 27, 2017

Collaborator

Supposedly we know exactly what is in our package.json, doesn't seem like the condition is necessary?

This comment has been minimized.

Copy link
@benedekh

benedekh Mar 27, 2017

Author Contributor

What happens if someone forgets to put the applicationId in the package.json? At least a condition is necessary for the alexa object whether it exists. (I don't know for an alexa-app if it is mandatory/optional to put the applicationId in the package.json either.)

This comment has been minimized.

Copy link
@dblock

dblock Mar 27, 2017

Collaborator

But this is a specific app, with a specific package.json.

@dblock

This comment has been minimized.

Copy link
Collaborator

commented Mar 27, 2017

Ok this is a lot easier. Is this what we have to do for all applications? Maybe something should be added in the README of alexa-app?

@benedekh

This comment has been minimized.

Copy link
Contributor Author

commented Mar 27, 2017

I tested the solution for the sample apps and as long as the applicationId was in the package.json or it was set as an alternative value (instead of an empty string), it worked. I didn't test for production (verify=true) tough. But as I remember in production the GET interface (intent simulator) is not available. Correct me please, if I'm wrong.

README.md Outdated
@@ -217,6 +217,11 @@ AlexaAppServer.start({

Each app (skill) is available at a url endpoint on the server, and responds to POST requests from the Echo. If you load an app's endpoint in your browser with a GET request, it will display an echo simulator that can be used to debug your application. With it, you can send different request types to your app, load slots with values you specify, etc and see the actual generated JSON output from your application.

### Show application ID

To show the application ID in the session correctly, set the `id` field of your alexa-app for that value.

This comment has been minimized.

Copy link
@dblock

dblock Mar 28, 2017

Collaborator

Add an example please.

@dblock

This comment has been minimized.

Copy link
Collaborator

commented Mar 29, 2017

Merged via 6924e90, thanks!

@dblock dblock closed this Mar 29, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.