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.

Sign up for GitHub

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

Jump to bottom

[Recombee (Actions)] Fix bugs #2564

Merged
merged 1 commit into from
Nov 12, 2024
Merged

[Recombee (Actions)] Fix bugs #2564

merged 1 commit into from
Nov 12, 2024

Conversation

danielforr
Copy link
Contributor

@danielforr danielforr commented Nov 4, 2024 •
edited
Loading

Hi,

After testing our integration in private beta we've found some bugs and things that should be changed, addressing all of them in this PR.

Changes:

  • fixed ecommerceIdMapping field paths
  • deleted format: 'hostname' from API URI field - was considered "required" (probably due to format) even when hidden in UI
  • added Product Added preset after being removed in 2445 - fixed the fields
  • simplified addBookmark defaultSubscription - unfortunately it wasn't working in the UI
  • addCartAddition was accidentally set to array even though it should process just 1 item
    • changed fields from array to object
    • changed perform to reflect change from array to one item object
    • changed tests to not expect batch requests
    • remove test with multiple items - not relevant anymore
    • regenerated snapshots and types

Testing

Include any additional information about the testing you have completed to
ensure your changes behave as expected. For a speedy review, please check
any of the tasks you completed below during your testing.

  • Added unit tests for new functionality
  • Tested end-to-end using the local server
  • [If destination is already live] Tested for backward compatibility of destination. Note: New required fields are a breaking change.
  • [Segmenters] Tested in the staging environment
  • [Segmenters] [If applicable for this change] Tested for regression with Hadron.

@danielforr danielforr requested a review from a team as a code owner November 4, 2024 16:27
@seg-atlantis-prod
Copy link

Atlantis commands can't be run on fork pull requests. To enable, set --allow-fork-prs or, to disable this message, set --silence-fork-pr-errors

@seg-atlantis-prod
Copy link 

Error parsing command: EOF found when expecting closing quote

@joe-ayoub-segment joe-ayoub-segment self-assigned this Nov 5, 2024
@joe-ayoub-segment
Copy link
Contributor

Thanks for the PR @danielforr . LGTM! We're not deploying code this week due to the election, so this will go out next Tuesday.

@joe-ayoub-segment
Copy link
Contributor

hi @danielforr just a quick clarification - there's no customers using this yet is there?

@danielforr
Copy link
Contributor Author

Hi @joe-ayoub-segment thanks for the quick review. There's no customers using this yet, the integration was just tested by us internally.

@joe-ayoub-segment joe-ayoub-segment merged commit a2ef07c into segmentio:main Nov 12, 2024
10 of 12 checks passed
@joe-ayoub-segment
Copy link
Contributor

hi @danielforr PR deployed. Please confirm that you are happy with the change!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joe-ayoub-segment joe-ayoub-segment joe-ayoub-segment approved these changes

Assignees

@joe-ayoub-segment joe-ayoub-segment

Labels
deploy:push mode:cloud partner-eng-review (cloud) Ready for Release team:external
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@danielforr @joe-ayoub-segment