Real-Dev-Squad / discord-slash-commands

MIT License
5 stars 53 forks source link

[Feature] Add APIs for fetching role information #86

Closed bksh05 closed 1 year ago

bksh05 commented 1 year ago

Issue

There was no API to get the role_id of roles in the server and role_ids were hard coded.

Description (fixes #79 )

This PR adds the below APIs 1: GET /roles: Fetches all role IDs and names. 2: GET /roles/:roleName: Retrieves role details (role_id and role_name) based on the provided role name.

These changes improve the system's ability to handle role-related operations and enhance the overall role-management experience.

Anything you would like to inform the reviewer about:

N.A

Test coverage

File % Stmts % Branch % Funcs % Lines Uncovered Line #s
All files 83.33 86.36 89.28 82.87
responses.ts 100 100 100 100
guildRoleHandler.ts 54.23 76.92 40 54.23 19-30,34-45,50-65
guildRole.ts 90.47 66.66 100 90 37-40,58-61
RitikJaiswal75 commented 1 year ago

Please add the coverage report that you get after running npm test rather than adding the code

bksh05 commented 1 year ago

Please add the coverage report that you get after running npm test rather than adding the code

@RitikJaiswal75 Last time I added a coverage report, you asked to show if the code that I wrote is covered or not 😂. Hence, this time I attached the code showing that whatever I wrote in this PR is 100% covered by test cases.

I would really appreciate it if we have a consensus on what is expected to attach in the PR 🤝. Though I am adding what I get after running npm test.

File % Stmts % Branch % Funcs % Lines Uncovered Line
All files 83.33 85.71 89.28 82.87
config 100 100 100 100
config.ts 100 100 100 100
src/constants 100 100 100 100
commands.ts 100 100 100 100
responses.ts 100 100 100 100
urls.ts 100 100 100 100
src/controllers 59.49 72.72 62.5 59.49
commandNotFound.ts 100 100 100 100
guildRoleHandler.ts 55.73 72.72 40 55.73 19-30,34-45,50-65
helloCommand.ts 100 100 100 100
mentionEachUser.ts 54.54 100 100 54.54 20-33
src/utils 95.04 89.47 100 94.79
JsonResponse.ts 100 100 100 100
checkDisplayType.ts 100 100 100 100
discordEphemeralResponse.ts 100 100 100 100
discordResponse.ts 100 100 100 100
filterUsersByRole.ts 100 100 100 100
getCommandName.ts 100 100 100 100
getGuildMemberDetails.ts 100 100 100 100
getMembersInServer.ts 88.88 100 100 87.5 17
guildRole.ts 90 66.66 100 89.47 37-40,58-61
lowerCaseMessageCommand.ts 100 100 100 100
updateNickname.ts 100 100 100 100
tests/fixtures 100 90 100 100
config.ts 100 100 100 100
fixture.ts 100 90 100 100 109

Note: My functions/methods/controllers are covered 100%. There are other functions in the same file whose test cases are not written at all. Hence the coverage is low. We can ask the respective developer to add those or create a separate issue for it.

RitikJaiswal75 commented 1 year ago

@RitikJaiswal75 Last time I added a coverage report, you asked to show if the code that I wrote is covered or not joy. Hence, this time I attached the code showing that whatever I wrote in this PR is 100% covered by test cases.

By that as well I meant the coverage report itself. you can see it shows the uncovered lines in the table that you attached.

I would really appreciate it if we have a consensus on what is expected to attach in the PR 🤝. Though I am adding what I get after running npm test.

we already have this, but some people tend to not understand the exact thing expected from that.

sahsisunny commented 1 year ago

Please add the coverage report that you get after running npm test rather than adding the code

@RitikJaiswal75 Last time I added a coverage report, you asked to show if the code that I wrote is covered or not 😂. Hence, this time I attached the code showing that whatever I wrote in this PR is 100% covered by test cases.

I would really appreciate it if we have a consensus on what is expected to attach in the PR 🤝. Though I am adding what I get after running npm test.

File % Stmts % Branch % Funcs % Lines Uncovered Line All files 83.33 85.71 89.28 82.87
config 100 100 100 100 config.ts 100 100 100 100 src/constants 100 100 100 100 commands.ts 100 100 100 100 responses.ts 100 100 100 100 urls.ts 100 100 100 100 src/controllers 59.49 72.72 62.5 59.49
commandNotFound.ts 100 100 100 100 guildRoleHandler.ts 55.73 72.72 40 55.73 19-30,34-45,50-65 helloCommand.ts 100 100 100 100 mentionEachUser.ts 54.54 100 100 54.54 20-33 src/utils 95.04 89.47 100 94.79
JsonResponse.ts 100 100 100 100 checkDisplayType.ts 100 100 100 100 discordEphemeralResponse.ts 100 100 100 100 discordResponse.ts 100 100 100 100 filterUsersByRole.ts 100 100 100 100 getCommandName.ts 100 100 100 100 getGuildMemberDetails.ts 100 100 100 100 getMembersInServer.ts 88.88 100 100 87.5 17 guildRole.ts 90 66.66 100 89.47 37-40,58-61 lowerCaseMessageCommand.ts 100 100 100 100 updateNickname.ts 100 100 100 100 tests/fixtures 100 90 100 100 config.ts 100 100 100 100 fixture.ts 100 90 100 100 109 Note: My functions/methods/controllers are covered 100%. There are other functions in the same file whose test cases are not written at all. Hence the coverage is low. We can ask the respective developer to add those or create a separate issue for it.

Add test coverage report only those files in which you have made some changes or added, please.

bksh05 commented 1 year ago

I see in test coverage for your 2 test files lines are skipped, please make sure your tests cover every line at least

Hey @Pratiyushkumar, What 2 test files are you talking about? Whatever code I have added is 100% covered. Please tag the line which you think is not covered.

Pratiyushkumar commented 1 year ago

I see in test coverage for your 2 test files lines are skipped, please make sure your tests cover every line at least

Hey @Pratiyushkumar, What 2 test files are you talking about? Whatever code I have added is 100% covered. Please tag the line which you think is not covered.

I am talking about the coverage that you put in the description of the PR image it doesn't give me any idea that your test cases are 100% covered.

Pratiyushkumar commented 1 year ago

Thank you for clearing my doubt, everything looks good to me