feat(integrations): add integrations settings by chickenn00dle · Pull Request #4541 · Automattic/newspack-plugin

chickenn00dle · 2026-03-04T16:56:54Z

All Submissions:

Have you followed the Newspack Contributing guideline?
Does your code follow the WordPress' coding standards and VIP Go coding standards?
Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Changes proposed in this Pull Request:

Closes https://linear.app/a8c/issue/NPPD-1325/framework-allow-integrations-to-declare-settings-fields-plus-prototype

Adds a settings framework for reader activation integrations. Integrations can now declare settings fields (text, checkbox, select, number, textarea, password) that are stored as WordPress options and managed via a new Audience Integrations admin page.

This PR also adds a prototype UI that mostly copies the UI for sync in Audience > Configuration.

Backend:

Integration base class: new settings_fields property, getter/setter methods with type-aware sanitization, register_settings_fields for declaring settings fields, and get_settings_config() for API responses.
Integrations: new get_all_integration_settings() and update_integration_settings() static methods.
Audience_Integrations wizard: REST API endpoints (GET /settings, POST /settings) gated behind a NEWSPACK_INTEGRATIONS_SETTINGS_ENABLED feature flag.

Frontend:

React settings page under the Audience wizard that renders each integration's settings fields and supports save/reset.
Page is conditionally registered only when the feature flag is enabled

How to test the changes in this Pull Request:

Enable the feature by adding the NEWSPACK_INTEGRATIONS_SETTINGS_ENABLED flag to wp-config.php. Also enabled logging to more easily identify sync data in the logs (define( 'Newspack_Log_Level', 3 );)
Build the plugin: n build newspack-plugin.
Navigate to Audience > Integrations in wp-admin.
Verify that integrations with declared settings_fields appear with their fields rendered correctly.
Change values, save, and confirm persistence on page reload.
As a reader create a new reader account and verify the sync looks correct.
Disable the feature flag in wp-config.php and confirm the page no longer appears.
As a reader create a new reader account and verify the sync looks correct. (Should follow settings defined in Audience > Configuration).

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?

includes/reader-activation/integrations/class-contact-pull.php

chickenn00dle · 2026-03-09T17:39:16Z

includes/reader-activation/integrations/class-esp.php

