fix: error handling, tests, CI builds (#1641, #1642)

Co-authored-by: dkoo <derrick.koo@automattic.com> Co-authored-by: leogermani <leogermani@automattic.com>
Automattic · Sep 4, 2024 · 791fdc3 · 791fdc3
1 parent 5e636ed
commit 791fdc3
Show file tree

Hide file tree

Showing 9 changed files with 98 additions and 105 deletions.
diff --git a/includes/class-newspack-newsletters-renderer.php b/includes/class-newspack-newsletters-renderer.php
@@ -1012,8 +1012,13 @@ function ( $block ) {
 			 * Embed block.
 			 */
 			case 'core/embed':
-				$oembed = _wp_oembed_get_object();
-				$data   = $oembed->get_data( $attrs['url'] );
+				/**
+				 * Filters the retrieval of the WP oEmbed object. Used for testing purposes
+				 *
+				 * @param WP_oEmbed $oembed WP_oEmbed object.
+				 */
+				$oembed = apply_filters( 'newspack_newsletters_get_oembed_object', _wp_oembed_get_object() );
+				$data = $oembed->get_data( $attrs['url'] );
 
 				if ( ! $data || empty( $data->type ) ) {
 					break;
@@ -1063,7 +1068,7 @@ function ( $block ) {
 							'height' => $data->height,
 							'href'   => $attrs['url'],
 						);
-						$markup   .= '<mj-image ' . self::array_to_attributes( $img_attrs ) . ' />';
+						$markup .= '<mj-image ' . self::array_to_attributes( $img_attrs ) . ' />';
 						if ( ! empty( $caption ) ) {
 							$markup .= '<mj-text ' . self::array_to_attributes( $caption_attrs ) . '>' . esc_html( $caption ) . ' - ' . esc_html( $data->provider_name ) . '</mj-text>';
 						}

diff --git a/...ce-providers/active_campaign/class-newspack-newsletters-active-campaign-usage-reports.php b/...ce-providers/active_campaign/class-newspack-newsletters-active-campaign-usage-reports.php
@@ -144,11 +144,15 @@ private static function get_default_campaign_data() {
 	 */
 	private function get_campaign_data( $last_n_days ) {
 		$last_campaigns_data = get_option( self::LAST_CAMPAIGNS_DATA_OPTION_NAME );
-		$campaigns_data      = self::get_default_campaign_data();
 
 		$current_campaign_data = $this->get_current_campaign_data( $last_n_days );
+		if ( \is_wp_error( $current_campaign_data ) ) {
+			return $current_campaign_data;
+		}
 		update_option( self::LAST_CAMPAIGNS_DATA_OPTION_NAME, $current_campaign_data );
 
+		$campaigns_data = self::get_default_campaign_data();
+
 		if ( ! $last_campaigns_data ) {
 			// No data about campaigns yet, so there is nothing to compare the new data with.
 			return $campaigns_data;

diff --git a/...es/service-providers/constant_contact/class-newspack-newsletters-constant-contact-sdk.php b/...es/service-providers/constant_contact/class-newspack-newsletters-constant-contact-sdk.php
@@ -644,7 +644,7 @@ public function remove_contacts_from_lists( $contact_ids, $list_ids ) {
 	public function upsert_contact( $email_address, $data = [] ) {
 		$contact = $this->get_contact( $email_address );
 		$body    = [];
-		if ( $contact ) {
+		if ( $contact && ! \is_wp_error( $contact ) ) {
 			$body = [
 				'email_address'    => get_object_vars( $contact->email_address ),
 				'list_memberships' => $contact->list_memberships,

diff --git a/includes/service-providers/mailchimp/class-newspack-newsletters-mailchimp-usage-reports.php b/includes/service-providers/mailchimp/class-newspack-newsletters-mailchimp-usage-reports.php
@@ -99,7 +99,7 @@ private static function get_list_activity_reports( $days_in_past_count = 1 ) {
 	 * Get usage reports for last n days.
 	 *
 	 * @param int $days_in_past How many days in past.
-	 * @return Newspack_Newsletters_Service_Provider_Usage_Report[] Usage reports.
+	 * @return Newspack_Newsletters_Service_Provider_Usage_Report[]|WP_Error Usage reports or error.
 	 */
 	public static function get_usage_reports( $days_in_past ) {
 
@@ -179,10 +179,13 @@ public static function get_usage_reports( $days_in_past ) {
 	/**
 	 * Creates a usage report.
 	 *
-	 * @return Newspack_Newsletters_Service_Provider_Usage_Report Usage report.
+	 * @return Newspack_Newsletters_Service_Provider_Usage_Report|WP_Error Usage report or error.
 	 */
 	public static function get_usage_report() {
 		$reports = self::get_usage_reports( 1 );
+		if ( \is_wp_error( $reports ) ) {
+			return $reports;
+		}
 		return reset( $reports );
 	}
 }
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -39,9 +39,7 @@
 		"url": "https://github.com/Automattic/newspack-newsletters/issues"
 	},
 	"dependencies": {
-		"@uiw/react-codemirror": "^3.2.10",
 		"classnames": "^2.5.1",
-		"csslint": "^1.0.5",
 		"mjml-browser": "^4.15.3",
 		"newspack-components": "^3.0.0",
 		"qs": "^6.13.0"

diff --git a/src/newsletter-editor/styling/index.js b/src/newsletter-editor/styling/index.js
@@ -3,37 +3,19 @@
 /**
  * WordPress dependencies
  */
+import { PlainText } from '@wordpress/block-editor';
 import { compose, useInstanceId } from '@wordpress/compose';
 import { ColorPicker, BaseControl, Panel, PanelBody, PanelRow } from '@wordpress/components';
 import { __ } from '@wordpress/i18n';
 import { useSelect, withDispatch, withSelect } from '@wordpress/data';
 import { useEffect, useRef } from '@wordpress/element';
 import SelectControlWithOptGroup from '../../components/select-control-with-optgroup/';
 
-/**
- * External dependencies
- */
-import { CSSLint } from 'csslint';
-import CodeMirror from '@uiw/react-codemirror';
-// eslint-disable-next-line import/no-extraneous-dependencies
-import 'codemirror/mode/css/css';
-// eslint-disable-next-line import/no-extraneous-dependencies
-import 'codemirror/addon/lint/lint';
-// eslint-disable-next-line import/no-extraneous-dependencies
-import 'codemirror/addon/lint/lint.css';
-// eslint-disable-next-line import/no-extraneous-dependencies
-import 'codemirror/addon/lint/css-lint';
-
 /**
  * Internal dependencies
  */
 import './style.scss';
 
-/**
- * Add CSSLint to global scope for CodeMirror.
- */
-window.CSSLint = window.CSSLint || CSSLint;
-
 const fontOptgroups = [
 	{
 		label: __( 'Sans Serif', 'newspack-newsletters' ),
@@ -264,18 +246,12 @@ export const Styling = compose( [
 						) }
 						hideLabelFromVision
 					>
-						<CodeMirror
+						<PlainText
 							className="components-textarea-control__input"
 							value={ customCss }
-							height={ 250 }
-							onChange={ instance => editPost( { meta: { custom_css: instance.getValue() } } ) }
-							options={ {
-								gutters: [ 'CodeMirror-lint-markers' ],
-								height: 'auto',
-								indentWithTabs: true,
-								mode: 'css',
-								lint: true,
-							} }
+							onChange={ content => editPost( { meta: { custom_css: content } } ) }
+							aria-label={ __( 'Custom CSS', 'newspack-newsletters' ) }
+							placeholder={ __( 'Write custom CSS…', 'newspack-newsletters' ) }
 						/>
 					</BaseControl>
 				</PanelRow>

diff --git a/src/newsletter-editor/styling/style.scss b/src/newsletter-editor/styling/style.scss
@@ -26,8 +26,28 @@
 		.components-base-control {
 			max-width: 100%;
 		}
-		.CodeMirror {
-			border: 1px solid wp-colors.$gray-700;
+		.block-editor-plain-text {
+			background: #fff !important;
+			border: 1px solid #1e1e1e !important;
+			border-radius: 2px !important;
+			box-shadow: none !important;
+			box-sizing: border-box;
+			color: #1e1e1e !important;
+			direction: ltr;
+			font-family: Menlo, Consolas, monaco, monospace !important;
+			font-size: 16px !important;
+			max-height: 250px;
+			padding: 12px !important;
+		}
+		@media (min-width: 600px) {
+			.block-editor-plain-text {
+				font-size: 13px !important;
+			}
+		}
+		.block-editor-plain-text:focus {
+			border-color: var(--wp-admin-theme-color) !important;
+			box-shadow: 0 0 0 1px var(--wp-admin-theme-color) !important;
+			outline: 2px solid #0000 !important;
 		}
 	}
 }

diff --git a/tests/test-renderer.php b/tests/test-renderer.php
@@ -57,28 +57,48 @@ public function test_render_mjml_component() {
 	}
 
 	/**
-	 * Test embed blocks rendering.
+	 * Filter the OEmbed return value.
+	 *
+	 * @param array $data The data to return.
 	 */
-	public function test_render_embed_blocks() {
-		// Make the embed request return a static custom response.
+	public function set_oembed_value( $data = [] ) {
+		global $newspack_newsletters_test_oembed_data;
+		$newspack_newsletters_test_oembed_data = $data;
 		add_filter(
-			'http_response',
-			function( $response ) {
-				$response['body'] = wp_json_encode(
-					array_merge(
-						json_decode( $response['body'], true ),
-						[
-							'title'         => 'Embed',
-							'url'           => 'embed.com',
-							'thumbnail_url' => 'embed.com/image',
-							'html'          => '<div>Embed</div>',
-						]
-					)
-				);
-				return $response;
+			'newspack_newsletters_get_oembed_object',
+			function() {
+				return new class() {
+					public function get_data() { // phpcs:disable Squiz.Commenting.FunctionComment.Missing
+						global $newspack_newsletters_test_oembed_data;
+						return (object) array_merge(
+							[
+								'title'            => 'Embed',
+								'url'              => 'embed.com',
+								'width'            => 480,
+								'height'           => 360,
+								'thumbnail_url'    => 'embed.com/image',
+								'thumbnail_width'  => 480,
+								'thumbnail_height' => 360,
+								'html'             => 'Embed',
+							],
+							$newspack_newsletters_test_oembed_data
+						);
+					}
+				};
 			}
 		);
+	}
 
+	/**
+	 * Test embed blocks rendering.
+	 */
+	public function test_render_embed_blocks() {
+		$this->set_oembed_value(
+			[
+				'type'          => 'video',
+				'provider_name' => 'YouTube',
+			]
+		);
 		// Video embed.
 		$inner_html = '<figure><div>https://www.youtube.com/watch?v=aIRgcb3cQ1Q</div></figure>';
 		$this->assertEquals(
@@ -97,6 +117,12 @@ function( $response ) {
 			'Renders image from video embed block with title as caption'
 		);
 
+		$this->set_oembed_value(
+			[
+				'type'          => 'photo',
+				'provider_name' => 'Flickr',
+			]
+		);
 		// Image with custom caption.
 		$inner_html = '<figure><div>https://flickr.com/photos/thomashawk/9274246246</div><figcaption>Automattic</figcaption></figure>';
 		$this->assertEquals(
@@ -111,13 +137,19 @@ function( $response ) {
 					'innerHTML'    => $inner_html,
 				]
 			),
-			'<mj-section url="https://flickr.com/photos/thomashawk/9274246246" padding="0"><mj-column padding="12px" width="100%"><mj-image src="embed.com" alt="Automattic" width="500" height="333" href="https://flickr.com/photos/thomashawk/9274246246" /><mj-text align="center" color="#555d66" line-height="1.56" font-size="13px" >Automattic - Flickr</mj-text></mj-column></mj-section>',
+			'<mj-section url="https://flickr.com/photos/thomashawk/9274246246" padding="0"><mj-column padding="12px" width="100%"><mj-image src="embed.com" alt="Automattic" width="480" height="360" href="https://flickr.com/photos/thomashawk/9274246246" /><mj-text align="center" color="#555d66" line-height="1.56" font-size="13px" >Automattic - Flickr</mj-text></mj-column></mj-section>',
 			'Renders image with inline figcaption as caption'
 		);
 
 		// Rich embed as HTML.
-		$inner_html       = '<figure><div>https://twitter.com/automattic/status/1395447061336711181</div></figure>';
-		$rendered_string  = Newspack_Newsletters_Renderer::render_mjml_component(
+		$inner_html = '<figure><div>https://twitter.com/automattic/status/1395447061336711181</div></figure>';
+		$this->set_oembed_value(
+			[
+				'type'          => 'rich',
+				'provider_name' => 'Twitter',
+			]
+		);
+		$rendered_string = Newspack_Newsletters_Renderer::render_mjml_component(
 			[
 				'blockName'    => 'core/embed',
 				'attrs'        => [