Skip to content

refactor(data-events): add abstract Connector class for shared methods #3015

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
Merged
dkoo merged 3 commits into from
Apr 18, 2024
Merged

refactor(data-events): add abstract Connector class for shared methods #3015

dkoo merged 3 commits into from
Apr 18, 2024

Conversation

@dkoo
Copy link
Contributor

@dkoo dkoo commented Mar 26, 2024

All Submissions:

Changes proposed in this Pull Request:

Just a small refactor of Data Events to get rid of some duplicated code in two very similar classes. Creates an abstract Connector class with shared methods that can be extended for ESP-specific functionality.

How to test the changes in this Pull Request:

No changes to functionality—test that RAS contact syncing still works as before with both Mailchimp and ActiveCampaign.

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully ran tests with your changes locally?

@dkoo dkoo added the [Status] Needs Review The issue or pull request needs to be reviewed label Mar 26, 2024
@dkoo dkoo self-assigned this Mar 26, 2024
@dkoo dkoo requested a review from a team as a code owner March 26, 2024 21:49
adekbadek
adekbadek reviewed Apr 5, 2024
View reviewed changes
Copy link
Member

@adekbadek adekbadek left a comment

Choose a reason for hiding this comment

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

Besides the refactor, there's one additive change: the newsletter_updated method of the new Connector class. Is that intentional?

@dkoo
Copy link
Contributor Author

dkoo commented Apr 16, 2024 •
edited
Loading

Besides the refactor, there's one additive change: the newsletter_updated method of the new Connector class. Is that intentional?

@adekbadek Whoops, you're right! The newsletter_updated method is specific to ActiveCampaign and already exists there, so it doesn't need to exist in the Connector class. Removed and also resolved conflicts with trunk in 712881d.

@github-actions github-actions bot added [Status] Approved The pull request has been reviewed and is ready to merge and removed [Status] Needs Review The issue or pull request needs to be reviewed labels Apr 17, 2024
@dkoo dkoo merged commit a6e8a6e into trunk Apr 18, 2024
@dkoo dkoo deleted the refactor/connector-abstract-class branch April 18, 2024 15:10
@matticbot
Copy link
Contributor

matticbot commented Apr 25, 2024

🎉 This PR is included in version 3.9.0-epic-ras-acc.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@matticbot
Copy link
Contributor

matticbot commented Apr 25, 2024

🎉 This PR is included in version 4.0.0-alpha.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

claudiulodro pushed a commit that referenced this pull request May 15, 2024
@adekbadek @matticbot
@dependabot @dkoo @jaredrethman @semantic-release-bot
Release May 15 (#3103)
2f0fa92 
* chore(deps-dev): bump @wordpress/browserslist-config

Bumps [@wordpress/browserslist-config](https://github.com/WordPress/gutenberg/tree/HEAD/packages/browserslist-config) from 5.37.0 to 5.38.0.
- [Release notes](https://github.com/WordPress/gutenberg/releases)
- [Changelog](https://github.com/WordPress/gutenberg/blob/trunk/packages/browserslist-config/CHANGELOG.md)
- [Commits](https://github.com/WordPress/gutenberg/commits/@wordpress/browserslist-config@5.38.0/packages/browserslist-config)

---
updated-dependencies:
- dependency-name: "@wordpress/browserslist-config"
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

* chore(deps-dev): bump @rushstack/eslint-patch from 1.10.1 to 1.10.2

Bumps [@rushstack/eslint-patch](https://github.com/microsoft/rushstack/tree/HEAD/eslint/eslint-patch) from 1.10.1 to 1.10.2.
- [Changelog](https://github.com/microsoft/rushstack/blob/main/eslint/eslint-patch/CHANGELOG.md)
- [Commits](https://github.com/microsoft/rushstack/commits/@rushstack/eslint-patch_v1.10.2/eslint/eslint-patch)

---
updated-dependencies:
- dependency-name: "@rushstack/eslint-patch"
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

* chore(deps-dev): bump @types/wordpress__blocks from 12.5.13 to 12.5.14

Bumps [@types/wordpress__blocks](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/wordpress__blocks) from 12.5.13 to 12.5.14.
- [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases)
- [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/wordpress__blocks)

---
updated-dependencies:
- dependency-name: "@types/wordpress__blocks"
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

* refactor(data-events): add abstract Connector class for shared methods (#3015)

* refactor: add abstract Connector class for shared methods

* chore: remove unused imports

* chore(deps-dev): bump @wordpress/browserslist-config

Bumps [@wordpress/browserslist-config](https://github.com/WordPress/gutenberg/tree/HEAD/packages/browserslist-config) from 5.38.0 to 5.39.0.
- [Release notes](https://github.com/WordPress/gutenberg/releases)
- [Changelog](https://github.com/WordPress/gutenberg/blob/trunk/packages/browserslist-config/CHANGELOG.md)
- [Commits](https://github.com/WordPress/gutenberg/commits/@wordpress/browserslist-config@5.39.0/packages/browserslist-config)

---
updated-dependencies:
- dependency-name: "@wordpress/browserslist-config"
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

* chore(deps): bump qs and @types/qs

Bumps [qs](https://github.com/ljharb/qs) and [@types/qs](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/qs). These dependencies needed to be updated together.

Updates `qs` from 6.12.0 to 6.12.1
- [Changelog](https://github.com/ljharb/qs/blob/main/CHANGELOG.md)
- [Commits](ljharb/qs@v6.12.0...v6.12.1)

Updates `@types/qs` from 6.9.14 to 6.9.15
- [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases)
- [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/qs)

---
updated-dependencies:
- dependency-name: qs
  dependency-type: direct:production
  update-type: version-update:semver-patch
- dependency-name: "@types/qs"
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

* fix: ensure only admins can reset starter content and newspack options (#3081)

* feat: add product option to autocomplete orders (#3072)

BREAKING CHANGE: Changes order autocompletion behavior for existing products!

* feat: add product option to autocomplete orders

* chore: update outdated docblock description

* feat(ras): skip campaign setup (#3051)

* feat(ras): skip campaign setup boilerplate

added new route, added fe logic

* feat(ras): added `is_skipped` to prerequisite status endpoint

* refactor: moved callback to named function

added ui to indicate async skipping flow

* feat: added skip logic to prerequisite component

combined "skipped" and "active" into single `isValid` const

* feat: `is_skipped` property to prerequisite type

added skip logic to RAS wizard

* fix: circle ci / typescript

* fix: ci / eslint

* refactor: remove redirect after skip

added allow continue when skipped.

* refactor: allow parametrized skip

* refactor: revert any type cast

* feat: pr feedback - added redirect, activation skip and remove `activationStep[0]`

* feat: pr feedback, appended `[skipped]` to select items

* chore(release): 4.0.0-alpha.1 [skip ci]

# [4.0.0-alpha.1](v3.8.0...v4.0.0-alpha.1) (2024-04-25)

### Bug Fixes

* ensure only admins can reset starter content and newspack options ([#3081](#3081)) ([4606721](4606721))

### Features

* add product option to autocomplete orders ([#3072](#3072)) ([4a2859b](4a2859b))
* **ras:** skip campaign setup ([#3051](#3051)) ([9ef0e6d](9ef0e6d))

### BREAKING CHANGES

* Changes order autocompletion behavior for existing products!

* feat: add product option to autocomplete orders

* chore: update outdated docblock description

* chore(release): 4.0.0-alpha.2 [skip ci]

# [4.0.0-alpha.2](v4.0.0-alpha.1...v4.0.0-alpha.2) (2024-04-25)

### Bug Fixes

* remove deprecated filter callback ([#3090](#3090)) ([5d7d0bf](5d7d0bf))

* chore(release): 4.0.0-alpha.3 [skip ci]

# [4.0.0-alpha.3](v4.0.0-alpha.2...v4.0.0-alpha.3) (2024-04-26)

### Bug Fixes

* enable Memberships fix cron job only when environment constant is defined ([#3087](#3087)) ([5d40297](5d40297))

* OAuth: use a custom table for transients (#3106)

* feat: start new custom table for oauth transients data

* fix: copy/paste error

* fix(google-oauth): use custom table for transients

* feat: cleanup old transients

* fix: clean up on the fly too, and limit to deleting 1000 at a time max

---------

Co-authored-by: Adam Cassis <adam.cassis@automattic.com>

* fix: autocomplete orders only for virtual products (#3111)

* fix: autocomplete orders only for virtual products

* chore: remove console.log

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: matticbot <sysops+ghmatticbot@automattic.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Derrick Koo <dkoo@users.noreply.github.com>
Co-authored-by: Jared Rethman <jaredrethman@gmail.com>
Co-authored-by: matticbot <semantic-release-bot@martynus.net>
Co-authored-by: dkoo <derrick.koo@automattic.com>
@matticbot
Copy link
Contributor

matticbot commented May 28, 2024

🎉 This PR is included in version 4.0.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

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

Reviewers

@adekbadek adekbadek adekbadek approved these changes

Assignees

@dkoo dkoo

Labels

released on @alpha released on @epic/ras-acc released [Status] Approved The pull request has been reviewed and is ready to merge

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@dkoo @matticbot @adekbadek