-
Notifications
You must be signed in to change notification settings - Fork 61
fix: resolve issue when updating or adding content type #998
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
base: main
Are you sure you want to change the base?
fix: resolve issue when updating or adding content type #998
Conversation
Hello @ratorik, thanks so much for your contribution. 🙌 Would it be possible to add a test for this addition, so we can avoid any regression in the future? |
Hey @Strift, Thanks for your feedback! I'll look into adding a test for this to help prevent regressions. That said, as mentioned in the ticket by @daandegooijer, the best long-term solution might be to revamp the lifecycles and align with the Strapi documentation. Since this was just a quick fix, maybe it makes sense to address the broader issue instead? Let me know what you think! |
Hi @ratorik, I think it's better to submit this small fix first, and then work on the broader fix in a second step. Refactoring a bigger part of the codebase will make the review process more heavy. As a result, changes would get deployed more slowly. So I'd recommend adding the tests for this PR and then working on the revamp. If you have the bandwidth to work on it, a broader fix would be welcome. Just consider that it could take me and other contributors a longer time to review it. |
Hey, I saw this is WIP but just thought I would send a reminder to translate comments in English before submitting for review again 🙏 |
…abled fix: resolve Meilisearch issue with draftAndPublish enabled When updating or adding an item with draftAndPublish enabled, the item is removed from Meilisearch, and the following error occurs: error: Meilisearch could not add entry with id: 9: Transaction query already complete This issue is described in meilisearch#997. The item should update correctly without errors when draftAndPublish is enabled.
…terUpdate, afterUpdateMany, afterDelete and afterDeleteMany
f4f692b
to
336d24e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes an issue where items with draftAndPublish enabled were removed from Meilisearch due to transaction errors. It simplifies lifecycle methods by using the provided result directly rather than fetching the complete entry, and updates tests and mocks accordingly.
- Removed the redundant call to fetch a complete entry in both afterCreate and afterUpdate lifecycle methods.
- Updated tests in lifecycle.test.js to verify behavior with the new result-based approach.
- Augmented mocks in strapi.js to include Meilisearch operations.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
File | Description |
---|---|
server/src/services/lifecycle/lifecycle.js | Refactored lifecycle handlers to use the provided result directly for Meilisearch operations. |
server/src/tests/lifecycle.test.js | Added/modified tests to cover all lifecycle events using the updated implementation. |
server/src/mocks/strapi.js | Updated mocks to support the new Meilisearch service methods. |
Hi, do we have a release date for this merge ? @Strift |
fix: resolve Meilisearch issue with draftAndPublish enabled
When updating or adding an item with draftAndPublish enabled, the item is removed from Meilisearch, and the following error occurs:
error: Meilisearch could not add entry with id: 9: Transaction query already complete
This issue is described in #997. The item should update correctly without errors when draftAndPublish is enabled.
Pull Request
Related issue
Fixes #997
What does this PR do?