-
Notifications
You must be signed in to change notification settings - Fork 669
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
NOISSUE: Listing of shared things with users & Update SDK #1923
Conversation
* fix bugs Signed-off-by: Arvindh <arvindh91@gmail.com> * fix bugs Signed-off-by: Arvindh <arvindh91@gmail.com> --------- Signed-off-by: Arvindh <arvindh91@gmail.com> Signed-off-by: dusanb94 <dusan.borovcanin@mainflux.com>
* fix bugs Signed-off-by: Arvindh <arvindh91@gmail.com> * fix bugs Signed-off-by: Arvindh <arvindh91@gmail.com> * fix list of things in a channel and Add connect disconnect endpoint Signed-off-by: Arvindh <arvindh91@gmail.com> * fix list of things in a channel and Add connect disconnect endpoint Signed-off-by: Arvindh <arvindh91@gmail.com> --------- Signed-off-by: Arvindh <arvindh91@gmail.com> Signed-off-by: dusanb94 <dusan.borovcanin@mainflux.com>
* fix list of things in a channel and Add connect disconnect endpoint Signed-off-by: Arvindh <arvindh91@gmail.com> * add: things share with other users Signed-off-by: Arvindh <arvindh91@gmail.com> --------- Signed-off-by: Arvindh <arvindh91@gmail.com> Signed-off-by: dusanb94 <dusan.borovcanin@mainflux.com>
* add: listing of channels, users, groups, things Signed-off-by: Arvindh <arvindh91@gmail.com> * add: listing of channels, users, groups, things Signed-off-by: Arvindh <arvindh91@gmail.com> * add: listing of channels, users, groups, things Signed-off-by: Arvindh <arvindh91@gmail.com> * add: listing of channels, users, groups, things Signed-off-by: Arvindh <arvindh91@gmail.com> --------- Signed-off-by: Arvindh <arvindh91@gmail.com> Signed-off-by: dusanb94 <dusan.borovcanin@mainflux.com>
* removed repeating code in list groups Signed-off-by: Arvindh <arvindh91@gmail.com> * add: list of user group Signed-off-by: Arvindh <arvindh91@gmail.com> * fix: otel handler operator name for endpoints Signed-off-by: Arvindh <arvindh91@gmail.com> --------- Signed-off-by: Arvindh <arvindh91@gmail.com> Signed-off-by: dusanb94 <dusan.borovcanin@mainflux.com>
Signed-off-by: Arvindh <arvindh91@gmail.com>
Signed-off-by: Arvindh <arvindh91@gmail.com>
Signed-off-by: Arvindh <arvindh91@gmail.com>
Signed-off-by: Arvindh <arvindh91@gmail.com>
users/api/endpoints.go
Outdated
func listMembersByChannelEndpoint(svc users.Service) endpoint.Endpoint { | ||
return func(ctx context.Context, request interface{}) (interface{}, error) { | ||
req := request.(listMembersByObjectReq) | ||
// in spiceDB channel is group kind |
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.
Start comment with a capital letter. Also, capitalize SpiceDB. What s "group kind" - is kind some tem from SpiceDB?
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.
We not having any separate type
for channel in spiceDB schema, Instead we using group
type for storing channel relations.
Here the function name is listMembersByChannelEndpoint
, which does listing of users in a channel
This function request we are passing req.objectKind = "groups"
,
This function is related to channels, then Why we here passing groups
instead of channels
? Like this kind of question will raise when someone see the code.
Answer is same as said in the first, We are not having separate type
for channel
in spiceDB schema, Instead we using same group
type for channel.
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.
updated comment
Signed-off-by: Arvindh <arvindh91@gmail.com>
pkg/sdk/go/groups.go
Outdated
return sdkerr | ||
} | ||
|
||
func (sdk mfSDK) RemoveUsersToGroup(groupID string, req UsersRelationRequest, token string) errors.SDKError { |
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.
RemoveUserFromGroup
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.
Changed in all instance
pkg/sdk/go/groups.go
Outdated
@@ -164,6 +145,64 @@ func (sdk mfSDK) DisableGroup(id, token string) (Group, errors.SDKError) { | |||
return sdk.changeGroupStatus(id, disableEndpoint, token) | |||
} | |||
|
|||
func (sdk mfSDK) AddUsersToGroup(groupID string, req UsersRelationRequest, token string) errors.SDKError { |
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.
AddUserToGroup (a single user)
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.
Changed in all instance
Signed-off-by: Arvindh <arvindh91@gmail.com>
Signed-off-by: Arvindh <arvindh91@gmail.com>
Signed-off-by: Arvindh <arvindh91@gmail.com>
Signed-off-by: Arvindh <arvindh91@gmail.com>
@@ -217,7 +217,9 @@ func (svc service) ListGroups(ctx context.Context, token string, memberKind, mem | |||
} | |||
|
|||
if len(ids) <= 0 { | |||
return groups.Page{}, errors.ErrNotFound | |||
return groups.Page{ |
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.
Why do we even consider less than 0 for IDs length (i.e. why not if len(ids) == 0
)?
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.
Since len() returns int
things/service.go
Outdated
} | ||
} | ||
|
||
if len(ids) <= 0 { |
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.
The same 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.
Since len() returns int
Signed-off-by: Arvindh <arvindh91@gmail.com>
Signed-off-by: Arvindh <arvindh91@gmail.com>
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.
LGTM
Pull request title should be
MF-XXX - description
orNOISSUE - description
where XXX is ID of issue that this PR relate to.Please review the CONTRIBUTING.md file for detailed contributing guidelines.
What does this do?
Which issue(s) does this PR fix/relate to?
Put here
Resolves #XXX
to auto-close the issue that your PR fixes (if such)List any changes that modify/break current functionality
Have you included tests for your changes?
Did you document any new/modified functionality?
Notes