-
Notifications
You must be signed in to change notification settings - Fork 0
Feat/platform for organization #173
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
// set DEFAULT_ORGANIZATION | ||
organization.DEFAULT_ORGANIZATION = orgEntity | ||
|
||
email := environment_variables.EnvironmentVariables.ORGANIZATION_ADMIN_EMAIL |
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.
Deploy with admin's email
err = s.organizationService.AddMember(ctx, &organization.OrganizationMember{ | ||
UserID: user.ID, | ||
UserID: admin.ID, | ||
OrganizationID: orgEntity.ID, | ||
Role: organization.OrganizationMemberRoleOwner, | ||
IsPrimary: true, | ||
}) |
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.
Add member -> update role on conflict.
_, ok = auth.GetAdminOrganizationMemberFromContext(reqCtx) | ||
if !ok { | ||
projectFilter.MemberID = &user.ID | ||
} |
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.
admin can view all projects.
end-users can view projects if they are on the project member list.
authService *auth.AuthService | ||
} | ||
|
||
func (d *DataInitializer) Install(ctx context.Context) error { |
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.
Add data on startup.
Can users be members of more than 2 organizations? as I understand, users is tightly with one organization now. As the auth service returns default organization now |
} | ||
} | ||
|
||
var DEFAULT_ORGANIZATION *Organization |
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.
is that thread safe :|
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.
No, definitely not, but it will be loaded only when the server starts.
Let me try using the mechanism once
.
return nil, nil | ||
} | ||
if len(projectEntities) != 1 { | ||
return nil, err |
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.
should return specific errors "cannot greater than 1"
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.
FindOne(ctx context.Context, filter ProjectFilter) (*Project, error)
- returns a pointer to indicate the possibility of no records found.
In the usecase would be simple to understand like:
if err != nil {
// it must be internal error
return
}
if project == nil {
create a project here
}
instead of
import a specific error from somewhere
if err != nil {
if !errors.Is(err, recordNotFoundErr) {
create ...
} else {
abort and return
}
}
WDYT?
string(organization.OrganizationMemberRoleReader): true, | ||
} | ||
|
||
func (s *AuthService) getOrganizationMember(reqCtx *gin.Context) (*organization.OrganizationMember, bool) { |
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.
should be GetDefaultOrganizationMember
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.
It's a private function for internal use, but I understand your point.
-> getDefaultOrganizationMember
return membership, true | ||
} | ||
|
||
func (s *AuthService) OrganizationMemberOptionalMiddleware() gin.HandlerFunc { |
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.
same here, with default
if len(memberEntities) == 0 { | ||
return nil, nil | ||
} | ||
if len(memberEntities) != 1 { |
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.
should introduce new error here
}) | ||
return | ||
} | ||
if exists != nil { |
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.
should check Name is change or not before update, We won't update user every login
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.
It's a trade-off: if we add fields to the user, it's harder to notice that we need to add the new field comparison here.
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.
let's go
I just want to keep the organization table for now, as we may change the design. |
Uh oh!
There was an error while loading. Please reload this page.