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

Facet by Meta #2954

Merged
merged 15 commits into from
Aug 26, 2022
Merged

Facet by Meta #2954

merged 15 commits into from
Aug 26, 2022

Conversation

felipeelia
Copy link
Member

Description of the Change

This PR is the following step to #2919, implementing a new type for meta fields.

It still needs tests.

How to test the Change

Changelog Entry

Added - New block to facet by meta fields

Credits

Props @felipeelia

@felipeelia felipeelia added this to the 4.3.0 milestone Aug 17, 2022
@felipeelia felipeelia self-assigned this Aug 17, 2022
Copy link
Contributor

@tott tott left a comment

Choose a reason for hiding this comment

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

Just some minor comments. One thing i'm wondering though - How will things work when people have multiple facet blocks of the same type. I did not see any variation in filters based on the block id or the like. Have you tested this?

public function render_block_preview( $request ) {
global $wp_query;

add_filter( 'ep_is_facetable', '__return_true' );
Copy link
Contributor

Choose a reason for hiding this comment

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

would it make sense to provide more context in the filter to be able to make a decision?

Copy link
Member Author

Choose a reason for hiding this comment

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

Addressed in f9ae942. I will apply a similar change to #2919 as well, especially for the renderer class part.


$meta_values = get_transient( self::TRANSIENT_PREFIX . $meta_key );
if ( ! $meta_values ) {
$meta_values = \ElasticPress\Indexables::factory()->get( 'post' )->get_all_distinct_values( "meta.{$meta_key}.raw" );
Copy link
Contributor

Choose a reason for hiding this comment

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

wondering if we need to truncate those values. there could be huge data in meta fields eventually. maybe truncate out only first 100 chars or so?

Copy link
Member Author

Choose a reason for hiding this comment

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

As I can see some usefulness in bringing more data in other scenarios, in 2626fb4 I've added the number as a parameter.

@felipeelia
Copy link
Member Author

With the change made in 7c1b813, this is the code needed to make Facet by Meta blocks available:

add_filter(
	'ep_facet_types',
	function ( $facet_types ) {
		if ( ! isset( $facet_types['meta'] ) && class_exists( '\ElasticPress\Feature\Facets\Types\Meta\FacetType' ) ) {
			$facet_types['meta'] = '\ElasticPress\Feature\Facets\Types\Meta\FacetType';
		}
		return $facet_types;
	}
);

Base automatically changed from feature/facet-types to develop August 25, 2022 20:42
@felipeelia felipeelia changed the title [WIP] Facet by Meta Facet by Meta Aug 26, 2022
@felipeelia felipeelia merged commit fac9cf3 into develop Aug 26, 2022
@felipeelia felipeelia deleted the feature/facet-meta-type branch August 26, 2022 12:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants