Skip to content
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

Added support for Get information for the current session's user #353

Merged
merged 3 commits into from
Aug 1, 2024

Conversation

ParthaI
Copy link
Contributor

@ParthaI ParthaI commented Jul 2, 2024

Get information for the current session's user: https://api.aiven.io/doc/#tag/Users/operation/UserInfo

@ParthaI ParthaI requested a review from a team as a code owner July 2, 2024 13:37
Copy link
Contributor

@byashimov byashimov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey! Thanks for your contribution!
If there is no particular reason to go with this client, I recommend to try our generated. Here is the method you are implementing https://github.com/aiven/go-client-codegen/blob/259be870943791d517994df16b18e601f0278ebe/handler/user/user.go#L107
It is experimental, but since it is generated it is more accurately than this one.

me.go Outdated Show resolved Hide resolved
me_test.go Outdated Show resolved Hide resolved
me.go Outdated Show resolved Hide resolved
me.go Outdated Show resolved Hide resolved
@ParthaI
Copy link
Contributor Author

ParthaI commented Jul 23, 2024

Hi @byashimov,

Thank you for your detailed feedback. In our project, we have used the Go package aiven-go-client. The suggested API is in the package go-client-codegen. Changing the Go package in our project might break our existing implementation.

If we could consider this PR, it would be greatly appreciated.

Please use *bool here to know if values was provided.

I have updated the Features type to map[string]any

Let's remove the test. It actually tests the struct and mocked value 🙏

Removed the test file.

  1. Please use a pointer for Features
  2. It might have more fields, as it has no strict schema
  3. Intercom is required field

Updated the Features attribute type and marked the attributes as required based on the API reference in the documentation.

Thanks!

Copy link
Contributor

@byashimov byashimov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey. Thanks for the updates.
Please take a look at these:

  1. there is no viewed_indicators field https://api.aiven.io/doc/#tag/Users/operation/UserInfo
  2. should remove pointers from required field

I recommend to copy this struct https://github.com/aiven/go-client-codegen/blob/main/handler/user/user.go#L718
It was generated from the spec

@ParthaI
Copy link
Contributor Author

ParthaI commented Jul 31, 2024

Hello @byashimov, thank you for your suggestions. I've made the changes accordingly. Could you please take another look?

@byashimov byashimov merged commit 7075116 into aiven:main Aug 1, 2024
3 of 4 checks passed
@byashimov
Copy link
Contributor

Thank you @ParthaI for the contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants