From 99e33a50896628a9ba7ccb881ab6bbe067993603 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Wed, 2 May 2018 14:23:39 -0400 Subject: [PATCH] RatingServiceSubmitNotify: Do not flag as notified on error If the submission failed, we probably want to try again next time around. * src/server/service/RatingServiceSubmitNotify.js (_maybeNotify): Extract logic from `#postProcessRaterData'. Only set notification flag in absence of dapi error. (postProcessRaterData): Use it. * test/server/service/RatingServiceSubmitNotifyTest.js: Update tests accordingly. --- .../service/RatingServiceSubmitNotify.js | 51 ++++++++++++++----- .../service/RatingServiceSubmitNotifyTest.js | 42 +++++++++++---- 2 files changed, 69 insertions(+), 24 deletions(-) diff --git a/src/server/service/RatingServiceSubmitNotify.js b/src/server/service/RatingServiceSubmitNotify.js index c333561..e142836 100644 --- a/src/server/service/RatingServiceSubmitNotify.js +++ b/src/server/service/RatingServiceSubmitNotify.js @@ -35,7 +35,8 @@ const RatingService = require( './RatingService' ); * * Notification status will persist using the provided DAO. The next time * such a notification is requested, it will only occur if the flag is not - * set. + * set. The flag is not set in the event of an error (determined by the + * DataApi; usually an HTTP error). */ module.exports = Trait( 'RatingServiceSubmitNotify' ) .extend( RatingService, @@ -69,7 +70,10 @@ module.exports = Trait( 'RatingServiceSubmitNotify' ) /** * Trigger previously provided DataApi when no results are available * - * Result count is determined by DATA.__prem_avail_count. + * Result count is determined by DATA.__prem_avail_count. If the + * notification is successful (determined by the DataApi), then a + * flag will be set preventing the request from being trigerred for + * subsequent rating data. * * @param {UserRequest} request user request * @param {Object} data rating data returned @@ -88,24 +92,43 @@ module.exports = Trait( 'RatingServiceSubmitNotify' ) if ( avail === 0 ) { - this._getNotifyState( quote_id, notified => - { - if ( notified === true ) - { - return; - } - - this._dapif( request ) - .request( { quote_id: quote_id }, () => {} ); - - this._setNotified( quote_id ); - } ); + this._maybeNotify( quote_id, request ); } this.__super( request, data, actions, program, quote ); }, + /** + * Perform notification if flag has not been set + * + * See #postProcessRaterData for more information. + * + * @param {number} quote_id effective quote/document id + * @param {UserRequest} request user request + * + * @return {undefined} + */ + 'private _maybeNotify'( quote_id, request ) + { + this._getNotifyState( quote_id, notified => + { + if ( notified === true ) + { + return; + } + + // make the request, only setting the notification flag if + // it is successful + this._dapif( request ) + .request( { quote_id: quote_id }, err => + { + err || this._setNotified( quote_id ); + } ); + } ); + }, + + /** * Get value of notification flag * diff --git a/test/server/service/RatingServiceSubmitNotifyTest.js b/test/server/service/RatingServiceSubmitNotifyTest.js index b4f5922..d053755 100644 --- a/test/server/service/RatingServiceSubmitNotifyTest.js +++ b/test/server/service/RatingServiceSubmitNotifyTest.js @@ -48,41 +48,63 @@ const { describe( 'RatingServiceSubmitNotify', () => { [ + // not available; make successful request and save flag { prem_avail_count: [ 0 ], prev_called: false, expected_request: true, + request_err: null, + save: true, }, + // not available; make failing request, don't save flag + { + prem_avail_count: [ 0 ], + prev_called: false, + expected_request: true, + request_err: Error(), + save: false, + }, + // available { prem_avail_count: [ 2 ], prev_called: false, expected_request: false, + request_err: null, + save: false, }, + // this shouldn't happen; ignore all but first index { - // this shouldn't happen; ignore all but first index prem_avail_count: [ 2, 2 ], prev_called: false, expected_request: false, + request_err: null, + save: false, }, - // save as above, but already saved { prem_avail_count: [ 0 ], prev_called: true, expected_request: false, + request_err: null, + save: false, }, + // available; don't make request { prem_avail_count: [ 2 ], prev_called: true, expected_request: false, + request_err: null, + save: false, }, + // this shouldn't happen; ignore all but first index { - // this shouldn't happen; ignore all but first index prem_avail_count: [ 2, 2 ], prev_called: true, expected_request: false, + request_err: null, + save: false, }, - ].forEach( ( { prem_avail_count, expected_request, prev_called }, i ) => + ].forEach( ( expected, i ) => it( `sends request on post process if no premiums (#${i})`, done => { const { @@ -111,6 +133,8 @@ describe( 'RatingServiceSubmitNotify', () => expect( data ).to.deep.equal( { quote_id: quote_id } ); requested = true; + + callback( expected.request_err, null ); }, } )(); @@ -133,7 +157,7 @@ describe( 'RatingServiceSubmitNotify', () => expect( qid ).to.equal( quote_id ); expect( key ).to.equal( 'submitNotified' ); - callback( null, prev_called ); + callback( expected.flag_error, expected.prev_called ); }; dao.setDocumentField = ( qid, key, value, callback ) => @@ -145,17 +169,15 @@ describe( 'RatingServiceSubmitNotify', () => notify_saved = true; }; - stub_rate_data.__prem_avail_count = prem_avail_count; + stub_rate_data.__prem_avail_count = expected.prem_avail_count; sut.request( request, response, quote, 'something', () => { - expect( requested ).to.equal( expected_request ); + expect( requested ).to.equal( expected.expected_request ); expect( save_called ).to.be.true; // only save notification status if we're notifying - expect( notify_saved ).to.equal( - !prev_called && expected_request - ); + expect( notify_saved ).to.equal( expected.save ); done(); } );