Skip to content
This repository has been archived by the owner. It is now read-only.

Fix Issues #136 - Access-Control-Allow-Origin #137

Merged
merged 1 commit into from Jul 23, 2014

Conversation

@alicoding
Copy link
Member

@alicoding alicoding commented Jul 22, 2014

Closes #136

@alicoding
Copy link
Member Author

@alicoding alicoding commented Jul 22, 2014

@humphd if you want to r? :)

@mjschranz
Copy link

@mjschranz mjschranz commented Jul 22, 2014

R+ from me

@mjschranz
Copy link

@mjschranz mjschranz commented Jul 22, 2014

Other than your failing test which is probably due to the new types for allowed somaina

@@ -8,8 +8,8 @@ export NODE_ENV="development"
export PORT=9090

# Allowed CORS domains
# Use comma for more domains e.g. http://localhost:5001,http://localhost:7777
export ALLOWED_CORS_DOMAINS="http://localhost:5001"
# Add domain that we allowed in the array

This comment has been minimized.

@humphd

humphd Jul 22, 2014
Member

"Add domains that we allow (e.g., for /api/sync route)"

@humphd
Copy link
Member

@humphd humphd commented Jul 22, 2014

One nit, plus deal with Travis as Matt suggests. Awesome work!

@alicoding
Copy link
Member Author

@alicoding alicoding commented Jul 22, 2014

@humphd that test either need to be skip or remove until I know how to fake req.headers.origin since we are using that.

@@ -3,11 +3,19 @@ var request = require('request');
var util = require('../lib/util');
var env = require('../../server/lib/environment');
var ALLOW_DOMAINS = process.env.ALLOWED_CORS_DOMAINS || env.get("ALLOWED_CORS_DOMAINS");
var firstDomain;

This comment has been minimized.

@sedge

sedge Jul 23, 2014
Contributor

Is this variable used in any other tests? If not, move it down inside the it() block.

@sedge
Copy link
Contributor

@sedge sedge commented Jul 23, 2014

Rebase then r+

@alicoding alicoding merged commit 73d9999 into mozilla:master Jul 23, 2014
1 check was pending
1 check was pending
@alicoding
continuous-integration/travis-ci The Travis CI build is in progress
Details
@alicoding alicoding deleted the alicoding:issues136 branch Jul 23, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants