-
Notifications
You must be signed in to change notification settings - Fork 242
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
feat_: light, no-network versions of permission checking functions #5168
Conversation
Jenkins BuildsClick to see older builds (12)
|
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.
Nice job. Just some questions and aesthetic suggestions
protocol/communities/manager.go
Outdated
viewSatisfied := false | ||
if !hasViewOnlyPermissions { | ||
viewSatisfied = true | ||
} else { | ||
viewSatisfied = (meAsMember != nil && meAsMember.GetChannelRole() == protobuf.CommunityMember_CHANNEL_ROLE_VIEWER) | ||
} |
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.
viewSatisfied := false | |
if !hasViewOnlyPermissions { | |
viewSatisfied = true | |
} else { | |
viewSatisfied = (meAsMember != nil && meAsMember.GetChannelRole() == protobuf.CommunityMember_CHANNEL_ROLE_VIEWER) | |
} | |
viewSatisfied := !hasViewOnlyPermissions || (meAsMember != nil && meAsMember.GetChannelRole() == protobuf.CommunityMember_CHANNEL_ROLE_VIEWER) |
protocol/communities/manager.go
Outdated
postSatisfied := false | ||
if !hasViewAndPostPermissions { | ||
postSatisfied = true | ||
} else { | ||
postSatisfied = (meAsMember != nil && meAsMember.GetChannelRole() == protobuf.CommunityMember_CHANNEL_ROLE_POSTER) | ||
} |
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.
postSatisfied := false | |
if !hasViewAndPostPermissions { | |
postSatisfied = true | |
} else { | |
postSatisfied = (meAsMember != nil && meAsMember.GetChannelRole() == protobuf.CommunityMember_CHANNEL_ROLE_POSTER) | |
} | |
postSatisfied := !hasViewAndPostPermissions || (meAsMember != nil && meAsMember.GetChannelRole() == protobuf.CommunityMember_CHANNEL_ROLE_POSTER) |
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.
I agree with Jo's comments, but otherwise things look good!
8dd311d
to
faa06a3
Compare
@@ -3026,6 +3098,27 @@ func (m *Manager) CheckAllChannelsPermissions(communityID types.HexBytes, addres | |||
return response, nil | |||
} | |||
|
|||
// Light version of CheckAllChannelsPermissionsLight, which does not use network requests. | |||
// Instead of checking wallet balances it checks if there is an access to a member list of chats. | |||
func (m *Manager) CheckAllChannelsPermissionsLight(communityID types.HexBytes) (*CheckAllChannelsPermissionsResponse, 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.
minor: maybe rename it CheckAllChannelsPermissionsOffline?
faa06a3
to
3b0135b
Compare
Light functions are based on the fact that permissions are met if the user is on community/channel member list. If the user is not on the list, permissions will not be met and the client is responsible to use regular permissions-check functions. For all permissions-check functions, corresponding light versions are added: CheckAllChannelsPermissionsLight, CheckChannelPermissionsLight, CheckPermissionToJoinLight. Issue #14220
3b0135b
to
d55e177
Compare
For all permissions-check functions, corresponding light versions are added: CheckAllChannelsPermissionsLight, CheckChannelPermissionsLight, CheckPermissionToJoinLight (we can discuss naming convention).
Light functions are based on the fact that permissions are met if the user is on community/channel member list. If the user is not on the list, permissions will not be met and the client is responsible to use regular permissions-check functions.
Issue #14220