-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Atendees table db #37
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah! I see you added Go files to run this SQL - which is very reasonable - but to match the existing pattern, I think it's probably easier and more consistant to add the SQL as a .sql
file in the sql
directory! Then everyone will have to run the SQL on their own local environments to create the table on their databases. I have a few lil comments about the create SQL but I'll leave them inline in the code so it's easier to identify what I'm talking about! Let me know if any of this could use additional clarification!
internal/database/atendee_table.go
Outdated
createTableQuery := ` | ||
CREATE TABLE IF NOT EXISTS attendees ( | ||
id INT AUTO_INCREMENT PRIMARY KEY, | ||
event_id INT, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can remove the event_id on this table; the rsvp table will eventually link the attendee to the event (since one attendee could RSVP for multiple different events)!
internal/database/atendee_table.go
Outdated
last_name VARCHAR(255) NOT NULL, | ||
email VARCHAR(255) NOT NULL, | ||
phone_number VARCHAR(20), | ||
image_url VARCHAR(255), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am a fan of short URLs (I work at Bitly, after all 😆 ) but in theory, I think a URL can be up to 2048 characters so just to be safe, but be better to set this to VARCHAR(2048)?
internal/database/atendee_table.go
Outdated
email VARCHAR(255) NOT NULL, | ||
phone_number VARCHAR(20), | ||
image_url VARCHAR(255), | ||
FOREIGN KEY (event_id) REFERENCES events(id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yay foreign keys!! We'll want to remove it in this case because we're removing event_id, but I love the idea of adding it to the RSVP table (for attendee_id when we add that and for event_id as well)!
0787b5c
to
39bb415
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great to me!! Once you merge this, perhaps post in the #sprinterns2025 channel to let people know that they should run your SQL on their local environments!
I added attendee table and fields ( id | event_id | first_name | last_name | email | phone_number | image_url )