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 crud endpoints #17

Merged
merged 12 commits into from
Oct 30, 2021
Merged

Conversation

annechung
Copy link
Contributor

@annechung annechung commented Oct 14, 2021

Notion ticket link

Ticket Name

Implementation description

  • GET/POST/PUT/DELETE endpoints for the scheduling model

Steps to test

  1. Create user and donor rows in their respective tables.
  2. Send requests to http://localhost:5000/scheduling and test out functionality!

Sample JSON for creating a scheduling entry (POST):

{
    "donorId": 1,
    "category": "Tea and coffee",
    "quantity": 3,
    "pickupLocation": "anywhere",
    "startTime": "2020-10-13",
    "endTime": "2020-10-14",
    "status": "Pending",
    "volunteersNeeded": 0
}

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 backend Backend change in progress labels Oct 14, 2021
@annechung annechung requested a review from Shehryar21 October 14, 2021 05:10
@annechung annechung changed the title feat: scheduling crud feat: scheduling crud endpoints Oct 14, 2021
@annechung annechung changed the title feat: scheduling crud endpoints feat: create scheduling crud endpoints Oct 14, 2021
@annechung annechung force-pushed the feature/scheduling-crud branch from fae2668 to e587856 Compare October 14, 2021 05:33
Copy link
Contributor

@Shehryar21 Shehryar21 left a comment

Choose a reason for hiding this comment

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

First Pass. I havent tested it through postman yet, but will do once the changes are implemented. Good job on it though! Also the tests are failing in CI. Can you take a look at that?

@Column({ type: DataType.DATE })
end_time!: Date;

@AllowNull(false)
@Column({ type: DataType.ENUM("Rejected", "Approved", "Pending") })
status!: Status;
Copy link
Contributor

Choose a reason for hiding this comment

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

Default value should be Approved. Can we add that in? We would not need @AllowNull(false) then? This would require some changes in the functions as well.

Copy link
Contributor

@linnall linnall Oct 21, 2021

Choose a reason for hiding this comment

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

Wouldn't the default value be Pending since when a schedule is created, it then has to be approved by an admin?

@Column({ type: DataType.INTEGER })
donor_id!: string;
id!: number;

@Column({ type: DataType.TEXT })
description!: string | null;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we change this to category instead? Please look at the following product spec: https://www.notion.so/uwblueprintexecs/Creating-Info-for-Food-Donor-Dropoff-60826a71e0114f81a837e6933922a4dd#e85ab7e5abbc42e09ac8327001f9c65d for different categories. I guess we could still keep it as a string instead of Enum, and the Frontend would only provide those listed options in the dropdown. In that case, only need to change the name.

@annechung
Copy link
Contributor Author

Think we've addressed all comments! @Shehryar21

Copy link
Member

@hanlinc27 hanlinc27 left a comment

Choose a reason for hiding this comment

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

haven't tested locally yet but looks good!

@annechung annechung merged commit 62742db into main Oct 30, 2021
@annechung annechung deleted the feature/scheduling-crud branch October 30, 2021 20:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants