Fix Discord & Google Calendar UUIDs #439
Conversation
nik-dange
requested review from
sumeet-bansal,
shravanhariharan2 and
dowhep
as code owners
|
Thanks for contributing! |
models/EventModel.ts
Outdated
| @Column('uuid', { nullable: true }) | ||
| discordEvent: Uuid; | ||
| @Column('varchar', { nullable: true }) | ||
| discordEvent: string; |
There was a problem hiding this comment.
In types/index.ts, we define Uuid with type Uuid = string and we just use it for clarification on the context we use it. As a result, I think it is better to leave Uuid as type here as well
There was a problem hiding this comment.
So to clarify, the column type can be varchar, but it should be discordEvent: Uuid?
| @@ -41,8 +41,8 @@ export class EventFactory { | |||
| deleted: false, | |||
| eventLink: faker.internet.url(), | |||
| thumbnail: FactoryUtils.getRandomImageUrl(), | |||
| discordEvent: uuid(), | |||
| googleCalendarEvent: uuid(), | |||
| discordEvent: faker.datatype.hexaDecimal(10), | |||
There was a problem hiding this comment.
Is this the way discord generate Uuid or is this just random string?
There was a problem hiding this comment.
This is just a random string. Discord UUIDs are different (see this error message: https://discord.com/channels/573028991527550986/1012204566122598431/1236042800043397261) than what Postgres allows for UUIDs (https://www.postgresql.org/docs/current/datatype-uuid.html), which is why I opted to make it a string
types/ApiRequests.ts
Outdated
| @@ -155,8 +155,8 @@ export interface OptionalEventProperties { | |||
| eventLink?: string; | |||
| requiresStaff?: boolean; | |||
| staffPointBonus?: number; | |||
| discordEvent?: Uuid; | |||
| googleCalendarEvent?: Uuid; | |||
| discordEvent?: string; | |||
There was a problem hiding this comment.
Same with the Uuid comment above
…into bug/discord-uuid-column merging from master
Description
What changes did you make? List all distinct problems that this PR addresses. Explain any relevant
motivation or context.
Made Discord UUIDs and Google Calendar UUIDs strings because they aren't V4 UUIDs
Changes
Type of Change
expected)
workflows, linting, etc.)
If you've selected Patch, Minor, or Major as your change type, make sure to bump the version before merging in
package.json!Testing
I have tested that my changes fully resolve the linked issue ...
Checklist
package.jsonfile.Screenshots
Please include a screenshot of your Postman testing passing successfully.