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/gpe 14858 update segment contract #1577

Open
wants to merge 6 commits into
base: dev
Choose a base branch
from

Conversation

matthewhallinan-gen
Copy link

As part of the in-progress Customer Segmentation feature, Journey Segments can be directly assigned to an external contact (representing the end user), rather than the user's session. This allows Segments to live beyond the lifetime of the session. A new field is being added to the Segment, for admins to configure how long these Segment assignments to a contact should last. This PR adds this new field to the CX as Code repo.

@matthewhallinan-gen
Copy link
Author

matthewhallinan-gen commented Feb 27, 2025

Few things I noticed while working on the PR:

  1. Scope is being removed from the public API as a request body field (see this ticket). There's a dev forum announcement for the removal; since it only ever had 1 value, it's redundant, and the removal should have no impact.. It's already removed from the backend service as of this ticket. I can block this ticket with the linked Pub API ticket and remove it while in the neighbourhood, but it’s probably better to make a separate ticket for it.

Should removing it here block its removal from Pub API? What's the process for removing fields from a request contract on CX as Code? What about the response contract (looks like CX as Code shares the schema for both)? Are we good to remove Scope, & does it need to be done before Pub API?

  1. The test structure confused me. 2 of the test cases are basic_attributes and optional_attributes - see here. The only thing being tested under optional attributes is description. However, under the Segment docs, colour, display_name and scope are the only ones listed as required; everything else is optional. That in turn doesn’t align with Pub API, where there are 4 required fields: colour, displayName, journey and context (see pic below).
    Considering Scope and shouldDisplayToAgent are optional and have defaults (and therefore should probably be tested under the optional tests in CX as Code), I’ve added assignmentExpirationDays into all tests where they’re added. So, the optional attributes test is still only testing description. But maybe all of this should be tidied up, so that the optional tests only test the fields which are optional as per Pub API?
    Again, probably something for a separate ticket?
Screenshot 2025-02-27 at 15 26 05
  1. Confusing point 2 further, isActive is present on the Segment schema but it doesn’t seem to be tested at all. Maybe also a separate ticket? Does it need adding to the testing? I guess it does.

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

Successfully merging this pull request may close these issues.

1 participant