Skip to content
This repository has been archived by the owner on Oct 30, 2018. It is now read-only.

Remove graphql, fix isFreeTier issue, fix credit card removal issue #79

Merged
merged 10 commits into from
Sep 12, 2017

Conversation

dylanlott
Copy link
Contributor

@dylanlott dylanlott commented Aug 28, 2017

This PR

  • removes graphQL code and routes
  • adds helmet for security best practices
  • fixes isFreeTier user issues
  • fixes credit card removal issues

addressing issues #64 and #65

@coveralls
Copy link

Coverage Status

Coverage decreased (-11.7%) to 36.12% when pulling 2061844 on dylanlott:remove-graphql into fc8fc75 on Storj:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.9%) to 48.73% when pulling 3ebcbbf on dylanlott:remove-graphql into fc8fc75 on Storj:master.

- use dotenv to handle env variables for local dev
- remove a stripe test key from vc
- remove some comments
@coveralls
Copy link

Coverage Status

Coverage increased (+1.04%) to 48.848% when pulling 3874f1a on dylanlott:remove-graphql into fc8fc75 on Storj:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.9%) to 48.671% when pulling 394e499 on dylanlott:remove-graphql into fc8fc75 on Storj:master.

@dylanlott
Copy link
Contributor Author

dylanlott commented Sep 9, 2017

This pull request also fixes isFreeTier and credit card removal issues.

Copy link
Contributor

@phutchins phutchins left a comment

Choose a reason for hiding this comment

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

Looks pretty good. Just a couple of comments and we can merge.

lib/engine.js Outdated
@@ -40,6 +43,7 @@ Engine.prototype.start = function(callback) {
var self = this;

log.info('starting the billing engine');
log.info(`connecting to mongoose on ${self._config.storage.mongoURI}`);
Copy link
Contributor

Choose a reason for hiding this comment

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

We should make sure that we're not possibly logging credentials here in the Mongo URI.

@@ -41,70 +41,149 @@ PaymentProcessorsRouter.prototype._addPaymentProcessor = function(req) {
});
};

// PaymentProcessorsRouter.prototype._setUserFreeTier = function(req, isFreeTier) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we committing commented code?

- removes commented code
- removes unnecessary log line
@coveralls
Copy link

Coverage Status

Coverage increased (+0.9%) to 48.671% when pulling 712af57 on dylanlott:remove-graphql into fc8fc75 on Storj:master.

@phutchins phutchins changed the title Remove graphql Remove graphql, fix isFreeTier issue, fix credit card removal issue Sep 12, 2017
Copy link
Contributor

@phutchins phutchins 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! Merging.

@phutchins phutchins merged commit 3d1107a into storj-archived:master Sep 12, 2017
@dylanlott dylanlott deleted the remove-graphql branch September 12, 2017 14:55
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants