-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Comments
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
|
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 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: |
Hey @akhrarovsaid I was using db-postgres in my project. Everything was directly from the website template I get from searchPlugin({
collections: ['posts'],
}), The search result record still does not delete for me with this change. To get this result:
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. |
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 The last time @r1tsuu You may find this interesting. |
looks like the same issue I reported earlier this year which may have been closed by mistake #5709 |
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) |
🚀 This is included in version v3.9.0 |
Any progress on this? |
…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)
Would like to confirm that this is still an issue, I'm having to manually delete search indices at the moment. |
The issue here is the 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 collectionsLet's say you've enabled // 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 Let me know if that works for you. |
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.`,
})
}
} |
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
This issue has been automatically locked. |
🚀 This is included in version v3.36.0 |
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
create-payload-app
. Use website template and postgres database.Which area(s) are affected? (Select all that apply)
plugin: search
Environment Info
The text was updated successfully, but these errors were encountered: