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

Deleted posts still appear in the reading list #1641

Open
rhymes opened this issue Jan 24, 2019 · 12 comments

Comments

Projects
None yet
7 participants
@rhymes
Copy link
Collaborator

commented Jan 24, 2019

Describe the bug

A deleted post appears in the reading list and I can't remove it manually.

To Reproduce
Steps to reproduce the behavior:

A DEV user creates a post, someone else adds it to their reading list. The post's author deletes the post. The owner of the reading list still sees the post listed.

Expected behavior

A deleted post should be removed from the reading list.

Desktop (please complete the following information):

  • OS: macOS
  • Browser: Firefox
  • Version: 65

Additional context

It might also be related to #1061

@triage-new-issues triage-new-issues bot added the triage label Jan 24, 2019

@aspittel aspittel added bug and removed triage labels Jan 24, 2019

@aspittel

This comment has been minimized.

Copy link
Collaborator

commented Jan 24, 2019

Thanks for catching this @rhymes!

@benhalpern

This comment has been minimized.

Copy link
Collaborator

commented Jan 24, 2019

There must be a logical issue with our removal from Algolia upon deletion. I have removed this article in question and will look to hunt down the bug.

We'll also do an index sweep asap to remove any other posts that might still exist after being deleted.

@benhalpern

This comment has been minimized.

Copy link
Collaborator

commented Jan 24, 2019

This is the general problem area if it is indeed a logical issue:

Other possible candidate: We may have had some memory bloat issues with the environment our async jobs were running in, causing occasional failures. It's one possible issue.

PS @rhymes I removed the link/screenshot from your issue just to help that article get forgotten as the author would have intended via deletion, now that we are aware of the issue.

@rhymes

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 24, 2019

Thanks for editing my issue @benhalpern and for removing it from my reading list

My first thought was that maybe the order of the operations gets mixed up in some corner cases.

I'm not sure I understand the logic intrigger_delayed_index because it can both index or remove from an index, like a toggle. I noticed also that there is a before_destroy action that outright removes the object.

Is there a case when the before destroy removes the article from the index, then trigger_delayed_index runs and puts the object back in the index before the object is "phisically" removed from the DB? 🧐

@lightalloy

This comment has been minimized.

Copy link
Collaborator

commented Feb 5, 2019

It seems like trigger_delayed_index is not useful in case of destroying. trigger_delayed_index runs after_destroy, so this check record.&persisted? will always be false and record.delay.remove_from_index! won't be executed. An article is actually removed from the index in before_destroy_actions (remove_algolia_index).
So I would specify an option auto_remove: false on algoliasearch and remove part of the trigger_delayed_index method to remove the confusion.

@lightalloy

This comment has been minimized.

Copy link
Collaborator

commented Feb 15, 2019

I've found a couple of problems related to this issue:

  • The cache key only depends on the user, so when an article from the reading list is destroyed, the cached_ids_of_articles won't be updated.
  • Same when an article goes unpublished
  • The reading list doesn't check if articles are published

Ideally, these issues shouldn't affect the list of the articles, cause they should be removed from the Algolia index when they get unpublished or destroyed.
But still it would be more correct to update cache and filter by published in my opinion, moreover, in some cases, reindexing is done asynchronously and there could possibly be issues like memory bloat like Ben said.

@benhalpern Do you think these issues worth fixing?

@rhymes

This comment has been minimized.

Copy link
Collaborator Author

commented May 6, 2019

After Ben's amazing changes to the reading list in #2695 - now I can see the three deleted posts that have been in my "reading list" until now:

Screenshot_2019-05-06 The DEV Community(1)

Screenshot_2019-05-06 The DEV Community(2)

I can't remove them because they are deleted from the website, but they are now appearing where until now they had disappeared (I've removed the titles and authors)

@benhalpern benhalpern self-assigned this May 7, 2019

@benhalpern

This comment has been minimized.

Copy link
Collaborator

commented May 7, 2019

Ah! I didn't realize this would be the case with the new approach, but this is much easier to debug with the new way we're doing this. The old way we did this was really hard to track down, but I'll get this one worked out once and for all soon.

@jessleenyc

This comment has been minimized.

Copy link
Collaborator

commented May 22, 2019

I believe @benhalpern knows what's going on but we're waiting to fix because it'll require running a long script.

@benhalpern benhalpern closed this May 31, 2019

@rhymes

This comment has been minimized.

Copy link
Collaborator Author

commented Jun 6, 2019

@benhalpern they are still there :D

@maestromac maestromac reopened this Jun 6, 2019

@benhalpern

This comment has been minimized.

Copy link
Collaborator

commented Jun 6, 2019

@rhymes I'll DM you and try to get to the bottom of this.

@sandordargo

This comment has been minimized.

Copy link

commented Jun 21, 2019

I have the same issue. I cannot remove this from my reading list:
https://dev.to/theobendixson/how-to-safeguard-your-productive-time-by-batching-bullshit-2p27

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.