Skip to content

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

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

Closed
TimLanzi opened this issue Nov 22, 2024 · 13 comments · Fixed by #12148
Closed

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

TimLanzi opened this issue Nov 22, 2024 · 13 comments · Fixed by #12148
Assignees
Labels
plugin: search @payloadcms/plugin-search prioritized

Comments

@TimLanzi
Copy link

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 TimLanzi added status: needs-triage Possible bug which hasn't been reproduced yet validate-reproduction Auto-added tag on create to tell bot to check recreation URL, removed after check. labels Nov 22, 2024
@github-actions github-actions bot removed the validate-reproduction Auto-added tag on create to tell bot to check recreation URL, removed after check. label Nov 22, 2024
Copy link
Contributor

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

@jacobsfletch jacobsfletch added the plugin: search @payloadcms/plugin-search label Nov 22, 2024
@akhrarovsaid
Copy link
Contributor

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

@TimLanzi
Copy link
Author

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.

@akhrarovsaid
Copy link
Contributor

akhrarovsaid commented Nov 22, 2024

@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.

@wkentdag
Copy link

wkentdag commented Dec 5, 2024

looks like the same issue I reported earlier this year which may have been closed by mistake #5709

@wkentdag
Copy link

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)

@r1tsuu r1tsuu reopened this Dec 17, 2024
@github-actions github-actions bot added the status: needs-triage Possible bug which hasn't been reproduced yet label Dec 17, 2024
Copy link
Contributor

🚀 This is included in version v3.9.0

@KoenRijpstra
Copy link

Any progress on this?

kendelljoseph pushed a commit that referenced this issue Feb 21, 2025
…ponent (#9932)

<!--

Thank you for the PR! Please go through the checklist below and make
sure you've completed all the steps.

Please review the
[CONTRIBUTING.md](https://github.com/payloadcms/payload/blob/main/CONTRIBUTING.md)
document in this repository if you haven't already.

The following items will ensure that your PR is handled as smoothly as
possible:

- PR Title must follow conventional commits format. For example, `feat:
my new feature`, `fix(plugin-seo): my fix`.
- Minimal description explained as if explained to someone not
immediately familiar with the code.
- Provide before/after screenshots or code diffs if applicable.
- Link any related issues/discussions from GitHub or Discord.
- Add review comments if necessary to explain to the reviewer the logic
behind a change

### What?

### Why?

### How?

Fixes #

-->
### What?
This PR fixes a runtime error encountered when navigating into a search
doc that had its' related collection doc deleted, but it itself remained
(if for example `deleteFromSearch` deletion failed for some reason).

### Why?
To prevent runtime errors for end-users using `plugin-search`.

### How?
By returning earlier if the field value is undefined or missing required
values in `LinkToDoc`.

Fixes #9443 (partially, see also: #9623)
@IGernhardt
Copy link

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

@akhrarovsaid
Copy link
Contributor

akhrarovsaid commented Mar 24, 2025

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.

@IGernhardt
Copy link

IGernhardt commented Mar 24, 2025

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.`,
    })
  }
}

DanRibbens added a commit that referenced this issue Apr 18, 2025
The plugin-search collection uses an `afterDelete` hook to remove search
records from the database. Since a deleted document in postgres causes
cascade updates for the foreign key, the query for the document by
relationship was not returning the record to be deleted.

The solution was to change the delete hook to `beforeDelete` for the
search enabled collections. This way we purge records before the main
document so the search document query can find and delete the record as
expected.

An alternative solution in #9623 would remove the `req` so the delete
query could still find the document, however, this just works outside of
transactions which isn't desirable.

fixes #9443
Copy link
Contributor

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

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 26, 2025
Copy link
Contributor

🚀 This is included in version v3.36.0

@github-actions github-actions bot unlocked this conversation Apr 29, 2025
@github-actions github-actions bot locked and limited conversation to collaborators Apr 29, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.