Ensure validation error sources property is always iterable

[delawski]

Summary

Fixes #6827

In some specific cases, the sources property returned with a validation error may not be an array (it is null instead). However, in the Site Scan feature we're relying on a fact that sources is iterable. When it's not the case, an error is thrown in WP admin.

This PR fixes this issue in the following ways:

1. 79c528b On server-side, we're ensuring the sources property is an array from the very beginning (as suggested in Error: s.sources is not iterable #6827 (comment) and the next comment).
2. f3591a5 Moreover, on client-side, we're checking if sources is a non-empty array before attempting to iterate over it.

Checklist

• My code is tested and passes existing tests.
• My code follows the Engineering Guidelines (updates are often made to the guidelines, check it out periodically).

[delawski]

@westonruter As noted in the original issue, I was not able to reproduce this error locally. So the backend part that you suggested is added as-is as I had no way to test it out.

When it comes to the JS part, I wrote some unit tests in order to make sure the issue is fixed.

[github-actions]

Plugin builds for 87a0424 are ready 🛎️!

• Download development build
• Download production build

[westonruter]

This change isn't needed due to the JS changes, right? Maybe we should leave the PHP changes as-is until we can identify the underlying cause. So only fix the symptoms until we can fix the underlying cause.

So that would mean reverting the PHP changes in this PR but leaving the JS changes.

[delawski]

Sounds good, I'll revert the backend changes.

[westonruter]

Would it be worthwhile to retain the PHP commit and its revert commit so that we can refer back to it when we return to identifying the underlying cause? I'm not bothered by such commits in the history 😄

[delawski]

Sure! Since the commits were reverted and then squashed, the history got messed. I've recreated the original commit along with the unit tests update (50e557b) and then reverted it (525b95c). It's now part of the git history.

[westonruter]

Rebasing onto develop will fix the PHPUnit failures.

[westonruter]

I tested this by manually forcing null sources via:

--- a/includes/validation/class-amp-validation-manager.php
+++ b/includes/validation/class-amp-validation-manager.php
@@ -790,7 +790,7 @@ public static function add_validation_error( array $error, array $data = [] ) {
 			$node = $data['node'];
 		}
 
-		if ( self::is_validate_request() ) {
+		if ( self::is_validate_request() && 0 ) {
 			if ( ! empty( $error['sources'] ) ) {
 				$sources = $error['sources'];
 			} elseif ( $node ) {

And:

--- a/includes/validation/class-amp-validation-manager.php
+++ b/includes/validation/class-amp-validation-manager.php
@@ -1591,7 +1591,7 @@ public static function decorate_filter_source( $value ) {
 		if ( isset( self::$current_hook_source_stack[ current_filter() ] ) ) {
 			$sources = self::$current_hook_source_stack[ current_filter() ];
 			array_pop( $sources ); // Remove self.
-			$source['sources'] = $sources;
+			$source['sources'] = null;
 		}
 		return implode(
 			'',

Before the changes in this PR I got an error on the Settings screen:

But after the changes in this PR, I no longer got any errors on:

1. Settings screen
2. Onboarding Wizard
3. Plugin activation