+	 */
+	public function register_settings_fields() {
+		$fields = [];
+		if ( ! Audience_Integrations::is_enabled() || ! Reader_Activation::is_esp_configured() ) {


One thing I wasn't certain about is whether we wanted to move the main esp configuration (api keys) into the integration. I opted to only move sync settings here and leave the main configuration in newsletter settings since newsletters should be usable without sync.

Copilot

Pull request overview

This PR adds a settings framework for reader activation integrations. It allows integrations (starting with the ESP integration) to declare settings fields that are stored as WordPress options and managed via a new Audience Integrations admin page, gated behind a NEWSPACK_INTEGRATIONS_SETTINGS_ENABLED constant.

Changes:

New Audience_Integrations wizard class with REST API endpoints (GET /settings, POST /settings, POST /settings/{id}/enabled) for managing integration settings
Integration base class extended with settings_fields, register_settings_fields() abstract method, settings getters/setters, metadata prefix management, and a unified get_settings_config() API method
Renaming of OPTION_PREFIX → INCOMING_FIELDS_OPTION_PREFIX, get_selected_fields() → get_incoming_fields(), set_selected_fields() → set_incoming_fields()

Reviewed changes

Copilot reviewed 14 out of 15 changed files in this pull request and generated 13 comments.

Show a summary per file

File	Description
`includes/reader-activation/integrations/class-integration.php`	Core: adds settings fields framework, metadata prefix management, renames incoming fields constants/methods, merges `get_enabled_outgoing_fields_raw_keys()` and `get_enabled_outgoing_fields_prefixed_keys()` into unified `get_enabled_outgoing_fields_keys($prefixed)`
`includes/reader-activation/integrations/class-esp.php`	Implements `register_settings_fields()`, `get_provider()`, `get_list_options()`, `get_master_list_id()`; adds feature flag branching in `can_sync()`, `push_contact_data()`, `get_incoming_available_contact_fields()`
`includes/reader-activation/class-integrations.php`	Adds `get_all_integration_settings()` and `update_integration_settings()` static methods; changes ESP auto-enable logic
`includes/reader-activation/class-reader-activation.php`	Routes ESP settings through the integration when feature flag is on
`includes/reader-activation/sync/class-metadata.php`	Routes `get_prefix()` through ESP integration when flag is on; renames method references in deprecated methods
`includes/reader-activation/integrations/class-contact-pull.php`	Updates renamed method calls from `get_selected_fields()` to `get_incoming_fields()`
`includes/wizards/audience/class-audience-integrations.php`	New wizard file providing admin page and REST API endpoints
`src/wizards/audience/views/integrations/index.js`	New React UI for the Integrations settings page
`src/wizards/index.tsx`	Conditionally registers the new integrations page component
`src/wizards/types/window.d.ts`	TypeScript type declaration for `newspackAudienceIntegrations` global
`includes/class-newspack.php`	Includes the new wizard file
`includes/class-wizards.php`	Instantiates `Audience_Integrations` wizard
`tests/unit-tests/integrations/class-test-integrations.php`	Adds new tests for metadata prefix, settings config, and renames test method names
`tests/unit-tests/integrations/class-sample-integration.php`	Implements `register_settings_fields()` abstract method
`tests/unit-tests/integrations/class-failing-sample-integration.php`	Implements `register_settings_fields()` abstract method (with a bug)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

includes/reader-activation/integrations/class-integration.php

tests/unit-tests/integrations/class-failing-sample-integration.php

includes/reader-activation/integrations/class-integration.php

src/wizards/audience/views/integrations/index.js

includes/wizards/audience/class-audience-integrations.php

src/wizards/audience/views/integrations/index.js

includes/reader-activation/integrations/class-integration.php

chickenn00dle · 2026-03-09T19:45:15Z

I had a look at the design update from Thomas and had a go at re-implementing this, but decided it might be better to push out what I have already and update the UI in a follow-up PR.

leogermani · 2026-03-10T21:37:10Z

I had a look at the design update from Thomas and had a go at re-implementing this, but decided it might be better to push out what I have already and update the UI in a follow-up PR.

I had a first pass at the code... I think we are touching more things than we should be. The prefix was intentionally left as a global option in #4544... and all those metadata methods are super fragile

Let's connect tomorrow and look at this together

leogermani

I gave it a thorough review now, and this is looking good. I left some comments in the code.

One of the points is that I think we should avoid that check to see if the new UI is enabled or not in order to route the fetching and setting of the settings. We should do something similar we did here.

The ideas is that we deprecate these global settings, and then route them to the ESP integration. First time we try to fetch them, we migrate the values from the global settings to the ESP integration setting.

This is cool because then the two UIs can actually exist while we are still in development and the transition is seamless. People will already be tweaking the ESP integration settings from the OLD UI - This is already what happens with enabled_outgoing_fields.

At first I thought we could leave the prefix as a global option. But it wasn't such a strong opinion. Now that you've done it, I think it's fine.

The UI for the incoming fields should also be a list of checkboxes. The available values come from get_incoming_available_contact_fields (that could be renamed to get_available_incoming_contact_fields)

leogermani · 2026-03-11T14:54:15Z

includes/reader-activation/integrations/class-esp.php

+		$provider = $this->get_provider();
+
+		// For Mailchimp, filter out groups and tags, only include remote lists.
+		if ( 'mailchimp' === $provider ) {


Mailchimp has a handy method to do that:

function get_lists( $audiences_only = false ) {

Thanks! Updated in 05014d7

leogermani · 2026-03-11T14:56:21Z

includes/reader-activation/integrations/class-esp.php

-				'ras_esp_master_list_id_not_found',
-				__( 'ESP master list ID is not set.', 'newspack-plugin' )
-			);
+		if ( Audience_Integrations::is_enabled() ) {


We shouldn't have this check. We should automatically route everything to the integration. For now, this ESP integration will be enabled by default

Updated in bc1448a

leogermani · 2026-03-11T14:58:20Z

includes/reader-activation/integrations/class-esp.php

 		}

-		$master_list_id = Reader_Activation::get_esp_master_list_id();
+		if ( Audience_Integrations::is_enabled() ) {


Again we should not have this check. We should migrate the options to the integration the first time we try to fetch it. Like we did here:

https://github.com/Automattic/newspack-plugin/pull/4544/changes#diff-b7214d34a99a1e90de19391b14bc5da959a54f98dd690ae60b2cebb384a4b89fR45

Great call! Updated in bc1448a

leogermani · 2026-03-11T15:04:42Z

includes/reader-activation/integrations/class-integration.php

-	public function set_selected_fields( $fields ) {
-		return \update_option( self::OPTION_PREFIX . $this->id, array_values( $fields ) );
+	public function get_enabled_outgoing_fields() {
+		return array_values( \get_option( self::OUTGOING_FIELDS_OPTION_PREFIX . $this->id, Sync\Metadata::get_default_fields() ) );


The default currently is an empty array. I think that's better

leogermani · 2026-03-11T15:05:46Z

includes/reader-activation/integrations/class-integration.php

 	 */
-	public function get_selected_fields() {
-		return \get_option( self::OPTION_PREFIX . $this->id, [] );
+	public function get_incoming_fields() {


should be get_enabled_incoming_fields?

I originally renamed this while looking at the POC which allowed users to enter any field values. But seeing this has changes, I've renamed all of the relevant methods to get_enabled_...

leogermani · 2026-03-11T15:06:20Z

includes/reader-activation/integrations/class-integration.php

 	 */
-	public function get_enabled_outgoing_fields() {
-		return array_values( \get_option( self::OUTGOING_FIELDS_OPTION_PREFIX . $this->id, Sync\Metadata::get_default_fields() ) );
+	public function update_incoming_fields( $fields ) {


should be update_enabled

Fixed in 05014d7

leogermani · 2026-03-11T15:11:23Z

includes/reader-activation/sync/class-metadata.php

 	 */
 	public static function get_prefix() {
+		// When the integrations feature is enabled, read prefix from the ESP integration.
+		if ( \Newspack\Audience_Integrations::is_enabled() ) {


remove this check and directly route it to the esp integration like we did in other methods in this class

Updated in bc1448a

leogermani · 2026-03-11T15:12:21Z

includes/reader-activation/class-integrations.php


-		// hardcode ESP integration as enabled for now.
-		self::enable( 'esp' );
+		// Auto-enable all integrations if the option has never been saved.


This was arbitrarily done by Claude and I never revisited. I reverted this to hardcode ESP in 05014d7

leogermani · 2026-03-11T15:13:55Z

includes/reader-activation/class-reader-activation.php

 		}
+
+		// When integration settings are enabled, route ESP settings to the integration.
+		if ( Audience_Integrations::is_enabled() ) {


Let's avoid this check and simply route the settings that we moved to the esp integration directly

Done in bc1448a

chickenn00dle · 2026-03-11T21:13:15Z

Thanks for the review @leogermani! Just noting I've tackled most of the feedback, but ran into one small issue while making changes and I'm out of time today so will have to pick this back up tomorrow.

chickenn00dle force-pushed the feat/add-integrations-settings branch from a2bf584 to 76aa8b3 Compare March 4, 2026 17:08

chickenn00dle commented Mar 4, 2026

View reviewed changes

includes/reader-activation/integrations/class-contact-pull.php Outdated Show resolved Hide resolved

chickenn00dle force-pushed the feat/add-integrations-settings branch from 39a7855 to 04f1105 Compare March 9, 2026 16:30

chickenn00dle changed the title ~~feat: add integrations settings~~ feat(integrations): add integrations settings Mar 9, 2026

chickenn00dle commented Mar 9, 2026

View reviewed changes

chickenn00dle marked this pull request as ready for review March 9, 2026 18:06

chickenn00dle requested a review from a team as a code owner March 9, 2026 18:06

Copilot AI review requested due to automatic review settings March 9, 2026 18:06

Copilot started reviewing on behalf of chickenn00dle March 9, 2026 18:06 View session

Copilot AI reviewed Mar 9, 2026

View reviewed changes

chickenn00dle added the [Status] Needs Review The issue or pull request needs to be reviewed label Mar 9, 2026

chickenn00dle added 12 commits March 11, 2026 12:25

feat: add integrations settings

767179b

feat: move sync settings

ec4f839

feat: add custom meta field

af1498f

fix: tests

0b5a09a

fix: register metadata fields for every integration

0e25367

fix: utilize metadata refactor

2a1df01

fix: update incoming getter/setter methods

7ab3aaf

fix: integration level metadata prefix

941e56e

fix: add unit tests

6a6902d

fix: use esp integration settings

38addbd

fix: copilot suggestions

d8a89f2

fix: phpunit

31e5936

leogermani requested changes Mar 11, 2026

View reviewed changes

chickenn00dle added 3 commits March 11, 2026 14:22

fix: feedback round 1 changes

05014d7

fix: lazy migrate settings

bc1448a

fix: incoming metadata settings

8a6fb10

chickenn00dle force-pushed the feat/add-integrations-settings branch from 5d22ba4 to 8a6fb10 Compare March 11, 2026 21:00

fix: unit tests

11cdebd

chickenn00dle force-pushed the feat/add-integrations-settings branch from 99af4ba to 11cdebd Compare March 11, 2026 21:14

Conversation

chickenn00dle commented Mar 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

All Submissions:

Changes proposed in this Pull Request:

How to test the changes in this Pull Request:

Other information:

Uh oh!

Uh oh!

chickenn00dle Mar 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

chickenn00dle commented Mar 9, 2026

Uh oh!

leogermani commented Mar 10, 2026

Uh oh!

leogermani left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chickenn00dle commented Mar 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

chickenn00dle commented Mar 4, 2026 •

edited

Loading

chickenn00dle Mar 9, 2026 •

edited

Loading