Some time ago I ran into a few planner commands that check if the API responses are undefined:
return request
.get(requestOptions)
.then((response: any) => {
const etag: string | undefined = response ? response['@odata.etag'] : undefined;
if (!etag) {
return Promise.reject(`Error fetching task details`);
}
return Promise.resolve(etag);
});This is accompanied by a test in the testfile:
it('fails validation when task details endpoint fails', (done) => {
sinonUtil.restore(request.get);
sinon.stub(request, 'get').callsFake((opts) => {
if (opts.url === `https://graph.microsoft.com/v1.0/planner/tasks/Z-RLQGfppU6H3663DBzfs5gAMD3o/details`) {
return Promise.resolve(undefined);
}
return Promise.reject('Invalid request');
});
// etc
});I wonder if there is some specific reason why this coded this way. If an endpoint fails, it probably does not return HTTP 200, and henceforth this checks and testcases are superfluous and can be removed.
The places where I noticed this are:
task addtask reference addtask reference removetask set
and also in serviceannouncement health get.
I'd think we should refactor this, or it should be clearly stated in a comment that this behavior was observed.
Some time ago I ran into a few planner commands that check if the API responses are undefined:
This is accompanied by a test in the testfile:
I wonder if there is some specific reason why this coded this way. If an endpoint fails, it probably does not return HTTP 200, and henceforth this checks and testcases are superfluous and can be removed.
The places where I noticed this are:
task addtask reference addtask reference removetask setand also in
serviceannouncement health get.I'd think we should refactor this, or it should be clearly stated in a comment that this behavior was observed.