-
Notifications
You must be signed in to change notification settings - Fork 677
SMQ-3108 - Add support for public and private metadata for users and clients #3155
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
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3155 +/- ##
===========================================
+ Coverage 53.21% 79.16% +25.95%
===========================================
Files 312 109 -203
Lines 32706 13203 -19503
===========================================
- Hits 17404 10452 -6952
+ Misses 14234 2133 -12101
+ Partials 1068 618 -450 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
9fefd43 to
72c75d5
Compare
72c75d5 to
bd9dfdd
Compare
3a95df7 to
37a317c
Compare
37a317c to
e8269da
Compare
dborovcanin
left a comment
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.
To keep it backward compatible and not break API, let's consider our Metadata private, and just add public one. I.e. we have metadata and publicMetadata fields. Private ones should not be visible even to superadmin, probably, but for now - let's just add public meta.
d31dca6 to
7253696
Compare
|
@felixgateru Please resolve conflicts. |
7253696 to
7c51ae9
Compare
| if uce.PublicMetadata != nil { | ||
| val["public_metadata"] = uce.PublicMetadata |
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 we are not emitting metadata ?
| meta, ok := data["public_metadata"].(map[string]any) | ||
| if ok { | ||
| c.Metadata = meta | ||
| c.PublicMetadata = meta | ||
| } |
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.
Missing metadata in decoding
| logType outputLog | ||
| }{ | ||
| { | ||
| desc: "update client name and public metadata successfully", |
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 a test case for updating public metadata only (also add unhappy flow for it).
| err: nil, | ||
| }, | ||
| { | ||
| desc: "update public metadata successfully as normal 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.
Add unhappy flow also (invalid format).
Signed-off-by: Felix Gateru <felix.gateru@gmail.com>
Signed-off-by: Felix Gateru <felix.gateru@gmail.com>
Signed-off-by: Felix Gateru <felix.gateru@gmail.com>
Signed-off-by: Felix Gateru <felix.gateru@gmail.com>
Signed-off-by: Felix Gateru <felix.gateru@gmail.com>
Signed-off-by: Felix Gateru <felix.gateru@gmail.com>
Signed-off-by: Felix Gateru <felix.gateru@gmail.com>
Signed-off-by: Felix Gateru <felix.gateru@gmail.com>
Signed-off-by: Felix Gateru <felix.gateru@gmail.com>
Signed-off-by: Felix Gateru <felix.gateru@gmail.com>
Signed-off-by: Felix Gateru <felix.gateru@gmail.com>
aa33db2 to
d48fc9b
Compare
Signed-off-by: Felix Gateru <felix.gateru@gmail.com>
What type of PR is this?
This is a feature as it adds support for private and public metadata for Users and Clients
What does this do?
This pr:
Which issue(s) does this PR fix/relate to?
Have you included tests for your changes?
Yes, tests have been updated.
Did you document any new/modified feature?
Yes, api docs have been updated for the changes
Notes