Skip to content

Conversation

jjchen01
Copy link
Collaborator

Implement endpoint for /invites

})

if len((projectIDs)) > 0 {
projects, err := api.projectService.Find(ctx, project.ProjectFilter{
Copy link
Contributor

Choose a reason for hiding this comment

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

double (( and ))

})
return
}
if len(projects) != len((projectIDs)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

double (( and ))

if publicID == "" {
reqCtx.AbortWithStatusJSON(http.StatusBadRequest, responses.ErrorResponse{
Code: "5cbdb58e-6228-4d9a-9893-7f744608a9e8",
Error: "missing project public ID",
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's invite public id

if err != nil || inviteEntity == nil {
reqCtx.AbortWithStatusJSON(http.StatusNotFound, responses.ErrorResponse{
Code: "121ef112-cb39-4235-9500-b116adb69984",
Error: "proj not found",
Copy link
Contributor

Choose a reason for hiding this comment

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

and invite not found too

}

func (s *InviteService) SendInviteEmail(ctx context.Context, e EmailMetadata, to string) error {
templateString := `<html><body><div style="font-family: Arial, sans-serif; max-width: 600px; margin: auto; border: 1px solid #ddd; border-radius: 8px; overflow: hidden;">
Copy link
Contributor

Choose a reason for hiding this comment

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

should we move it to template file some where.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We should move it when we have the second template. (next time)


err := api.inviteService.DeleteInviteByID(ctx, inviteEntity.ID)
if err != nil {
reqCtx.JSON(http.StatusNotFound, responses.ErrorResponse{
Copy link
Contributor

Choose a reason for hiding this comment

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

it should be error as the not found should from auth.GetAdminInviteFromContext(reqCtx)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's likely an internal server error because auth.GetAdminInviteFromContext(reqCtx) will return "not found".

headers += "MIME-Version: 1.0\r\n"
headers += "Content-Type: text/html; charset=\"UTF-8\"\r\n"
headers += "From: " + envs.SMTP_SENDER_EMAIL + "\r\n"
headers += "To: " + to + "\r\n"
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we support multiple emails sending or email validation here. It may dangerous without mail address validation

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Email validation (with allow/block lists and spam detection) is too difficult to handle at this moment.
If we support sending multiple emails, we will introduce a message queue and a task dispatcher to handle it offline. This is overkill at the moment.
(Another excuse to implement it next time)

@locnguyen1986
Copy link
Contributor

Almost are minor comment.
just want to confirm one logic, should the organization mapped with domain so all users with same domain can join organization.
nvm, it's too complication now, let they do it manually :D
lgtm

@locnguyen1986
Copy link
Contributor

pls resolve the conflict and feel free to merge

@jjchen01 jjchen01 merged commit e758a01 into main Sep 17, 2025
1 check passed
@jjchen01 jjchen01 deleted the feat/project-member branch September 17, 2025 06:29
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