Skip to content
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

feat: create scheduling model + migration #7

Merged
merged 8 commits into from
Oct 2, 2021

Conversation

annechung
Copy link
Contributor

Notion ticket link

Create Scheduling Table

Implementation description

  • created new model file and corresponding migration

Steps to test

  1. run migrations
  2. verify that the scheduling table has been created

What should reviewers focus on?

Checklist

  • My PR name is descriptive and in imperative tense
  • My commit messages are descriptive and in imperative tense. My commits are atomic and trivial commits are squashed or fixup'd into non-trivial commits
  • I have run the appropriate linter(s)
  • I have requested a review from the PL, as well as other devs who have background knowledge on this PR or who will be building on top of this PR

@annechung annechung added the backend Backend change label Sep 25, 2021
@Column({ type: DataType.BOOLEAN })
volunteer_needed!: boolean;

@Column({ type: DataType.INTEGER })
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this the volunteer that is allocated for the specific schedule slot?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if there's multiple volunteers should this be an array instead?

volunteer_id!: number[] | null

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need more than one volunteer for any slots? @Shehryar21

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it wouldn't hurt to support it

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could support that but I am just thinking that then we probably would want to ask how many volunteers needed as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, we could change volunteers_needed to an int, and if volunteers aren't needed this would just be 0

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup that works. Default value would be 0

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated!

Co-authored-by: Hanlin Cheng <h62cheng@uwaterloo.ca>
@annechung annechung changed the title feature: create scheduling model + migration feat: create scheduling model + migration Sep 25, 2021
Co-authored-by: Hanlin Cheng <h62cheng@uwaterloo.ca>
@Column({ type: DataType.BOOLEAN })
volunteer_needed!: boolean;

@Column({ type: DataType.INTEGER })
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup that works. Default value would be 0

@Shehryar21
Copy link
Contributor

Update: We actually do not need the migration files. Local db is updated when we do docker-compose up but for us that wasnt happening because node mailer was not configured and was causing the backend to crash. I will update the team in the next work session so please dont merge it.

volunteers_needed!: number;

@Column({ type: DataType.ARRAY(DataType.INTEGER) })
volunteer_ids!: number[] | null;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once we have a volunteer table, volunteer_id would be a foreign key reference to the volunteer table. I am not sure if we can have a list of foreign key references. We might have to create another table schedule_volunteer_mapping which maps volunteer id to scheduling id in the future. For now, we can probably keep it like this since we dont have a volunteer table, but if you can leave a comment about this, that would be nice.

id!: number;

@Column({ type: DataType.INTEGER })
donor_id!: string;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also leave a comment for now that donor_id would be changed to have a foreign key reference to donors table?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added the comments!

@Shehryar21
Copy link
Contributor

LGTM! I see the table being created.
image

@annechung annechung merged commit 0f2116f into main Oct 2, 2021
@annechung annechung deleted the feature/scheduling-model branch October 2, 2021 18:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend Backend change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants