In search_api_default_content_deploy we extend the ContentEntity Datasource and just overwrite the getItemId() method.
Unfortunately getPartialItemIds() has it's own redundant logic to build the item ID.

I think getPartialItemIds() should leverage getItemId() instead for a better DX.

Issue fork search_api-3471987

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

mkalkbrenner created an issue. See original summary.

mkalkbrenner opened merge request !165

mkalkbrenner’s picture

mkalkbrenner
Primary language German
Location 🇩🇪
commented
Status: Active » Needs review
mkalkbrenner’s picture

mkalkbrenner
Primary language German
Location 🇩🇪
commented

The API is inconsistent.
getItemId() checks for the current config.
getItemIds() doesn’t.
So getItemId() is not suitable for deletions caused by config changes.

I consider that to be a bug.

mkalkbrenner’s picture

mkalkbrenner
Primary language German
Location 🇩🇪
commented

My initial approach was to call getItemId() from getItemIds() but due to the different behaviour the tests failed.
But as this is a bigger issue I now introduced formatItemId() to have something small to overwrite.

drunken monkey made their first commit to this issue’s fork.

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented

Thanks a lot for suggesting this! The changes do make sense, great if this helps other modules provide their own entity-based datasources.
I had a few remarks and pushed the associated suggested changes. If you are happy with those, I’ll merge.

mkalkbrenner’s picture

mkalkbrenner
Primary language German
Location 🇩🇪
commented

Looks good to me. I'll test the latest version of the patch on Monday.

mkalkbrenner’s picture

mkalkbrenner
Primary language German
Location 🇩🇪
commented
Status: Needs review » Reviewed & tested by the community

I adjusted the code in search_api_default_content_deploy to the latest changes and it works :-)

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Status: Reviewed & tested by the community » Fixed

Good to hear, thanks for testing!
Merged.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.