Hotfix/user level reports

[vikasrohit]

No description provided.

[maxceem]

@vikasrohit I'm not sure how does it work. Here gem.lastCall.args is used but a few lines above on line 250 it's restored by gem.restore().

[vikasrohit]

Yes, that is valid argument. However, I just copied the unit tests we already have for project reports route and updated them to meet my requirements. :)

[vikasrohit]

And strange thing is that, it is working fine. However, I did find one minor issue with unit tests of project reports route, they are still using magic numbers i.e. user ids are used as numbers instead of using them from the variables/constants and I fixed that in my unit tests.

[maxceem]

Hey, @xxcxy could you please help us to clarify this moment how does it work?

first it restores the stub:

gem.restore();

and after it uses it for testing:

const [user, project, member, embedUrl] = gem.lastCall.args;

See initial code d08f9f3#diff-fa4fefc23d4de0d05b580cf93c44207bR206

[maxceem]

I guess it's not critical, but we usually return responses in JSON, including situations when some error happens. In the report endpoints in case of errors responses are returned as plain text. Here are some examples and how they could be rewritten to return JSON.

return res.status(500).send(err.toString());

can be replaced by:

next(err);  // this would set HTTP status of error to `500` by default and format error JSON response properly

When some custom error code should be returned like for this case:

return res.status(403).send('User is not allowed to access the report');

it can be replaced by:

const err = new Error('User is not allowed to access the report');
err.status = 403;
return next(err);

[maxceem]

Other than returning strings instead of JSON in case of errors everything else looks good to me.

[vikasrohit]

Other than returning strings instead of JSON in case of errors everything else looks good to me.

I thought of it but my thought process was because we return resources in response and for this endpoint the resource is actually the URL which is primitive data type instead of object. Still, we can think on it later and can fix it in a combined efforts.

We can fix the error response similarly later. Just curious, are we using that way of sending error responses in all other routes?

[maxceem]

I thought of it but my thought process was because we return resources in response and for this endpoint the resource is actually the URL which is primitive data type instead of object. Still, we can think on it later and can fix it in a combined efforts.

Sure, it's not critical. Just for consistency, it's nice to know that we always get JSON in response, also to comply with Topcoder API V5 standard:

We can fix the error response similarly later. Just curious, are we using that way of sending error responses in all other routes?

Yes, all other routes return JSON in case of errors (at least I don't any which don't).