SMQ-2719 - Update list domain invitations endpoint #2965
Conversation
felixgateru
changed the title
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2965 +/- ##
==========================================
+ Coverage 28.15% 29.05% +0.89%
==========================================
Files 360 30 -330
Lines 56962 6019 -50943
==========================================
- Hits 16040 1749 -14291
+ Misses 40139 4172 -35967
+ Partials 783 98 -685 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
domains/middleware/authorization.go
Outdated
| return am.svc.ListInvitations(ctx, session, page) | ||
| } | ||
|
|
||
| func (am *authorizationMiddleware) ListInviteeInvitations(ctx context.Context, session authn.Session, page domains.InvitationPageMeta) (invs domains.InvitationPage, err error) { |
There was a problem hiding this comment.
I think we can have two function ,
ListInvitations and ListDomainInvitations
ListInvitationslist invitee invitationsListDomainInvitationslist all the invitations sent on domain
No need for CheckAdmin because SuperAdmin inherently get the permission
felixgateru
force-pushed
the
smq2719-invitations
branch
from
de8ad17 to
b34f063
Compare
felixgateru
changed the title
felixgateru
force-pushed
the
smq2719-invitations
branch
from
b34f063 to
f3c11ef
Compare
arvindh123
left a comment
arvindh123
left a comment
There was a problem hiding this comment.
I could not see like this in domainID: chi.URLParam(r, "domainID"), in decoder requests of SendDomainInvitations, ListDomainInvitation
They are obtained from the session since it already fetches them from the url |
domains/api/http/endpoint.go
Outdated
| if !ok { | ||
| return nil, svcerr.ErrAuthorization | ||
| } | ||
| req.InvitationPageMeta.DomainID = session.DomainID |
There was a problem hiding this comment.
Please move this logic to service layer
arvindh123
left a comment
arvindh123
left a comment
There was a problem hiding this comment.
supermq/domains/api/http/endpoint.go
Line 261 in f3c11ef
This might not be needed , Please verify this one
domains/events/events.go
Outdated
| if lie.DomainID != "" { | ||
| val["domain_id"] = lie.DomainID | ||
| } |
There was a problem hiding this comment.
Domain ID is mandatory, It could not be optional
in listInvitationsEvent super admin is not required , instead we can have requested user id
arvindh123
force-pushed
the
smq2719-invitations
branch
from
c774efd to
a1dbf68
Compare
felixgateru
force-pushed
the
smq2719-invitations
branch
from
a1dbf68 to
894b583
Compare
domains/events/events.go
Outdated
| if lie.State.String() != "" { | ||
| val["state"] = lie.State.String() | ||
| } |
There was a problem hiding this comment.
This will never be true. Please check State.
Signed-off-by: Felix Gateru <[email protected]>
Signed-off-by: Felix Gateru <[email protected]>
Signed-off-by: Felix Gateru <[email protected]>
Signed-off-by: Felix Gateru <[email protected]>
Signed-off-by: Felix Gateru <[email protected]>
Signed-off-by: Felix Gateru <[email protected]>
Signed-off-by: Felix Gateru <[email protected]>
Signed-off-by: Felix Gateru <[email protected]>
dborovcanin
force-pushed
the
smq2719-invitations
branch
from
9abd9ff to
dda902c
Compare
Signed-off-by: Felix Gateru <[email protected]>
Signed-off-by: Felix Gateru <[email protected]>
3 participants
What type of PR is this?
This is a refactor as it enables dedicated invitee invitation listing by adding
ListInviteeInvitationsWhat does this do?
This pr:
ListDomainInvitationsWhich issue(s) does this PR fix/relate to?
Have you included tests for your changes?
Yes, tests have been included
Did you document any new/modified feature?
Yes, in code documentation is included
Notes