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

Feature implementation for #1471 #1472

Merged
merged 18 commits into from
Aug 9, 2021
Merged

Conversation

tott
Copy link
Contributor

@tott tott commented Aug 28, 2019

This changeset implements a way to determine the mapping version currently on the index.
It compares it against the expected mapping version and displays a
dismissable notice.
The notice dismissal is dependent on the es_version
This changeset also introduces the _meta mapping to store the mapping
version for a more predicatble detection

currently on the index.
It compares it against the expected mapping version and displays a
dismissable notice.
The notice dismissal is dependent on the es_version
This changeset also introduces the _meta mapping to store the mapping
version for a more predicatble detection
@tott
Copy link
Contributor Author

tott commented Aug 28, 2019

This fixes #1471

return false;
}

$mapping_file_wanted = '5-2.php';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note to self - Refactor in next version to abstract this out to dedicated function

@tott
Copy link
Contributor Author

tott commented Aug 29, 2019

Tests are currently failing due to the way other tests are structured. Will need to change admin notices test cases to properly reflect the expected mapping version and create new test cases for the version notice

includes/classes/AdminNotices.php Outdated Show resolved Hide resolved
includes/classes/AdminNotices.php Outdated Show resolved Hide resolved
includes/classes/AdminNotices.php Outdated Show resolved Hide resolved
includes/classes/AdminNotices.php Show resolved Hide resolved
@oscarssanchez oscarssanchez self-assigned this Sep 10, 2019
@jeffpaul jeffpaul added this to the Future Release milestone Sep 17, 2019
@petenelson petenelson self-requested a review January 14, 2020 16:54
@petenelson
Copy link
Contributor

@tlovett1 @tott I'm going to take a look at any final cleanup needed for this before we finish/merge the code review

@petenelson petenelson removed their request for review January 17, 2020 19:37
@oscarssanchez
Copy link
Contributor

Should we add the same for other mappings we have? (terms, user, etc) and, if we do it, we should probably rename the new introduced filters. Also needed to update the since tags.

includes/classes/AdminNotices.php Show resolved Hide resolved
}

if ( ! $mapping_file_current || $mapping_file_wanted !== $mapping_file_current ) {
$html = sprintf( '%1$s <em>--setup</em> %2$s', esc_html__( 'It seems the mapping data in your index does not match the Elasticsearch version used. We recommend to reindex your content using the sync button on the top of the screen or through wp-cli by adding the', 'elasticpress' ), esc_html__( 'flag.', 'elasticpress' ) );
Copy link
Member

Choose a reason for hiding this comment

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

In terms of i18n, this string isn't great. It would be much better for translators if this were:

$html = sprintf(
	/* translators: 1. <em>; 2. </em> */
	esc_html__( 'It seems the mapping data in your index does not match the Elasticsearch version used. We recommend to reindex your content using the sync button on the top of the screen or through wp-cli by adding the %1$s--setup%2$s flag', 'elasticpress' ),
	'<em>',
	'</em>'
);

@mckdemps mckdemps assigned felipeelia and unassigned oscarssanchez Jul 27, 2021
@mckdemps mckdemps modified the milestones: Future Release, 3.6.2 Jul 27, 2021
@felipeelia
Copy link
Member

@oscarssanchez, I've fixed WP Acceptance tests in #2281 and pushed a commit addressing a few things here. Do you mind checking everything again?

@oscarssanchez
Copy link
Contributor

Looks good @felipeelia

@oscarssanchez oscarssanchez dismissed stale reviews from tlovett1 and felipeelia August 3, 2021 15:24

Outdated

@oscarssanchez oscarssanchez merged commit 47670f2 into develop Aug 9, 2021
@oscarssanchez oscarssanchez deleted the feature/validate-mapping branch August 9, 2021 02:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants