Skip to content
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

Clean up comment HasReplies cache on child comment deletion #1507

Merged
merged 2 commits into from
Oct 3, 2022

Conversation

paskal
Copy link
Sponsor Collaborator

@paskal paskal commented Oct 2, 2022

Previously, the cache kept the entry and deletion of the parent comment after child deletion was not possible for the rest of cache life (5m) duration. Now it's possible to delete a parent comment after the deletion of the child comment by a user or admin.

Resolves #1481

@paskal paskal added this to the v1.10.2 milestone Oct 2, 2022
@paskal paskal requested a review from umputun as a code owner October 2, 2022 23:45
Copy link
Owner

@umputun umputun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looking good. the only thing i would suggest is to add tests on the service level, not just functional test on the rest level, as you did. Those test should be relatively easy to do as you have access to the cache and can check if comments added/removed. Alternatively, you can define a local interface for repliesCache.LoadincCache, mock it and check the interaction with the mock.

@paskal paskal force-pushed the paskal/renew_cache_on_delete branch from 9a324e8 to f4f8e39 Compare October 3, 2022 07:48
Previously, the cache kept the entry and deletion of the parent comment
after child deletion was not possible for the rest
of cache life (5m) duration. Now it's possible to delete
a parent comment after the deletion of the child comment
by a user or admin.

Resolves #1481
@paskal paskal force-pushed the paskal/renew_cache_on_delete branch from f4f8e39 to ed93ca6 Compare October 3, 2022 08:23
@umputun umputun merged commit dd1ba9b into master Oct 3, 2022
@umputun umputun deleted the paskal/renew_cache_on_delete branch October 3, 2022 08:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot delete/edit comment with replies even if they're removed
2 participants