Fix #1409 installerOptions.failure() options can be undefined #1410
Conversation
| @@ -566,7 +569,6 @@ describe('OAuth', async () => { | |||
| }, | |||
| failure: async (error, installOptions, req, res) => { | |||
| assert.fail(error.message); | |||
| res.send('failure'); | |||
| @@ -588,7 +590,6 @@ describe('OAuth', async () => { | |||
| }, | |||
| failure: async (error, installOptions, req, res) => { | |||
| assert.fail(error.message); | |||
| res.send('failure'); | |||
| // Call the failure callback | ||
| if (options !== undefined && options.failure !== undefined) { | ||
| this.logger.debug('calling passed in options.failure'); | ||
| options.failure(error, installOptions!, req, res); |
There was a problem hiding this comment.
Removing ! here now compiles!
| @@ -447,6 +447,7 @@ describe('OAuth', async () => { | |||
| assert.fail('should have failed'); | |||
| }, | |||
| failure: async (error, installOptions, req, res) => { | |||
| assert.isDefined(installOptions); | |||
There was a problem hiding this comment.
This fails before the changes in this PR
There was a problem hiding this comment.
May I also suggest updating the test title (here and for the other tests) to reflect this new requirement? Perhaps changing "should call the failure callback..." to "should call the failure callback with installation options..."?
There was a problem hiding this comment.
Great suggestion -- will update the test label before merging this
| @@ -447,6 +447,7 @@ describe('OAuth', async () => { | |||
| assert.fail('should have failed'); | |||
| }, | |||
| failure: async (error, installOptions, req, res) => { | |||
| assert.isDefined(installOptions); | |||
There was a problem hiding this comment.
May I also suggest updating the test title (here and for the other tests) to reflect this new requirement? Perhaps changing "should call the failure callback..." to "should call the failure callback with installation options..."?
Summary
This pull request resolves #1409
Requirements (place an
xin each[ ])