Skip to content

Conversation

mdojwa
Copy link
Contributor

@mdojwa mdojwa commented Jun 16, 2025

Deploy preview

Scroll down to the checks section for the link to the deploy preview of your branch.

Links

Link your Jira ticket.

Description

Versioning

Pull request titles should comply with the Conventional Commits specification. This is required to update the project's version.

Review and release

Apply changes and resolve conflicts. Once the described functionality lands on prod, let us know on #platform-docs-releases that your PR is ready for release. We will merge and publish the changes.

@mdojwa mdojwa requested a review from a team as a code owner June 16, 2025 15:12
Copy link
Collaborator

@oliwiapolec oliwiapolec left a comment

Choose a reason for hiding this comment

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

Few more notes that apply to all/the whole PR:

  1. The ‘Required’ column in tables should be before the ‘Data type’ column
  2. Please add info about these new methods to the changelog
  3. I’m also thinking about taking out some parameters outside the methods to a separate section as they apply to multiple of them, but I’d have to see this after the changes from this review are applied 😌

We can apply table formatting when all other changes are approved 👌


### Create Greeting

Creates a new greeting with specified parameters within a license.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Creates a new greeting with specified parameters within a license.
Creates a new greeting.


**3)** Possible values for the `rules[].condition` parameter:

| Possible value | Notes |
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd add them just like:

  • and
  • or

I don't think this requires an explanation table 😌


**1)** Possible values for the `elements[].buttons[].role` parameter:

| Possible value | Notes |
Copy link
Collaborator

Choose a reason for hiding this comment

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

If there are no notes I'd make this a list instead of a table as well

  • default
  • primary
  • danger


### Delete Greeting

Deletes a greeting specified by `id`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Deletes a greeting specified by `id`.
Deletes a greeting.


### List Greetings

Returns a list of greetings, optionally filtered by groups. The method supports pagination to handle large result sets.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Returns a list of greetings, optionally filtered by groups. The method supports pagination to handle large result sets.
Returns a list of greetings.


#### Request

| Parameter | Data type | Required | Default | Notes |
Copy link
Collaborator

Choose a reason for hiding this comment

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

We don't have a 'default' column in our docs' tables, if there are any default values, they should be added in the notes

| ---------- | --------- | -------- | ------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `groups` | `array` | No | All accesible groups | Array of group IDs to filter greetings. Cannot be used with `page_id`. |
| `page_id` | `string` | No | - | Page ID for pagination. When provided, `groups` and `limit` parameters are ignored as they're encoded in the page ID. Cannot be used with `groups` or `limit`. |
| `limit` | `int` | No | `100` | Number of greetings per page. Must be between 1 and 100. Cannot be used with `page_id`. |
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
| `limit` | `int` | No | `100` | Number of greetings per page. Must be between 1 and 100. Cannot be used with `page_id`. |
| `limit` | `int` | No | `100` | Number of greetings per page; default: 100. Limited to 100. Cannot be used with `page_id`. |

| Field | Data type | Notes |
| ----------------- | --------- | ------------------------------------------------------------------------------------------------------------ |
| `greetings` | `array` | Array of greeting objects. See [Create Greeting](#create-greeting) for the structure of greeting objects. |
| `found_greetings` | `int` | Total number of greetings found (not just in current page). |
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
| `found_greetings` | `int` | Total number of greetings found (not just in current page). |
| `found_greetings` | `int` | The total number of greetings. |

| ----------------- | --------- | ------------------------------------------------------------------------------------------------------------ |
| `greetings` | `array` | Array of greeting objects. See [Create Greeting](#create-greeting) for the structure of greeting objects. |
| `found_greetings` | `int` | Total number of greetings found (not just in current page). |
| `next_page_id` | `string` | Page ID for the next page of results. Only present if there are more results available. |
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
| `next_page_id` | `string` | Page ID for the next page of results. Only present if there are more results available. |
| `next_page_id` | `string` | Page ID for the next page of results; returned only if more results are available. |

| `active_from` | `string` | ISO 8601 datetime when the greeting becomes active (if set). |
| `active_until` | `string` | ISO 8601 datetime when the greeting stops being active (if set). |
| `name` | `string` | Greeting name. |
| `group` | `int` | ID of the group the greeting belongs to. |
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
| `group` | `int` | ID of the group the greeting belongs to. |
| `group` | `int` | ID of the group the greeting is assigned to. |

@mdojwa mdojwa force-pushed the CDP-200-publish_greetings branch from e54276f to c897285 Compare July 18, 2025 18:19
@mdojwa mdojwa force-pushed the CDP-200-publish_greetings branch from c897285 to 3f2bbf5 Compare July 18, 2025 18:24
@oliwiapolec
Copy link
Collaborator

I see you're making some changes to the PR — please re-request the review when you're ready and changes from my review are applied 👌

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.

2 participants