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

modify non-handler methods to not write a response #122

Merged
merged 2 commits into from
Sep 1, 2023

Conversation

ewollesen
Copy link
Contributor

In prepping the send invitation endpoint for care partner alerting, it has become necessary to have a new endpoint that won't error immediately when an invitee already has an accepted invitation from the invitor.

Since checking for duplicate invitations is firmly in the realm of business logic, this separation also makes sense, as keeping transport logic (HTTP-related things) separate from business logic makes for easier understanding and simpler tests too.

Part of BACK-2500

@ewollesen ewollesen force-pushed the eric/only-handlers-write branch 2 times, most recently from 03f6a85 to f36d90d Compare August 31, 2023 21:31
@ewollesen ewollesen changed the title modify checkForDuplicateInvite to not write a response modify non-handler methods to not write a response Aug 31, 2023
@tjotala tjotala self-requested a review September 1, 2023 19:36
tjotala
tjotala previously approved these changes Sep 1, 2023
Copy link
Member

@tjotala tjotala left a comment

Choose a reason for hiding this comment

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

LGTM, just curiosity question

api/invite.go Outdated
a.logger.Infof("invited [%s] user is already a member of the care team of %v", ib.Email, invitorID)
log.Println(statusExistingMemberMessage)
Copy link
Member

Choose a reason for hiding this comment

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

Just curious: any reason to bring this log.Println to the top-level code instead of leaving in checkAccountAlready...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope, overzealous cut & paste. I'll fix really quick.

In prepping the send invitation endpoint for care partner alerting, it has
become necessary to have a new endpoint that won't error immediately when an
invitee already has an accepted invitation from the invitor.

Since checking for duplicate invitations is firmly in the realm of business
logic, this separation also makes sense, as keeping transport
logic (HTTP-related things) separate from business logic makes for easier
understanding and simpler tests too.

Part of BACK-2500
In prepping the send invitation endpoint for care partner alerting, it has
become necessary to have a new endpoint that won't error immediately when an
invitee already has an accepted invitation from the invitor.

Since checking for Care Team membership is firmly in the realm of business
logic, this separation also makes sense, as keeping transport
logic (HTTP-related things) separate from business logic makes for easier
understanding and simpler tests too.

Part of BACK-2500
@ewollesen
Copy link
Contributor Author

Heh, had to force push to prevent that ugly merge commit. :D

Copy link
Member

@tjotala tjotala left a comment

Choose a reason for hiding this comment

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

LGTM, still :)

@ewollesen ewollesen merged commit 6116fb2 into master Sep 1, 2023
3 checks passed
@ewollesen ewollesen deleted the eric/only-handlers-write branch September 1, 2023 20:23
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