Skip to content

fix(webhooks): deprecate global endpoint #3492

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

Merged
miguelpeixe merged 8 commits into from
Oct 25, 2024
Merged

fix(webhooks): deprecate global endpoint #3492

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
8ec1864
fix(webhooks): deprecate global endpoint
miguelpeixe Oct 23, 2024
7a60f7b
chore: update tests
miguelpeixe Oct 23, 2024
4309ea5
chore: fix tests
miguelpeixe Oct 23, 2024
80d94c1
test: add teardown function
miguelpeixe Oct 23, 2024
43550f8
chore: fix tests
miguelpeixe Oct 23, 2024
c275de2
fix: typo
miguelpeixe Oct 23, 2024
31b317b
chore: update textdomain
miguelpeixe Oct 23, 2024
2bb2cdb
feat: refactor Modal and frontend validation
miguelpeixe Oct 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 6 additions & 11 deletions includes/data-events/class-api.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ public static function permission_callback() {
if ( ! current_user_can( 'manage_options' ) ) {
return new \WP_Error(
'newspack_rest_forbidden',
esc_html__( 'You cannot use this resource.', 'newspack' ),
esc_html__( 'You cannot use this resource.', 'newspack-plugin' ),
[
'status' => 403,
]
Expand All @@ -129,9 +129,6 @@ private static function get_endpoint_args() {
'sanitize_callback' => 'sanitize_text_field',
],
],
'global' => [
'type' => 'boolean',
],
'disabled' => [
'type' => 'boolean',
],
Expand Down Expand Up @@ -168,7 +165,7 @@ public static function test_url( $request ) {
[
'success' => false,
'code' => false,
'message' => esc_html__( 'Invalid URL.', 'newspack' ),
'message' => esc_html__( 'Invalid URL.', 'newspack-plugin' ),
]
);
}
Expand Down Expand Up @@ -249,16 +246,16 @@ public static function upsert_endpoint( $request ) {
if ( empty( $args['url'] ) || \esc_url_raw( $args['url'], [ 'https' ] ) !== $args['url'] ) {
return new \WP_Error(
'newspack_webhooks_invalid_url',
esc_html__( 'Invalid URL.', 'newspack' ),
esc_html__( 'Invalid URL.', 'newspack-plugin' ),
[
'status' => 400,
]
);
}
if ( ! $args['global'] && empty( $args['actions'] ) ) {
if ( empty( $args['actions'] ) ) {
return new \WP_Error(
'newspack_webhooks_invalid_actions',
esc_html__( 'You must select actions to trigger this endpoint. Set it to global if this endpoint is meant for all actions.', 'newspack' ),
esc_html__( 'You must select actions to trigger this endpoint.', 'newspack-plugin' ),
[
'status' => 400,
]
Expand All @@ -267,15 +264,13 @@ public static function upsert_endpoint( $request ) {
if ( empty( $args['id'] ) ) {
$endpoint = Webhooks::create_endpoint(
$args['url'],
$args['actions'] ?? [],
$args['global']
$args['actions'] ?? []
);
} else {
$endpoint = Webhooks::update_endpoint(
$args['id'],
$args['url'],
$args['actions'] ?? [],
$args['global'],
$args['disabled']
);
}
Expand Down
44 changes: 32 additions & 12 deletions includes/data-events/class-webhooks.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,16 +220,38 @@ public static function send_late_requests() {
}
}

/**
* Get endpoint errors.
*
* @param string $url Endpoint URL.
* @param array $actions Array of action names.
*
* @return WP_Error
*/
private static function get_endpoint_errors( $url, $actions ) {
$errors = new WP_Error();
if ( empty( $url ) ) {
$errors->add( 'newspack_webhooks_invalid_url', __( 'Invalid URL.', 'newspack' ) );
}
if ( empty( $actions ) ) {
$errors->add( 'newspack_webhooks_empty_actions', __( 'The endpoint must have at least one action.', 'newspack' ) );
}
return $errors;
}

/**
* Create a webhook endpoint.
*
* @param string $url Endpoint URL.
* @param array $actions Array of action names.
* @param bool $global Whether the endpoint should be triggered for all actions.
*
* @return array|WP_Error Endpoint or error.
*/
public static function create_endpoint( $url, $actions = [], $global = false ) {
public static function create_endpoint( $url, $actions ) {
$errors = self::get_endpoint_errors( $url, $actions );
if ( $errors->has_errors() ) {
return $errors;
}
$endpoint = \wp_insert_term(
$url,
self::ENDPOINT_TAXONOMY,
Expand All @@ -241,7 +263,6 @@ public static function create_endpoint( $url, $actions = [], $global = false ) {
return $endpoint;
}
\update_term_meta( $endpoint['term_id'], 'actions', $actions );
\update_term_meta( $endpoint['term_id'], 'global', $global );
return self::get_endpoint_by_term( $endpoint['term_id'] );
}

Expand All @@ -251,12 +272,11 @@ public static function create_endpoint( $url, $actions = [], $global = false ) {
* @param string $id An unique identifier for the endpoint.
* @param string $url Endpoint URL.
* @param array $actions Array of action names.
* @param bool $global Whether the endpoint should be triggered for all actions.
*
* @throws \InvalidArgumentException If the ID is invalid.
* @return void
*/
public static function register_system_endpoint( $id, $url, $actions = [], $global = false ) {
public static function register_system_endpoint( $id, $url, $actions = [] ) {

if ( ! is_string( $id ) || ctype_digit( $id ) || ! preg_match( '/^[a-z0-9-_]+$/', $id ) || ! empty( self::$system_endpoints[ $id ] ) ) {
throw new \InvalidArgumentException( 'Endpoint ID must be a unique string containing only lowercase letters, numbers, dashes and underscores, and it can not be only numerical.' );
Expand All @@ -266,7 +286,6 @@ public static function register_system_endpoint( $id, $url, $actions = [], $glob
'id' => $id,
'url' => $url,
'actions' => $actions,
'global' => $global,
'label' => $id,
'disabled' => false,
'disabled_error' => null,
Expand All @@ -282,12 +301,15 @@ public static function register_system_endpoint( $id, $url, $actions = [], $glob
* @param int $id Endpoint ID.
* @param string $url Endpoint URL.
* @param array $actions Array of action names.
* @param bool $global Whether the endpoint should be triggered for all actions.
* @param bool $disabled Whether the endpoint is disabled.
*
* @return array|WP_Error Endpoint or error.
*/
public static function update_endpoint( $id, $url, $actions = [], $global = false, $disabled = false ) {
public static function update_endpoint( $id, $url, $actions = [], $disabled = false ) {
$errors = self::get_endpoint_errors( $url, $actions );
if ( $errors->has_errors() ) {
return $errors;
}
$endpoint = \get_term( $id, self::ENDPOINT_TAXONOMY, ARRAY_A );
if ( ! $endpoint ) {
return new WP_Error( 'newspack_webhooks_endpoint_not_found', __( 'Webhook endpoint not found.', 'newspack' ) );
Expand All @@ -301,7 +323,6 @@ public static function update_endpoint( $id, $url, $actions = [], $global = fals
]
);
\update_term_meta( $endpoint['term_id'], 'actions', $actions );
\update_term_meta( $endpoint['term_id'], 'global', $global );
\update_term_meta( $endpoint['term_id'], 'disabled', $disabled );
return self::get_endpoint_by_term( $endpoint['term_id'] );
}
Expand Down Expand Up @@ -387,7 +408,6 @@ public static function get_endpoint_by_term( $endpoint ) {
'id' => $endpoint->term_id,
'url' => $endpoint->name,
'actions' => (array) \get_term_meta( $endpoint->term_id, 'actions', true ),
'global' => (bool) \get_term_meta( $endpoint->term_id, 'global', true ),
'label' => \get_term_meta( $endpoint->term_id, 'label', true ),
'bearer_token' => \get_term_meta( $endpoint->term_id, 'bearer_token', true ),
'disabled' => $disabled,
Expand Down Expand Up @@ -514,7 +534,7 @@ private static function get_endpoints_for_action( $action_name ) {
array_filter(
$endpoints,
function( $endpoint ) use ( $action_name ) {
return ! $endpoint['disabled'] && ( $endpoint['global'] || in_array( $action_name, $endpoint['actions'], true ) );
return ! $endpoint['disabled'] && in_array( $action_name, $endpoint['actions'], true );
}
)
);
Expand Down Expand Up @@ -832,7 +852,7 @@ private static function send_request( $request_id ) {
}
$code = \wp_remote_retrieve_response_code( $response );
$message = \wp_remote_retrieve_response_message( $response );

$response_body = wp_remote_retrieve_body( $response );
$response_body = json_decode( $response_body, true );

Expand Down
28 changes: 11 additions & 17 deletions src/components/src/modal/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
/**
* WordPress dependencies.
*/
import { Component } from '@wordpress/element';
import { forwardRef } from '@wordpress/element';
import { Modal as BaseComponent } from '@wordpress/components';

/**
Expand All @@ -18,21 +18,15 @@ import './style.scss';
*/
import classnames from 'classnames';

class Modal extends Component {
/**
* Render.
*/
render() {
const { className, isWide, isNarrow, ...otherProps } = this.props;
const classes = classnames(
'newspack-modal',
isWide && 'newspack-modal--wide',
isNarrow && 'newspack-modal--narrow',
className
);
function Modal( { className, isWide, isNarrow, ...otherProps }, ref ) {
const classes = classnames(
'newspack-modal',
isWide && 'newspack-modal--wide',
isNarrow && 'newspack-modal--narrow',
className
);

return <BaseComponent className={ classes } { ...otherProps } />;
}
}
return <BaseComponent className={ classes } { ...otherProps } ref={ ref } />;

export default Modal;
}
export default forwardRef( Modal );
63 changes: 36 additions & 27 deletions src/wizards/connections/views/main/webhooks.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import moment from 'moment';
*/
import { sprintf, __ } from '@wordpress/i18n';
import { CheckboxControl, MenuItem } from '@wordpress/components';
import { useEffect, useState } from '@wordpress/element';
import { useEffect, useState, useRef } from '@wordpress/element';
import apiFetch from '@wordpress/api-fetch';
import { Icon, settings, check, close, reusableBlock, moreVertical } from '@wordpress/icons';
import { ESCAPE } from '@wordpress/keycodes';
Expand Down Expand Up @@ -152,6 +152,8 @@ const Webhooks = () => {
const [ editing, setEditing ] = useState( false );
const [ editingError, setEditingError ] = useState( false );

const modalRef = useRef( null );

const fetchEndpoints = () => {
setInFlight( true );
apiFetch( { path: '/newspack/v1/webhooks/endpoints' } )
Expand Down Expand Up @@ -200,9 +202,27 @@ const Webhooks = () => {
setDeleting( false );
} );
};
const validateEndpoint = endpoint => {
const errors = [];
if ( ! endpoint.url ) {
errors.push( __( 'URL is required.', 'newspack-plugin' ) );
}
if ( ! endpoint.actions || ! endpoint.actions.length ) {
errors.push( __( 'At least one action is required.', 'newspack-plugin' ) );
}
if ( errors.length ) {
setEditingError( { message: errors.join( ' ' ) } );
} else {
setEditingError( false );
}
return errors;
}
const upsertEndpoint = endpoint => {
const errors = validateEndpoint( endpoint );
if ( errors.length ) {
return;
}
setInFlight( true );
setEditingError( false );
apiFetch( {
path: `/newspack/v1/webhooks/endpoints/${ endpoint.id || '' }`,
method: 'POST',
Expand Down Expand Up @@ -251,6 +271,12 @@ const Webhooks = () => {
setTestError( false );
}, [ editing ] );

useEffect( () => {
if ( editingError ) {
modalRef?.current?.querySelector('.components-modal__content')?.scrollTo( { top: 0, left: 0, behavior: 'smooth' } );
}
}, [ editingError ] );

return (
<Card noBorder className="mt64">
{ false !== error && <Notice isError noticeText={ error.message } /> }
Expand All @@ -266,7 +292,7 @@ const Webhooks = () => {
/>
<Button
variant="primary"
onClick={ () => setEditing( { global: true } ) }
onClick={ () => setEditing( {} ) }
disabled={ inFlight }
>
{ __( 'Add New Endpoint', 'newspack-plugin' ) }
Expand Down Expand Up @@ -298,17 +324,10 @@ const Webhooks = () => {
return (
<>
{ __( 'Actions:', 'newspack-plugin' ) }{ ' ' }
{ endpoint.global ? (
<span className="newspack-webhooks__endpoint__action">
{ __( 'global', 'newspack-plugin' ) }
</span>
) : (
endpoint.actions.map( action => (
<span key={ action } className="newspack-webhooks__endpoint__action">
{ action }
</span>
) )
) }
{ endpoint.actions.map( action => (
<span key={ action } className="newspack-webhooks__endpoint__action">
{ action }
</span> ) ) }
</>
);
} }
Expand Down Expand Up @@ -439,6 +458,7 @@ const Webhooks = () => {
) }
{ false !== editing && (
<Modal
ref={ modalRef }
title={ __( 'Webhook Endpoint', 'newspack-plugin' ) }
onRequestClose={ () => {
setEditing( false );
Expand Down Expand Up @@ -521,32 +541,21 @@ const Webhooks = () => {
/>
<Grid columns={ 1 } gutter={ 16 }>
<h3>{ __( 'Actions', 'newspack-plugin' ) }</h3>
<CheckboxControl
checked={ editing.global }
onChange={ value => setEditing( { ...editing, global: value } ) }
label={ __( 'Global', 'newspack-plugin' ) }
help={ __(
'Leave this checked if you want this endpoint to receive data from all actions.',
'newspack-plugin'
) }
disabled={ inFlight }
/>
{ actions.length > 0 && (
<>
<p>
{ __(
'If this endpoint is not global, select which actions should trigger this endpoint:',
'Select which actions should trigger this endpoint:',
'newspack-plugin'
) }
</p>
<Grid columns={ 2 } gutter={ 16 }>
{ actions.map( ( action, i ) => (
<CheckboxControl
key={ i }
disabled={ editing.global || inFlight }
disabled={ inFlight }
label={ action }
checked={ ( editing.actions && editing.actions.includes( action ) ) || false }
indeterminate={ editing.global }
onChange={ () => {
const currentActions = editing.actions || [];
if ( currentActions.includes( action ) ) {
Expand Down
Loading