-
Notifications
You must be signed in to change notification settings - Fork 5
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
Refactor MerchStoreService + Misc. Bug Fixes #436
Conversation
Thanks for contributing! |
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.
Looks good to me. Kinda just skimmed the changes since its mostly just moving between files and focused on the bug fixes.
this.emailService = emailService; | ||
} | ||
|
||
public async findItemByUuid(uuid: Uuid, user: UserModel): Promise<PublicMerchItemWithPurchaseLimits> { |
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.
Inconsistent external vs internal types, service returning a public type
…into feature/merch-store-refactor merging from master
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.
did not break portal
LGTM 🚀 🚀
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.
Tested locally on Postman - LGTM!
Info
Closes #200.
Description
The MerchStoreService contains logic for both Merch Store functionality as well as Merch Order functionality. Refactoring into a
MerchStoreService
and aMerchOrderService
for ease of development moving forward. Also random bug fixes.Changes
MerchOrderService
Type of Change
expected)
workflows, linting, etc.)
If you've selected Patch, Minor, or Major as your change type, make sure to bump the version before merging in
package.json
!Testing
I have tested that my changes fully resolve the linked issue ...
Checklist
package.json
file.Screenshots
Please include a screenshot of your Postman testing passing successfully.