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

Feat: add CGW Accounts API specification #190

Merged
merged 7 commits into from
Jul 22, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions src/types/accounts.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
export type Account = {
accountId: string
groupId: string | null
address: string
}

export type AccountDataType = {
id: string
name: string
description: string | null
isActive: boolean
}

export type AccountDataSetting = {
name: string
description: string | null
enabled: boolean
Copy link
Member

Choose a reason for hiding this comment

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

There seems to be an overlap here. Can we combine this into a shared type?

Also is isActive and enabled the same thing?

Copy link
Member Author

Choose a reason for hiding this comment

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

  • isActive is an AccountDataType-only property. It indicates if the DataType (CF Safes, watchlist, etc.) is generally available in the CGW. The idea behind this is to provide a list of available DataTypes so the UI could build the list of checkboxes (or another widget) for the user to select using this as a source of truth. An alternative approach would be to use this isActive internally in the CGW and only return the active ones to the UI. This would be simpler but also less transparent. Do you have any preference regarding this? 🙂
  • enabled is an AccountDataSetting-only property. It represents a specific user setting (i.e.: whether the checkbox is marked or it isn't)

Regarding the overlap you commented on, I think you are right and this API can also be simplified. I'm working in the CGW on that regard and will update this accordingly later on 🙂

Copy link
Member Author

Choose a reason for hiding this comment

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

I've modified the API on the CGW side, and I have also modified this PR accordingly in eaa943b

  • Account.accountId was renamed to simply id (as it's the type main identifier).
  • I've solved the overlap between the AccountDataType and AccountDataSetting types.

}

export type CreateAccountDto = {
Copy link
Member

Choose a reason for hiding this comment

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

Please don't use the DTO acronym in type names. It makes sense within the internal CGW architecture, but for clients it's a non-concept.

Copy link
Member Author

Choose a reason for hiding this comment

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

Changed in eaa943b

address: string
}

export type UpsertAccountDataSettingsDto = {
accountDataSettings: {
id: string
enabled: boolean
Copy link
Member

Choose a reason for hiding this comment

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

Another (partial) duplication of AccountDataSetting?

Copy link
Member Author

Choose a reason for hiding this comment

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

Also solved in eaa943b

}[]
}
118 changes: 118 additions & 0 deletions src/types/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,13 @@ import type { RelayCountResponse, RelayTransactionRequest, RelayTransactionRespo
import type { RegisterRecoveryModuleRequestBody } from './recovery'
import type { Contract } from './contracts'
import type { AuthNonce } from './auth'
import {
Account,
AccountDataSetting,
AccountDataType,
CreateAccountDto,
UpsertAccountDataSettingsDto,
} from './accounts'

export type Primitive = string | number | boolean | null

Expand Down Expand Up @@ -472,6 +479,39 @@ export interface paths extends PathRegistry {
}
}
}
'/v1/accounts': {
post: operations['create_account']
parameters: {
path: null
credentials: 'include'
}
}
'/v1/accounts/{address}': {
get: operations['get_account']
delete: operations['delete_account']
parameters: {
path: {
address: string
}
credentials: 'include'
}
}
'/v1/accounts/data-types': {
get: operations['get_account_data_types']
parameters: {
path: null
}
}
'/v1/accounts/{address}/data-settings': {
get: operations['get_account_data_settings']
put: operations['put_account_data_settings']
parameters: {
path: {
address: string
}
credentials: 'include'
}
}
Comment on lines +505 to +514
Copy link
Member

Choose a reason for hiding this comment

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

Not sure if we have to define the body here as well but when calling putAccountDataSettings there is no type checking so I can pass anything I want as the second parameter in putAccountDataSettings(wallet.address, {}) without an error 🤔 @katspaugh do you remember if this is expected?

}

export interface operations {
Expand Down Expand Up @@ -1225,4 +1265,82 @@ export interface operations {
}
}
}
create_account: {
parameters: {
body: CreateAccountDto
credentials: 'include'
}
responses: {
200: {
schema: Account
}
403: unknown
422: unknown
}
}
get_account_data_types: {
parameters: null
responses: {
200: {
schema: AccountDataType[]
}
}
}
get_account_data_settings: {
parameters: {
path: {
address: string
}
credentials: 'include'
}
responses: {
200: {
schema: AccountDataSetting[]
}
403: unknown
}
}
put_account_data_settings: {
parameters: {
path: {
address: string
}
credentials: 'include'
body: UpsertAccountDataSettingsDto
}
responses: {
200: {
schema: AccountDataSetting[]
}
403: unknown
}
}
get_account: {
parameters: {
path: {
address: string
}
credentials: 'include'
}
responses: {
200: {
schema: Account
}
403: unknown
}
}
delete_account: {
parameters: {
path: {
address: string
}
credentials: 'include'
}
responses: {
204: {
schema: void
}
403: unknown
}
}
}
Loading