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

Fix #167: REST API documentation #179

Merged
merged 2 commits into from
Jun 21, 2024
Merged

Conversation

romanstrobl
Copy link
Member

No description provided.

@romanstrobl romanstrobl added the documentation Improvements or additions to documentation label Jun 20, 2024
@romanstrobl romanstrobl self-assigned this Jun 20, 2024
Copy link
Member

@banterCZ banterCZ left a comment

Choose a reason for hiding this comment

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

Looks good to me.


### User Claims API (Deprecated)

This REST API is deprecated, see Swagger for usage information.
Copy link
Member

Choose a reason for hiding this comment

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

What about using a warning box?

Suggested change
This REST API is deprecated, see Swagger for usage information.
<!-- begin box warning -->
This REST API is deprecated, see Swagger for usage information.
<!-- end -->


The following paths require a ROLE_WRITE authority:
- `/admin/**` - supported REST API for User Data Store
- `/public/**` - deprecated REST API for user claims
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- `/public/**` - deprecated REST API for user claims
- `/public/**` - **deprecated** REST API for user claims

Comment on lines 89 to 91
Authorization: Basic YWRtaW46YWRtaW4=

The username and password is a Base-64 encoded string in format username:password.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Authorization: Basic YWRtaW46YWRtaW4=
The username and password is a Base-64 encoded string in format username:password.
`Authorization: Basic YWRtaW46YWRtaW4=`
The username and password is a Base-64 encoded string in the format `username:password`.

Copy link
Contributor

@jnpsk jnpsk left a comment

Choose a reason for hiding this comment

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

Nice!
I found only a few minor problems. Those that should be fixed:

  • There is inconsistency in the placing of the asterisk. We should keep the symbol in the param column.
  • You use photo instead of attachment in some description.

The others are just nitpicking.

Comment on lines +120 to +123
- Headers:
- `Authorization: Basic ...`

##### Query Params
Copy link
Contributor

Choose a reason for hiding this comment

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

Header is a bullet list while query params is a heading. Is this the usual formatting of our API documentation?

|--------------------------------------------------------------|----------|----------------------------------------------------------------------------------------------------|
| `userId`<span class="required" title="Required">*</span> | `String` | User identifier of document owner. |
| `documentId`<span class="required" title="Required">*</span> | `String` | Identifier of the related document. |
| `photoType`<span class="required" title="Required">*</span> | `String` | One of `photoType`: `person`, `document_front_side`, `document_back_side`, `person_with_document`. |
Copy link
Contributor

Choose a reason for hiding this comment

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

A small detail, just for consistency with other lists of expected values.

Suggested change
| `photoType`<span class="required" title="Required">*</span> | `String` | One of `photoType`: `person`, `document_front_side`, `document_back_side`, `person_with_document`. |
| `photoType`<span class="required" title="Required">*</span> | `String` | One of: `person`, `document_front_side`, `document_back_side`, `person_with_document`. |


| Parameter | Type | Description |
|--------------------------------------------------------------|----------|----------------------------------------------------------------------------------------------------|
| `photoType`<span class="required" title="Required">*</span> | `String` | One of `photoType`: `person`, `document_front_side`, `document_back_side`, `person_with_document`. |
Copy link
Contributor

Choose a reason for hiding this comment

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

dtto


| Param | Type | Description |
|--------------|----------------------------------------------------------|---------------------------------------------------------------------|
| `userId` | `String`<span class="required" title="Required">*</span> | User identifier of the owner of fetched documents. |
Copy link
Contributor

Choose a reason for hiding this comment

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

There is inconsistency in the placing of the asterisk. We should keep the symbol in the param column.


| Param | Type | Description |
|--------------|----------------------------------------------------------|---------------------------------------------------------------------|
| `userId` | `String`<span class="required" title="Required">*</span> | User identifier of the owner of deleted documents. |
Copy link
Contributor

Choose a reason for hiding this comment

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

As it is in the request, the documents are yet to be deleted. This past tense is used in more places.

Suggested change
| `userId` | `String`<span class="required" title="Required">*</span> | User identifier of the owner of deleted documents. |
| `userId` | `String`<span class="required" title="Required">*</span> | User identifier of the owner of documents to be deleted. |

| `documentId`<span class="required" title="Required">*</span> | `String` | Identifier of the related document. |
| `attachmentType`<span class="required" title="Required">*</span> | `String` | One of `text`, `image_base64`, `binary_base64`. |
| `attachmentData`<span class="required" title="Required">*</span> | `String` | Base-64 encoded attachment data. |
| `externalId` | `String` | Optional external identifier of the photo (e.g. identifier used in the bank system). |
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
| `externalId` | `String` | Optional external identifier of the photo (e.g. identifier used in the bank system). |
| `externalId` | `String` | Optional external identifier of the attachment (e.g. identifier used in the bank system). |

|------------------------------------------------------------------|----------|--------------------------------------------------------------------------------------|
| `attachmentType`<span class="required" title="Required">*</span> | `String` | One of `text`, `image_base64`, `binary_base64`. |
| `attachmentData`<span class="required" title="Required">*</span> | `String` | Base-64 encoded attachment data. |
| `externalId` | `String` | Optional external identifier of the photo (e.g. identifier used in the bank system). |
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
| `externalId` | `String` | Optional external identifier of the photo (e.g. identifier used in the bank system). |
| `externalId` | `String` | Optional external identifier of the attachment (e.g. identifier used in the bank system). |

@romanstrobl romanstrobl merged commit 3a00f14 into develop Jun 21, 2024
3 checks passed
@romanstrobl romanstrobl deleted the issues/167-api-documentation branch June 21, 2024 10:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants