Skip to content

Conversation

@cemalkilic
Copy link
Contributor

update oauth grant list response structure for api consistency

@coveralls
Copy link

coveralls commented Nov 6, 2025

Pull Request Test Coverage Report for Build 19165582260

Details

  • 9 of 13 (69.23%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.003%) to 68.311%

Changes Missing Coverage Covered Lines Changed/Added Lines %
internal/api/oauthserver/authorize.go 0 4 0.0%
Totals Coverage Status
Change from base Build 19136539906: -0.003%
Covered Lines: 14003
Relevant Lines: 20499

💛 - Coveralls

Comment on lines 588 to 590
ClientID: client.ID.String(),
ClientName: utilities.StringValue(client.ClientName),
ClientURI: utilities.StringValue(client.ClientURI),
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we're nesting these fields under the client object, I think it might make more sense to avoid repeating the client_ suffix for the field names — WDYT?

{
  "client": {
    "id": "...",
    "name": "...",
    "uri": "...",
  }
}

If the client_id is the id column in the table, I think it shouldn't result in any confusion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤦 vibe coded this "small" change, sorry. fixed it now!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

p.s I updated this, but it also impacts the existing endpoint (GET /oauth/authorizations/{authorization_id}) response, I'll add this to release notes as it'll break the existing beta users'.

@cemalkilic cemalkilic changed the title feat(oauthserver): update oauth grant list response structure feat(oauthserver): update oauth grant list & authorization details response structure Nov 7, 2025
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.

4 participants