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

Get cancellation policies by reservations operation #650

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

DenysShylov
Copy link

Summary

New operation documentation: Get cancellation policies by reservations operation.

Follow style guide

Style guide

Check during review

  • The changelog and potentially a deprecation entries are in place.
    • The changelog timestamp is date of merge to develop.
  • JSON example extended.
    • New properties are added to the correct place in the JSON.
  • New properties in the table are added to the correct place.
  • Correct formatting:
    • Spacing is consistent between titles, sections, tables, ...
    • Correct JSON format - indentation.
  • DateTime properties should always be defined in ISO format.

Copy link
Contributor

@VBuben VBuben left a comment

Choose a reason for hiding this comment

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

Sorry it took me some time to get to it. Thanks for the docs, most of it looks good, only two comments and it's good for me! 🚀

Make the changes in the code and regenerate docs based on that.

Comment on lines 116 to 118
## Get cancellation policies by reservations

Gets cancellation policies for enterprise grouped by reservation for granular cancellation policies flow. This operation supports [Portfolio Access Tokens](../guidelines/multi-property.md).
Copy link
Contributor

Choose a reason for hiding this comment

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

@Filip-Macecek @DenysShylov I think we should slap Restricted over the endpoint? 🤔 It can be done via [Restricted] annotation in the controller.

Copy link
Author

Choose a reason for hiding this comment

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

will add this attribute to the code. also added restricted description here

| `ClientToken` | string | required | Token identifying the client application. |
| `AccessToken` | string | required | Access token of the client application. |
| `Client` | string | required | Name and version of the client application. |
| `ReservationIds` | array of string | optional, max 100 items | List of reservation identifiers. |
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
| `ReservationIds` | array of string | optional, max 100 items | List of reservation identifiers. |
| `ReservationIds` | array of string | required, max 100 items | List of reservation identifiers. |

I think this one is required right?

Copy link
Author

Choose a reason for hiding this comment

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

you are right, missed this attribute in parameters, fix already in the PR)

Copy link
Contributor

@MikeAdamsMews MikeAdamsMews left a comment

Choose a reason for hiding this comment

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

All I managed to find was one full stop I wasn't happy with :-D

| :-- | :-- | :-- | :-- |
| `CancellationPolicies` | array of [Cancellation policy data grouped by reservation.](cancellationpolicies.md#cancellation-policy-data-grouped-by-reservation) | required, max 1300 items | List of cancellation policies data grouped by reservation. |

#### Cancellation policy data grouped by reservation.
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
#### Cancellation policy data grouped by reservation.
#### Cancellation policy data grouped by reservation


| Property | Type | Contract | Description |
| :-- | :-- | :-- | :-- |
| `CancellationPolicies` | array of [Cancellation policy data grouped by reservation.](cancellationpolicies.md#cancellation-policy-data-grouped-by-reservation) | required, max 1300 items | List of cancellation policies data grouped by reservation. |
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
| `CancellationPolicies` | array of [Cancellation policy data grouped by reservation.](cancellationpolicies.md#cancellation-policy-data-grouped-by-reservation) | required, max 1300 items | List of cancellation policies data grouped by reservation. |
| `CancellationPolicies` | array of [Cancellation policy data grouped by reservation](cancellationpolicies.md#cancellation-policy-data-grouped-by-reservation) | required, max 1300 items | List of cancellation policies data grouped by reservation. |

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

Successfully merging this pull request may close these issues.

3 participants