Raised New PR : (https://github.com/Real-Dev-Squad/discord-slash-commands/pull/98) Added description to the message after using /verify #85 #90
Conversation
shreya-mishra
added
enhancement
|
Where is the test of the changes? |
| "@types/node": "^18.11.18", | ||
| "@types/node-fetch": "^2.6.2", | ||
| "@typescript-eslint/eslint-plugin": "^5.47.1", | ||
| "@typescript-eslint/parser": "^5.47.1", | ||
| "eslint": "^8.31.0", | ||
| "jest": "^29.3.1", | ||
| "jsonwebtoken": "^9.0.1", |
There was a problem hiding this comment.
Why are we installing the jsonwebToken package?
There was a problem hiding this comment.
to mock cloudfareworker jwt
| @@ -0,0 +1,14 @@ | |||
| "use strict"; | |||
| import jsonwebtoken from "jsonwebtoken"; | |||
There was a problem hiding this comment.
why do we need this package? It is not being used anywhere so why are we mocking it?
There was a problem hiding this comment.
yes we did it afterwards
There was a problem hiding this comment.
check in latest commit
src/controllers/verifyCommand.ts
Outdated
|
|
||
| const response = await sendUserDiscordData( | ||
| token, | ||
| const response = await sendUserDiscordData( token, |
There was a problem hiding this comment.
is there too much whitespace before token
There was a problem hiding this comment.
fixed automatically, via eslint
tests/mocks/__mocks__
Outdated
There was a problem hiding this comment.
Please remove this blank file
| @@ -1,3 +1,4 @@ | |||
| const crypto = require("crypto"); | |||
There was a problem hiding this comment.
We should not require crypto as it is available globally from node 18
There was a problem hiding this comment.
without this line it was giving below error
There was a problem hiding this comment.
but now te issue is, this require is not allowed but when I am using import it gives the same error:/
| // const mockResponse = response.INTERNAL_SERVER_ERROR; | ||
| // jest | ||
| // .spyOn(global, "fetch") | ||
| // .mockImplementation(() => | ||
| // Promise.resolve(new JSONResponse(mockResponse)) | ||
| // ); |
There was a problem hiding this comment.
Please remove the commented code
shreya-mishra
changed the title
added |
shreya-mishra
changed the title
Description
after using /verify command, the reply message is showing just a link.
Ticket : Added Description to the message after using /verify
Output: Not adding output as it consist of auth URL!
Adding TestCases of /verify feature !