(chore): make post SyncManager extendable

[edwinsiebel]

Post SyncManager has hardcoded 'post'

In the syncmanager for post, the "post type" is hardcode: 'post'. This makes it very hard (almost impossible) to extend the syncmanager when wanting to have a syncmanager for a custom post type.

Alternate Designs

The syncmanager in this PR still has the default 'post' posttype, yet is possible to be overridden.
Just create a new CustomPostSyncManager which extends SyncManager (the post syncmanager that is), and set:

public $indexable_slug = 'my_cpt';

Benefits

More DRY, less code.

Possible Drawbacks

None. still the same settings.

Verification Process

All the syncing is still possible, with the same results.

Checklist:

• I have read the CONTRIBUTING document.
• My code follows the code style of this project.
• [-] My change requires a change to the documentation.
• [-] I have updated the documentation accordingly.
• [-] I have added tests to cover my change.
• [] All new and existing tests passed.

Applicable Issues

None

Changelog Entry

Post SyncManager can now be used for a custom post type.

[felipeelia]

Hi @edwinsiebel,

Do you mind sharing with us what is your use case? Although we have that post there, it should be just a reference to the Indexable, not the post type. In fact, EP will index all public post types (default and custom), as you can see here. If your CPT is not public, you can add it to the list with the ep_indexable_post_types filter.

[edwinsiebel]

Hi @felipeelia,

Our use case is to use the Indexable concept per post type; this so the mapping per post type can be different, and the data we push to ES per post type can be managed (customized).

I really like the idea of Indexables, but the problem as now is that you need to have a custom SyncManager together with the Custom Indexable. The custom indexable is fine, great even. The custom sync manager would we overkill, as the defaults for Post SyncManager are fine for cpts as well.

I hope I explained my use case with clarity.

[oscarssanchez]

If I get the overall idea right @edwinsiebel once you have an indexable for your CPT, will you make use of the EP filters to query the specific path for the CPT indexable based on the query args post type? Won't you have mixed results if you happen to have a query with all post types?

[edwinsiebel]

hi @oscarssanchez ,

True, that might happen. However, I presume you're working more towards Indexables per (custom) post type or a group of (custom) post types, right?
The thing I'm missing (and try to solve), it more control what is sent to ES per cpt, eventhought the data could be uniform (as cpts are pretty uniform).

In the future, it might a nice change to have a 'post' and a 'page' indexable as default, and have the possibility to add more (custom) indexables.

However, my change does not change the way ElasticPress works, however makes it more extendable, if needed.

[oscarssanchez]

Hi @edwinsiebel ,

Thanks for the additional information, we have met internally and think this is ok to merge. Would you mind submitting another commit for the linting problems?

Thanks!

[edwinsiebel]

Hi @oscarssanchez ,

I'm trying to do the linting (and fixing), however, I get the following error:

➜  elasticpress git:(feat/extendable-post-syncmanager) composer run lint-fix    
The "dealerdirect/phpcodesniffer-composer-installer" plugin was skipped because it requires a Plugin API version ("^1.0") that does not match your Composer installation ("2.0.0"). You may need to run composer update with the "--no-plugins" option.
The "composer/installers" plugin was skipped because it requires a Plugin API version ("^1.0") that does not match your Composer installation ("2.0.0"). You may need to run composer update with the "--no-plugins" option.
> phpcbf elasticpress.php includes
ERROR: Referenced sniff "10up-Default" does not exist

Run "phpcbf --help" for usage information

Script phpcbf elasticpress.php includes handling the lint-fix event returned with error code 3

Any idea what I'm doing wrong?

[oscarssanchez]

Hi @edwinsiebel

Are you running composer v2 ? It seems this could be the cause. Downgrading for a moment should resolve for now.

[edwinsiebel]

Hi @oscarssanchez ,

Ah, that was the case. Linting is now 'green'.

[oscarssanchez]

Hi @edwinsiebel can we solve the lint issues detected please https://github.com/10up/ElasticPress/pull/2196/checks?check_run_id=2509586826#step:5:17

[edwinsiebel]

Hi @oscarssanchez , seems this change is outdated (GH also mentions it).

The correct code is: https://github.com/10up/ElasticPress/pull/2196/files#diff-f92959da0a1c85031962be4852910a0972795a92f0a67cdb4f4fd04f9561cd40R32, which is different from the one that you're merging.