Skip to content

Commit

Permalink
fix(webhooks): deprecate global endpoint (#3492)
Browse files Browse the repository at this point in the history
  • Loading branch information
@miguelpeixe
miguelpeixe authored Oct 25, 2024
1 parent cd0b859 commit 63e8ab2
Show file tree
Hide file tree
Showing 5 changed files with 131 additions and 118 deletions.
17 changes: 6 additions & 11 deletions includes/data-events/class-api.php
View file
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
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
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
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

0 comments on commit 63e8ab2

Please sign in to comment.