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

Improve connector registration logic #1546

Merged
merged 2 commits into from
Aug 12, 2024
Merged

Conversation

delawski
Copy link
Contributor

@delawski delawski commented Aug 7, 2024

Fixes #1500.

Connector classes can be validated in a single loop pass and only registered if they pass all the checks. The checks should involve validating that all connector dependencies are satisfied. We should not rely on a third-party to do that for us.

Sample Connector

Note: The following is intended to replace the connector example presented in https://github.com/xwp/stream/wiki/Creating-a-Custom-Connector

A custom connector can be really simple. There are just a few tasks it must do:

  • Register the connector class in Stream.
  • Define action or actions (WordPress hooks) along with a callback function which should be triggered whenever an action is called. The callback function is responsible for creating the actual log record.
  • Define labels used on the admin screens.

Below, you'll find an example of a connector plugin. It's a Stream Observer which logs visits to the Stream Settings and Alerts admin screens. While it would not be useful in a real-life scenario, it can show basic concepts behind Stream connectors really well.

The plugin consists of only 2 files. In the first one, the main file containing the necessary WordPress plugin header, the connector class is registered in Stream via the wp_stream_connectors filter:

<?php
/**
 * Plugin Name: Stream Observer Connector
 *
 * @package Stream_Observer
 */

namespace Stream_Observer;

/**
 * Register the Stream Observer connector.
 *
 * @param array $classes Array of connector class names.
 *
 * @return array
 */
add_filter(
	'wp_stream_connectors',
	function ( $classes ) {
		require plugin_dir_path( __FILE__ ) . '/classes/class-connector.php';

		$classes[] = new Connector();

		return $classes;
	}
);

The second file contains the connector class definition:

<?php
/**
 * Stream Observer - a connector to log Stream admin screens visits.
 *
 * @package Stream_Observer
 */

namespace Stream_Observer;

class Connector extends \WP_Stream\Connector {
	/**
	 * "Visited" action slug.
	 */
	const ACTION_VISITED = 'visited';

	/**
	 * "Admin" context slug.
	 */
	const CONTEXT_ADMIN = 'admin';

	/**
	 * Connector slug.
	 *
	 * @var string
	 */
	public $name = 'stream_observer';

	/**
	 * Actions registered for this connector.
	 *
	 * These are actions (WordPress hooks) that the Stream_Observer connector
	 * will listen to. Whenever an action from this list is triggered, Stream
	 * will run a callback function defined in the connector class.
	 *
	 * The callback function names follow the format: `callback_{action_name}`.
	 *
	 * @var array
	 */
	public $actions = array(
		'current_screen',
	);

	/**
	 * Return translated connector label.
	 *
	 * @return string
	 */
	public function get_label() {
		return __( 'Stream Observer', 'stream-observer' );
	}

	/**
	 * Return translated context labels.
	 *
	 * @return array
	 */
	public function get_context_labels() {
		return array(
			self::CONTEXT_ADMIN => __( 'Admin', 'stream-observer' ),
		);
	}

	/**
	 * Return translated action labels.
	 *
	 * @return array
	 */
	public function get_action_labels() {
		return array(
			self::ACTION_VISITED => __( 'Visited', 'stream-observer' ),
		);
	}

	/**
	 * Track `current_screen` action and log Stream screens visits.
	 *
	 * @param \WP_Screen $current_screen Current `WP_Screen` object.
	 *
	 * @return void
	 */
	public function callback_current_screen( $current_screen ) {
		if ( ! ( $current_screen instanceof \WP_Screen ) ) {
			return;
		}

		// We want to log visits to the Alerts or Settings screens.
		$tracked_screens = array(
			'edit-wp_stream_alerts'          => __( 'Alerts', 'stream-observer' ),
			'stream_page_wp_stream_settings' => __( 'Settings', 'stream-observer' ),
		);

		if ( ! isset( $tracked_screens[ $current_screen->id ] ) ) {
			return;
		}

		$this->log(
			/* translators: %1$s: Stream admin screen title, e.g. "Alerts".. */
			__( 'Stream "%1$s" screen visited', 'stream-observer' ),
			// This array will be compacted and saved as Stream meta.
			array(
				'title' => $tracked_screens[ $current_screen->id ],
				'id'    => $current_screen->id,
			),
			// Stream admin screens don't use numeric IDs so we set object ID to 0.
			0,
			self::CONTEXT_ADMIN,
			self::ACTION_VISITED,
		);
	}
}

Here you'll find the list of actions the connector should listen to:

public $actions = array(
	'current_screen',
);

Along with a callback function which calls $this->log( ... ) to store the record:

public function callback_current_screen( $current_screen )

Checklist

  • Project documentation has been updated to reflect the changes in this pull request, if applicable.
  • I have tested the changes in the local development environment (see contributing.md).
  • I have added phpunit tests.

Release Changelog

  • Fix: Do not rely on third-parties in checking if a custom connector dependencies are satisfied

Connector classes can be validated in a single loop pass and only registered if they pass all the checks. The checks should involve validating that all the connector dependencies are satisfied.
@delawski delawski marked this pull request as ready for review August 9, 2024 09:26
@delawski delawski requested a review from tharsheblows August 9, 2024 09:26
@delawski delawski added this to the 4.1.0 milestone Aug 9, 2024
Copy link
Contributor

@tharsheblows tharsheblows left a comment

Choose a reason for hiding this comment

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

@delawski This is great! It's so much tidier. I have one question that is a question so I'm leaving this approved but not merged simply because I want to know the answer. 😊

// Bail if no class loaded.
if ( ! class_exists( $class_name ) ) {
// Bail if no class loaded or it does not extend WP_Stream\Connector.
if ( ! class_exists( $class_name ) || ! is_subclass_of( $class_name, 'WP_Stream\Connector' ) ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 thought
I keep going back and forth on this. There's a check on is_subclass_of() below, is there a performance reason for it here also? (I'm asking because I don't know!)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! No, I don't think we need that check here. I think it must have slipped through my fingers when doing another iteration on this logic. I'm going to remove it 👍

@delawski delawski merged commit 8fb1b59 into develop Aug 12, 2024
2 checks passed
@delawski delawski deleted the feature/1500-custom-connectors branch August 12, 2024 11:37
@delawski
Copy link
Contributor Author

I have merged this PR and updated the https://github.com/xwp/stream/wiki/Creating-a-Custom-Connector page contents with a further polished (thanks to GPT) connector example.

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.

Add documentation on how to create new connectors
2 participants