Skip to content

Conversation

@albertcui
Copy link
Member

No description provided.

});
}

function sendNotifications(match) {
Copy link
Member

Choose a reason for hiding this comment

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

you probably have to check the origin of the parse request and do this for scanner only (similar to what was done for scenarios), otherwise, people requesting parses might cause multiple notifications?

Copy link
Member Author

Choose a reason for hiding this comment

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

code pointer?

Copy link
Member

Choose a reason for hiding this comment

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

https://github.com/odota/core/blob/master/svc/parser.js#L39

you could add something like doNotifications as a property on the match, or just check directly

return next();
});

notifications.route('/')
Copy link
Member

Choose a reason for hiding this comment

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

what is this route used for

Copy link
Member Author

Choose a reason for hiding this comment

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

It's used to store the notification token on the server so that we know who to send notifications to.

console.error(err.stack || err);
} else {
console.log('completed parse of match %s', match.match_id);
sendNotifications(match);
Copy link
Member

Choose a reason for hiding this comment

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

I think we should make this only enabled if a config option is set. probably there are people using this that don't need this feature

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants