-
Notifications
You must be signed in to change notification settings - Fork 0
Gr donations service domain dtos #17
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
Conversation
- Remove domain DTO files --- now going to use controller's DTO files for consistency and to prevent confusion. - Update service to use DTOs with flat structure (not using stored) - Fix enum references to use PascalCase (DonationType.ONE_TIME) - Move test file to standard location alongside service --- this was my bad and was an inconsistency in my ticket writing. - Update test expectations to match flat DTO structure - Add missing entity fields to test mocks - Fix linting issues (remove unused imports) All tests passing (50/50). Service now returns correct DTO structures.
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.
My changes:
I made these changes since we needed to keep the codebase clean, consistent and follow best practices. I did remove the DTO files that were previously assigned. They were needed for the service file to be made whilst we were collaborating in parallel, but now that we have two DTOs (for controller and service) it's better to just keep one to avoid confusion. It was nothing done wrong on your part!
- Remove domain DTO files --- now going to use controller's DTO files for consistency and to prevent confusion.
- Update service to use DTOs with flat structure (not using stored)
- Fix enum references to use PascalCase (DonationType.ONE_TIME)
- Move test file to standard location alongside service --- this was my bad and was an inconsistency in my ticket writing.
- Update test expectations to match flat DTO structure
- Add missing entity fields to test mocks
- Fix linting issues (remove unused imports)
stored: my recommendation is to not use it for this because it adds unnecessary complexity and makes the API inconsistent.
Changes requested:
- Remove email validation from service - since DTO now has
@isEmail()decorator - Fix findPublic() logic - Should return ALL donations with privacy applied, not just ones with public dedications. What this function does is it returns public facing data (we'd remove sensitive info like emails) but based on whether showDedicationPublicly is checked or not we'd choose whether to include that.
Example scenario:
Donation A: Anonymous, no dedication → Should show with hidden name
Donation B: Public donor, private dedication → Should show name but hide message
Donation C: Public donor, public dedication → Show everything
Current code only returns Donation C. Should return all three with appropriate privacy filters
Here's a more specific example for our codebase:

:
- rewrite getTotalDonations() - Use SQL SUM()/COUNT() instead of fetching all data and using .reduce(). this is important for performance so all the calculations can be done in the db instead.
- Add dedication message validation - Can't show dedication publicly without a message
I know it's a lot - please feel free to reach out for clarification!
57df9c4 to
6ce1bd9
Compare
…-module' into gr-donations-service-domain-dtos with modifications.
Changes Made (merged service and controller branches)1. Service Layer (
|
ℹ️ Issue
Closes #9
📝 Description
Added create, findall, findone, findpublic, and get total donations service functions.
Input validation in create function (negative donation amount, invalid email, etc.)
✔️ Verification
Added unit tests for service functions with mock repository.
🏕️ (Optional) Future Work / Notes
N/A