payload icon indicating copy to clipboard operation
payload copied to clipboard

Search records don't get deleted when related document is deleted

Open TimLanzi opened this issue 1 year ago • 7 comments

Describe the Bug

When I delete a document that is synced with the search plugin, the search result document is still available.

Link to the code that reproduces this issue

N/A

Reproduction Steps

  1. Use create-payload-app. Use website template and postgres database.
  2. Create account and seed database.
  3. Delete a Post.
  4. Go to Search Results collection. Deleted post will still be in Search Results.

Which area(s) are affected? (Select all that apply)

plugin: search

Environment Info

Binaries:
  Node: 22.7.0
  npm: 10.8.2
  Yarn: 1.22.22
  pnpm: 9.8.0
Relevant Packages:
  payload: 3.1.0
  next: 15.0.3
  @payloadcms/db-postgres: 3.1.0
  @payloadcms/email-nodemailer: 3.1.0
  @payloadcms/graphql: 3.1.0
  @payloadcms/live-preview: 3.1.0
  @payloadcms/live-preview-react: 3.1.0
  @payloadcms/next/utilities: 3.1.0
  @payloadcms/payload-cloud: 3.1.0
  @payloadcms/plugin-form-builder: 3.1.0
  @payloadcms/plugin-nested-docs: 3.1.0
  @payloadcms/plugin-redirects: 3.1.0
  @payloadcms/plugin-search: 3.1.0
  @payloadcms/plugin-seo: 3.1.0
  @payloadcms/richtext-lexical: 3.1.0
  @payloadcms/translations: 3.1.0
  @payloadcms/ui/shared: 3.1.0
  react: 19.0.0-rc-65a56d0e-20241020
  react-dom: 19.0.0-rc-65a56d0e-20241020
Operating System:
  Platform: linux
  Arch: x64
  Version: #202405300957~1732141768~22.04~f2697e1 SMP PREEMPT_DYNAMIC Wed N
  Available memory (MB): 31971
  Available CPU cores: 8

TimLanzi avatar Nov 22 '24 17:11 TimLanzi

Please add a reproduction in order for us to be able to investigate.

Depending on the quality of reproduction steps, this issue may be closed if no reproduction is provided.

Why was this issue marked with the invalid-reproduction label?

To be able to investigate, we need access to a reproduction to identify what triggered the issue. We prefer a link to a public GitHub repository created with create-payload-app@beta -t blank or a forked/branched version of this repository with tests added (more info in the reproduction-guide).

To make sure the issue is resolved as quickly as possible, please make sure that the reproduction is as minimal as possible. This means that you should remove unnecessary code, files, and dependencies that do not contribute to the issue. Ensure your reproduction does not depend on secrets, 3rd party registries, private dependencies, or any other data that cannot be made public. Avoid a reproduction including a whole monorepo (unless relevant to the issue). The easier it is to reproduce the issue, the quicker we can help.

Please test your reproduction against the latest version of Payload to make sure your issue has not already been fixed.

I added a link, why was it still marked?

Ensure the link is pointing to a codebase that is accessible (e.g. not a private repository). "example.com", "n/a", "will add later", etc. are not acceptable links -- we need to see a public codebase. See the above section for accepted links.

Useful Resources

github-actions[bot] avatar Nov 22 '24 17:11 github-actions[bot]

Hey @TimLanzi,

I'm not able to reproduce your issue. The plugin itself hasn't changed much over the past several releases aside from some minor logging stuff. Some more details about your environment would be nice. Are you using db-sqlite by chance? Do you have a custom beforeSync or other hooks that could be interfering? Anything come up in logs?

Try to include a minimal reproduction of the issue you're experiencing because I don't see anything out of the ordinary here.

See here: https://github.com/user-attachments/assets/3cbf09c5-9ae6-4a41-9f75-cac8f9a3f717

akhrarovsaid avatar Nov 22 '24 21:11 akhrarovsaid

Hey @akhrarovsaid

I was using db-postgres in my project. Everything was directly from the website template I get from create-payload-app. I made no changes to that code, and the search result document did not delete automatically. The website template does have a custom beforeSync, but I have now removed that and the custom searchOverrides so my plugin looks like this:

  searchPlugin({
    collections: ['posts'],
  }),

The search result record still does not delete for me with this change.

To get this result:

  1. Use create-payload-app database is PostgreSQL template is Website.
  2. Install dependencies and run dev server.
  3. You can remove the custom beforeSync and searchOverrides in /src/plugins/index.ts
  4. Create your admin account for the application
  5. Click the link at the top of the dashboard to "Seed Your Database"
  6. Go to a Post and delete it.
  7. See that it is still there in the Search Results.

All my environment info is above. I can confirm that with the db-mongo adapter, the delete functions as expected. All my issues have been when I use the db-postgres adpater.

TimLanzi avatar Nov 22 '24 22:11 TimLanzi

@TimLanzi,

Yep, reproduced on monorepo with postgres. I also noticed that the tests in a PR I authored started to fail against relational db's like pg and thought it had something to do with the tests themselves even though the same tests were passing in 3.0.2. This makes a lot of sense. I, however, don't actually think this is an issue with plugin-search as it was pretty much unchanged as I mentioned. I think this has to do with a change somewhere else that was released with 3.1.0. I may be wrong here though.

The last time plugin-search was touched was beta.121.

@r1tsuu You may find this interesting.

akhrarovsaid avatar Nov 22 '24 23:11 akhrarovsaid

looks like the same issue I reported earlier this year which may have been closed by mistake https://github.com/payloadcms/payload/issues/5709

wkentdag avatar Dec 05 '24 03:12 wkentdag

I think this has been closed by mistake!

the commit message in 29ad1fc diverges from the description of #9932. The original description claimed to fix this issue, but was later updated to note that it fixes a tangential problem. The fix for this bug remains WIP in #9623, but somehow the stale PR description got included in the merge (?)

- Fixes https://github.com/payloadcms/payload/issues/9443 (partially, see also: https://github.com/payloadcms/payload/pull/9623)
+ Related https://github.com/payloadcms/payload/issues/9443 (see also: https://github.com/payloadcms/payload/pull/9623)

wkentdag avatar Dec 17 '24 05:12 wkentdag

🚀 This is included in version v3.9.0

github-actions[bot] avatar Dec 18 '24 21:12 github-actions[bot]

Any progress on this?

KoenRijpstra avatar Feb 17 '25 13:02 KoenRijpstra

Would like to confirm that this is still an issue, I'm having to manually delete search indices at the moment.

IGernhardt avatar Mar 24 '25 15:03 IGernhardt

Hey @KoenRijpstra @IGernhardt

The issue here is the afterDelete hook added onto the collections passed to the plugin. The find in that hook fails to surface the related search index. I think it's because, from the perspective of the transaction, after the document has been deleted there is nothing to actually surface with the find - thus the search doc fails to be deleted. The real fix here isn't removing the find from the transaction, but to instead use a beforeDelete hook within the plugin itself instead of an afterDelete.

Try adding the below snippet to the collection configs of the collections that you have enabled in the search plugin.

Sample beforeDelete hook to add to your collections

Let's say you've enabled plugin-search for your pages collection, then in the pages config:

// Pages.ts CollectionConfig
...
hooks: {
  beforeDelete: [
    async ({ collection, id, req: { payload }, req }) => {
      const searchSlug = 'search' // Your search slug
      try {
        await payload.delete({
          collection: searchSlug,
          req,
          where: {
            doc: {
              equals: {
                relationTo: collection.slug,
                value: id,
              },
            },
          },
        })
      } catch (err: unknown) {
        payload.logger.error({
          err,
          msg: `Error deleting ${searchSlug} doc.`,
        })
      }
    },
  ],
},
...

The above snippet is actually a small improvement over the current behavior. It should delete the search index, but it also removes a redundant find in the plugins existing afterDelete hook. We are already doing a payload.delete no need for a payload.find as well.

Let me know if that works for you.

akhrarovsaid avatar Mar 24 '25 16:03 akhrarovsaid

Hi @akhrarovsaid,

Thank you for the code snippet! It works nicely, seems reliable, and I modified it to create a drop in hook across all of my enabled collections. May be premature for me to say, but that seems like a contender to replace the current behavior if the code is adjusted such that the search slug is automatically inferred from the search config. I think re-indexing will still fail to delete entries, however (unless it uses the delete hook under the hood which I doubt).

Here is your code, adjusted slightly to be a universal hook:

// Pages.ts CollectionConfig
...
hooks: {
  beforeDelete: [deleteSearchIndex],
},
...
// deleteSearchIndex.ts Hook
import { CollectionBeforeDeleteHook } from "payload"

export const deleteSearchIndex: CollectionBeforeDeleteHook = async ({ collection, id, req: { payload }, req }) => {
  const searchSlug = 'search' // Your search slug
  try {
    await payload.delete({
      collection: searchSlug,
      req,
      where: {
        doc: {
          equals: {
            relationTo: collection.slug,
            value: id,
          },
        },
      },
    })
  } catch (err: unknown) {
    payload.logger.error({
      err,
      msg: `Error deleting ${searchSlug} doc.`,
    })
  }
}

IGernhardt avatar Mar 24 '25 18:03 IGernhardt

This issue has been automatically locked. Please open a new issue if this issue persists with any additional detail.

github-actions[bot] avatar Apr 26 '25 05:04 github-actions[bot]

🚀 This is included in version v3.36.0

github-actions[bot] avatar Apr 29 '25 16:04 github-actions[bot]