Improve request progress ID check #266
Conversation
| @@ -12,7 +12,7 @@ export default class InternalProgressEventHandler implements EventHandler<'messa | |||
| private logger = log4js.getLogger( 'InternalProgressEventHandler' ); | |||
|
|
|||
| private isValidId( id: string ): id is Snowflake { | |||
| return !!id.match( /[0-9]{18}/ ); | |||
| return /^[0-9]{18,19}$/.test( id ); | |||
There was a problem hiding this comment.
The length limit seems unnecessary and error-prone; could we not just have a + instead?
There was a problem hiding this comment.
I'm wondering why we need this function in the first place, I'd assume discord.js would provide something similar by itself
There was a problem hiding this comment.
Checked the documentation and apparently that isn't a thing. Huh.
There was a problem hiding this comment.
SPGoding: I've kept the minimum 18 digits – is this okay then? Unless something changes greatly in the future of Discord data, the message IDs that show up in the channels will never be 17 digits or less.
violine1101, sort of in reply to your messages: At the moment, if an invalid ID is used here, there is still a try-catch to make sure the bot doesn't crash. However, when doing this, a less informative error message is provided. So no, the function isn't really necessary, but it is if informing the user of an invalid ID is desired.
Co-authored-by: mdashlw <[email protected]>
Purpose
Improve the request progress message ID check. Currently,
100003893070974167Mis accepted as a valid message ID, because the string is simply tested for containing an 18-digit number.Approach
Ensure the message ID takes up the entire string, and allow for 18- and 19-digit message IDs (and above).