1287529 - Creating a translation when there is a deleted translation returns a 500 ISE

Status: RESOLVED WONTFIX

(developer.mozilla.org Graveyard :: Editing, defect, P3)

Description

[John Whitlock [:jwhitlock]]

What did you do?
================
1. Create and delete a translation of an English page
2. Start a new translation of the English page in the same locale
3. Save the translation

What happened?
==============
A 500 ISE is returned.  The database error is something like:

IntegrityError: (1062, "Duplicate entry '50-it' for key 'wiki_document_parent_id_1890db2968e98804_uniq'")

What should have happened?
==========================
The user should have been prevented from starting the translation, and advised to contact staff to restore or purge the deleted translation.

Is there anything else we should know?
======================================
This also stops translations of subpages of a deleted translation.  For example, if /it/Mozilla/Firefox was deleted, then /it/Mozilla/Firefox/Version_1 will attempt to create it on save and fail with an ISE.

A recent traceback in Sentry:

https://sentry.prod.mozaws.net/operations/mdn-prod/issues/332755/

Comment 1

[John Whitlock [:jwhitlock]]

3-year-old discussion on bug 1034802.

Deleted documents can be viewed in the Django admin:

https://developer.mozilla.org/admin/wiki/document/?deleted__exact=1

I think there was a rough consensus that a deleted document can be purged after a week, which would avoid this issue in many cases.  Purging should also delete or disable DocumentDeletionLog entries, to avoid confusion, but that may be a new bug.

Comment 3

[John Whitlock [:jwhitlock]]

As far as the UI goes, an Internal Server Error is a bad experience.

My preference:

Check for a deleted document at the start of translation, not at save.

If the document was deleted less than 3 days ago, display an error page "There was a recently deleted translation of this page. Please contact a admin for assistance." and either the admin email or a link to Bugzilla.

If the document was deleted more than 3 days ago, purge it and allow the new translation.

Further:

Add a periodic task that auto-purges documents after 7 days, and deletes or disables associated DocumentDeletionLogs.

Comment 4

[John Whitlock [:jwhitlock]]

This is still an issue.

Comment 5

[Peter Bengtsson [:peterbe]]

(In reply to John Whitlock [:jwhitlock] from comment #3)

As far as the UI goes, an Internal Server Error is a bad experience.

My preference:

Check for a deleted document at the start of translation, not at save.

If the document was deleted less than 3 days ago, display an error page
"There was a recently deleted translation of this page. Please contact a
admin for assistance." and either the admin email or a link to Bugzilla.

If the document was deleted more than 3 days ago, purge it and allow the new
translation.

This solution looks sophisticated but also complicated.
If someone attempts to create a new translation and there already was a tombstone there, why confront the user with problems??

Another angle is; if someone tries to create a new translation over an existing tombstone, do we delete that old tombstone or do we resurrect it and make this new translation attempt a new revision?

Further:

Add a periodic task that auto-purges documents after 7 days, and deletes or
disables associated DocumentDeletionLogs.

Filed in General: https://bugzilla.mozilla.org/show_bug.cgi?id=1544837

Comment 6

[Eric Shepherd [:sheppy]]

This solution looks sophisticated but also complicated. If someone attempts to create a new translation and there already was a tombstone there, why confront the user with problems??

Agreed; a solution ought to be found to that.

Another angle is; if someone tries to create a new translation over an existing tombstone, do we delete that old tombstone or do we resurrect it and make this new translation attempt a new revision?

While my first instinct is to say that preserving the history doesn't matter for a deleted page, this opens up the potential for unfortunate and serious mistakes to be made, losing a page and its history by deleting and replacing.

So my thinking is this: can we leave the original in the deleted page pool (pending being purged, if it hasn't been already), and just start from scratch? Might be helpful to add to the comment on the first revision something like "Replaced deleted page <whatever info is useful to find it in the trash>."

Comment 7

[Peter Bengtsson [:peterbe]]

Kadir,
Can you help us decide this one. I'm tempted to go for John's suggestion because it's not horribly hard to do. But I was hoping for something even simpler.

What do you think should happen if someone tries to create a new page when, unbeknownst to them, there was already a tombstone page with the exact same slug + locale?

If a page was deleted, was it because it was filled with spam junk? If so, I don't see any good reason to resurrect the document but with a new revision on top.

If there a risk that if we properly delete the tombstones, when we need to reuse the slug + locale, that we remove valuable documents that were incorrectly deleted by rogue actors?

Who can delete documents? If it's only really trusted people who can do that, perhaps we can assume that it's correctly deleted and should remain deleted. And if that's the case, why bother with a tombstone pattern at all?

Comment 8

[Peter Bengtsson [:peterbe]]

Note-to-self; Deleting a wiki page requires the wiki.delete_document permission. No idea who gets those.

Comment 9

[Kadir Topal [:atopal]]

My take on this: Keeping copies of deleted documents around is not the right way to solve potential data loss issues. We should have a way to recover from catastrophic data loss anyway. Since the plan is to move to Git for documents, it's one more reason to simplify this. Let's make deletions final and hopefully simplify a whole lot of code. I guess we'll need this soon too, since we want to delete a few dozen languages on MDN (see bug 1469079).

Comment 10

[Peter Bengtsson [:peterbe]]

(In reply to Kadir Topal [:atopal] from comment #9)

Let's make deletions final

Wait! What does that mean? Does it mean, if someone has the permission to do so, we literally delete it? Like, DELETE FROM wiki WHERE id = 1234.
I.e. to scrap the tombstone stuff entirely?

Who are these people who delete documents??

Note-to-self; when you delete this is the code that happens:
https://github.com/mozilla/kuma/blob/78216c4a949fa75ff421d3b0a71bd8792c67f24d/kuma/wiki/views/delete.py#L67
It does two things:

1. Creates a DocumentDeletionLog entry in the database
2. Sets deleted=True on the Document and commits the update.

Note-to-self 2:
The delete_document permission does NOT belong to any Group in the database dump I got. And I have, the same list of Groups as in prod. Which means only superusers can delete Wiki documents.

Comment 11

[GitHub Bugzilla PR Linker]

Attachment: Link to GitHub pull-request: https://github.com/mozilla/kuma/pull/5377

Comment 12

[BMO Automation]

MDN Web Docs' bug reporting has now moved to GitHub. From now on, please file content bugs at https://github.com/mdn/sprints/issues/ and platform bugs at https://github.com/mdn/kuma/issues/.