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

41 update get post and put endpoints for donateditem with full program and donor details #49

Conversation

saikiran0405
Copy link
Collaborator

**Fixes #41 **

What was changed?

  • Updated the GET, POST, and PUT endpoints for managing Donated Items to handle new relationships involving Program and Donor entities.
  • Written unit tests for each endpoint according to the requirement.

Why was it changed?

  • Previously, Program and Donor Email were stored directly as strings, but they have now been replaced by foreign keys to the Program and Donor tables. The endpoints need to be updated to ensure the full Program and Donor details are properly fetched, stored, and updated

How was it changed?

  • Previously, Program and Donor Email were directly stored and returned. Now that these columns have been replaced with foreign keys (programId and donorId):
  • Previously, Program name and Donor email were stored directly as they were received from the request.

Now, the process should be updated by using validations:

Program Validation: Validated the received Program name against the Program table and retrieve its Program ID.

Donor Validation: Validated the received Donor email against the Donor table and retrieve its Donor ID.

Copy link
Collaborator

@truffer11 truffer11 left a comment

Choose a reason for hiding this comment

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

Once again, really solid work! Not too many notes for this sprint, just a few comments that may have been repetitive code. But those are simple changes/checks. Keep up the good work!


});

it('handles errors when the provided Program or Donor does not exist', async () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

In the long term, will this unit test be able to work going into the future? Especially when we get to a point where we definitely have more than 30 users? I personally would consider making it a certain amount higher than the amount of users/programs we have.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@truffer11 It wouldn't check in the real or actual database; it will use mock prisma client and you can see at this line that it resolves to value null and it always resolves to null values irrespective of the size of database

mockPrismaClient.program.findUnique.mockResolvedValue(null);
mockPrismaClient.donor.findUnique.mockResolvedValue(null);

const donorId = 19;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Similar to what I mentioned above, you may want to consider modifying your testing to work beyond just 20 total donors

@@ -21,6 +21,7 @@ router.get('/', async (req: Request, res: Response) => {
const donors = await prisma.donor.findMany();
res.json(donors);
} catch (error) {
console.error('Error fetching donor:', error);
res.status(500).json({ message: 'Error fetching donors' });
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am unable to test what both of these do, but do both of these do different things? At a glance, they seem to have the same functionality

app.use(express.json());
app.use('/donatedItem', donatedItemRoutes);

describe('DonatedItem API Tests', () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

These tests look really good and pretty extensive, good job!

programId: 1,
donorId: 1,
dateDonated: new Date().toISOString()
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

what data is this? can we move this to other relevant mock data files?

mockPrismaClient.donor.findUnique.mockResolvedValue(newDonor);
mockPrismaClient.donatedItem.create.mockResolvedValue({ id: 1, ...newItem });
mockPrismaClient.donatedItem.update.mockResolvedValue({ id: 1, ...updateData });
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice! Keeping the required mock resolved values for prisma at one place seems more clear

@Anjali0407-git Anjali0407-git merged commit cebacf3 into main Oct 16, 2024
3 checks passed
@Anjali0407-git Anjali0407-git deleted the 41-update-get-post-and-put-endpoints-for-donateditem-with-full-program-and-donor-details branch October 16, 2024 20:53
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.

Update GET, POST, and PUT Endpoints for /donatedItem with Full Program and Donor Details
4 participants