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

Commit to meal request mutation #105

Merged
merged 12 commits into from
Dec 23, 2023
Merged

Commit to meal request mutation #105

merged 12 commits into from
Dec 23, 2023

Conversation

williamhpark
Copy link
Contributor

@williamhpark williamhpark commented Dec 6, 2023

Notion Ticket Link

https://www.notion.so/uwblueprintexecs/Create-commitToMealRequests-mutation-8c569bebbc1f4f54ad19bdc79efdaff5?pvs=4

Implementation Description

  • Changed the meal_description field in the DonationInfo model to food_description, to match the term used in the UI.
  • Used an enum for the user info roles and meal statuses, to improve readability and type checking.
  • Defined a commitToMealRequest mutation that allows a Donor to commit to one or multiple meal requests at a time.
  • Defined tests for the commitToMealRequest mutation, both happy path and important error cases

Steps To Test

  1. Test the commitToMealRequest mutation. For donorId, use a User document that has info.role = Donor (I used 652d73501b41877577c18eb4). For mealRequestIds, use a meal_request ID or IDs for documents with status = Open (I used 64da70f493361369d40aab2d). Example mutation provided below:
mutation testCommitToMealRequest {
      commitToMealRequest(
        donorId: "652d73501b41877577c18eb4",
        mealRequestIds: ["64da70f493361369d40aab2d"],
        foodDescription: "Pizza",
        additionalInfo: "No nuts"
      )
      {
        mealRequests {
          id
          requestor {
            id
          }
          description
          status
          dropOffDatetime
          dropOffLocation
          mealInfo {
            portions
            dietaryRestrictions
            mealSuggestions
          }
          onsiteStaff {
            name
            email
            phone
          }
          dateCreated
          dateUpdated
          deliveryInstructions
          donationInfo {
            donor {
              id
            }
            commitmentDate
            foodDescription
            additionalInfo
          }
        }
      }
    }
  • The mutation should succeed and the meal request information should be returned.
  • The meal request status should be changed to Fulfilled, and the donationInfo should be populated or updated.
  • The donation_info.commitment_date should be changed to today's date.
  1. Run the automated tests: make betest
  • All tests should pass.

What Should Reviewers Focus On?

  • Focus on the logic in backend/app/services/implementations/meal_request_service.py, since this is where the meal request document is updated and error handling is done.

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 added tests for my changes
  • 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

@williamhpark williamhpark changed the title Wpark/commit meal request Commit to meal request mutation Dec 6, 2023
Copy link
Contributor

@shahanneda shahanneda left a comment

Choose a reason for hiding this comment

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

Hey William, this looks great, thanks for working on it and also writing great tests!

I just went ahead and made a tiny change, replacing "donor_id" to requester in the graphql mutation, this is so its consistent with the other requests, since the plan is to later on put a check higher up that makes sure that the whatever user is put as the "requester" is indeed the user logged in for security reasons, and that'll be easier to do if the name of the field is consistent across all the mutations & queries.

Though I do see the value of calling it "donor_id", since in theory the admin could be the one calling the endpoint, but we don't have any plans to allow the admin to do this, so in this case I think having it as "requester" is fine.

Feel free to merge when you want!

@shahanneda shahanneda merged commit e80bd10 into main Dec 23, 2023
2 checks passed
@shahanneda shahanneda deleted the wpark/commit-meal-request branch December 23, 2023 03:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants