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

Delete all content throws error when subscriber is associated to a post #7875

Closed
kirrg001 opened this issue Jan 23, 2017 · 7 comments · Fixed by #7962 or #8282
Closed

Delete all content throws error when subscriber is associated to a post #7875

kirrg001 opened this issue Jan 23, 2017 · 7 comments · Fixed by #7962 or #8282
Assignees
Labels
bug [triage] something behaving unexpectedly help wanted [triage] Ideal issues for contributors to help with server / core Issues relating to the server or core of Ghost

Comments

@kirrg001
Copy link
Contributor

kirrg001 commented Jan 23, 2017

When trying to delete all content, we are getting a SQL error. Seems that deleting the content does't work if there is a subscriber associated with the post because of a constraint.

delete from `posts` where `id` = 294 - ER_ROW_IS_REFERENCED_: 
Cannot delete or update a parent row: a foreign key constraint fails 
(`blog-495478`.`subscribers`, CONSTRAINT `subscribers_post_id_foreign` FOREIGN KEY 
(`post_id`) REFERENCES `posts` (`id`))

Important: This bug does not exist using sqlite.

Steps to Reproduce

  1. Subscribe to a post
  2. Go to Labs
  3. Press Delete All Content button

Technical details:

  • Ghost Version: 0.11.4
  • Node Version: 4.4.7
  • Database: MySQL
@kirrg001 kirrg001 added help wanted [triage] Ideal issues for contributors to help with LTS server / core Issues relating to the server or core of Ghost bug [triage] something behaving unexpectedly labels Jan 23, 2017
@vivekannan
Copy link
Contributor

https://github.com/TryGhost/Ghost/blob/master/core/server/api/db.js#L94

Replace this,

var collections = [
            models.Post.findAll(queryOpts),
            models.Tag.findAll(queryOpts)
];

with this,

var collections = [
            models.Subscriber.findAll(queryOpts),
            models.Post.findAll(queryOpts),
            models.Tag.findAll(queryOpts)
];

@kirrg001
Copy link
Contributor Author

@vivekannan Thanks 👍

We would be happy about a PR, which 1. reproduces the error in a test and add's your fix on top to proof that it fixes this bug.

@vivekannan
Copy link
Contributor

OK. I am struggling to get the db integration test to create subscribers associated with a post so that the it fails during the deleteAllContent test. Also, how are posts properly deleted even with tags associated with them? Shouldn't it raise the same constraint error when deleting posts with subscribers?

@kirrg001
Copy link
Contributor Author

@vivekannan Did you run your test with MySQL?

NODE_ENV=testing-mysql TRAVIS=true

@vivekannan
Copy link
Contributor

Yes. I did run the test with NODE_ENV set to testing-mysql. However, the test passes as posts with subscribers are not created while testing the deleteAllContents API. So, I have to make changes to the test so that it creates dummy posts with subscribers.

https://github.com/TryGhost/Ghost/blob/master/core/test/integration/api/api_db_spec.js

The testUtils.setup doesn't create posts with subscribers.

@kirrg001 kirrg001 self-assigned this Feb 8, 2017
kirrg001 added a commit to kirrg001/Ghost that referenced this issue Feb 8, 2017
closes TryGhost#7875

- we need to delete the subscribers before deleting the posts
kirrg001 added a commit to kirrg001/Ghost that referenced this issue Feb 8, 2017
closes TryGhost#7875

- we need to delete the subscribers before deleting the posts
kevinansfield pushed a commit that referenced this issue Feb 8, 2017
closes #7875
- we need to delete the subscribers before deleting the posts
kevinansfield pushed a commit that referenced this issue Feb 8, 2017
closes #7875
- we need to delete the subscribers before deleting the posts
@ErisDS
Copy link
Member

ErisDS commented Mar 3, 2017

We have this same issue for trying to delete a single post when it has a subscriber tied to it, but not when trying to delete a subscriber (I'm sure we fixed that one, but can't find the issue).

I have confirmed this on Ghost(Pro) blog today:

Create post, subscribe from post:

  • try to delete post -> ER_ROW_IS_REFERENCED_
  • try to delete subscriber -> works
  • now delete post -> works

Also, the fix implemented here for deleteAllContent is, IMO, the wrong fix.

Delete all Content is meant to delete content = posts & tags. Deleting subscribers is unexpected behaviour IMO.

In the alpha, we have removed the constraint that is causing these issues. So this issue is LTS only.

@ErisDS ErisDS reopened this Mar 3, 2017
@kirrg001
Copy link
Contributor Author

kirrg001 commented Mar 3, 2017

Delete all Content is meant to delete content = posts & tags. Deleting subscribers is unexpected behaviour IMO.

I see 👍

Have scheduled it for the next LTS.

kirrg001 added a commit to kirrg001/Ghost that referenced this issue Mar 24, 2017
refs TryGhost#7875

- do not delete subscribers if deleting all the content
- ensure dropping all the content and deleteting a single post will delete the reference in the subscriber if exist
ErisDS pushed a commit that referenced this issue Apr 4, 2017
refs #7875

- do not delete subscribers if deleting all the content
- ensure dropping all the content and deleteting a single post will delete the reference in the subscriber if exist
kirrg001 added a commit to kirrg001/Ghost that referenced this issue Apr 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug [triage] something behaving unexpectedly help wanted [triage] Ideal issues for contributors to help with server / core Issues relating to the server or core of Ghost
Projects
None yet
3 participants