Page MenuHomePhabricator

CX2: Prevent autosaving to mark the translation as “in-progress” after publishing if there are no further changes
Closed, ResolvedPublic

Description

After an article is published, it is expected to appear listed in the "published" list in the dashboard. Currently, after publishing it, the auto-save process can mark it as "in-progress" even if no changes have been made by the user, which is not expected.

There are two cases that can result in this problem:

  1. Translator was editing some sections. Autosave was happening in its triggers (timers, debounced events), Translator publish it. While the publish was happening, the save was also going on in parallel (either started before or along with publish OR from a debounced event handler - implies non-empty save queue).
  2. A published translation is restored(opened in the editor). No changes were done by translator. Some save happened, putting the translation into draft state.

For the case #1, the tool can wait for empty save queue before publish request is sent to backend. This can be done by checking for save queue, process it, wait for response, and do publish call. During this time Publish button shows 'Publishing', considering the previous clean-up as part of the publishing process and to abstracts the internal steps from user.

For the case #2, we should make sure no save happens in a restored translation without a real user edit. Autosaving should not be triggered until the user makes further changes to the content.

Details

Related Gerrit Patches:
mediawiki/extensions/ContentTranslation : masterWait for autosave if needed while trying to publish

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 25 2018, 5:19 PM
Pginer-WMF triaged this task as Medium priority.Jul 25 2018, 5:20 PM

Change 461481 had a related patch set uploaded (by Petar.petkovic; owner: Petar.petkovic):
[mediawiki/extensions/ContentTranslation@master] Prevent autosaving while publishing the page

https://gerrit.wikimedia.org/r/461481

santhosh added a subscriber: santhosh.EditedSep 25 2018, 10:39 AM

The current patch does not address the issue of waiting for save to finish. It disable autosave. That is what the task description also says.

Autosaving should not be triggered until the user makes further changes to the content after publishing.

From the commit message:

Since we cannot stop scheduled autosave, new control variable is introduced, to allow disabling the autosaving process. Currently, only use case is to disable autosaving when attempting to publish, which requires (re)enabling the autosave in case of failure, canceling or too much MT. We need not stop, we need to wait.

We need not stop, we need to wait. When publish button is pressed, save all the items in queue. Make sure to introduce the save request promise a class level property so that publish handler can wait for that. In the success handler of the save request, proceed publishing.

I can also think of another solution: Disable the publish button while saving is going on(when we have a non-empty save queue). In CX1 we had several complaints about edit not saved. So in CX2, we should never disable saving, I guess it would be a wrong pattern. Let the save happen as and when required. It is important to avoid losing data. Let the publish happen when everything saved.

Saving is an important process, and we want it to be reliable. That is, even if something unexpected happens (errors, warnings, etc.) we should avoid data from the user to be lost.

The current ticket is intended to avoid the case where a published translation is unnecessarily marked as in-progress again without the user making any change. Avoiding this will prevent user confusion, but we need to make sure that we support it without making the overall saving process more fragile, excessively complex or prone to fail.

The ticket describing "Autosaving should not be triggered until the user makes further changes to the content after publishing." was just a suggested solution, but others could be also valid. For example, it would be ok to let the autosave happen, identify that no changes were made and keep the translation in the "published" state.

There are two scenario where a published translation can go to draft state unintentionally:

  1. Translator was editing some sections. Autosave was happening in its triggers(timers, debounced events), Translator publish it. While the publish was happening, the save was also going on in parallel(either started before or along with publish OR from a debounced event handler - implies non-empty save queue).
  2. A published translation is restored(opened in the editor). No changes were done by translator. Some save happened, putting the translation into draft state.

For the case #1, I was proposing to wait for empty save queue before publish request is sent to backend. This can be done by checking for save queue, process it, wait for response, and do publish call. During this time Publish button shows 'Publishing'. I was also proposing alternative of disabling publish when there is non-empty save queue. The first solution abstracts the process from user.

For the case #2, we should make sure no save happens in a restored translation without a real user edit.

There are two scenario where a published translation can go to draft state unintentionally:

Thanks for the clarification. I had only scenario 2 in mind, but you are right, it makes sense to support both. I'll update the description with more details about the proposed solutions.

For the case #1, I was proposing to wait for empty save queue before publish request is sent to backend. This can be done by checking for save queue, process it, wait for response, and do publish call. During this time Publish button shows 'Publishing'. I was also proposing alternative of disabling publish when there is non-empty save queue. The first solution abstracts the process from user.

The first solution makes perfect sense to me. As part of the publishing process it makes sense to complete pending work (including checking for warnings, for example).

For the case #2, we should make sure no save happens in a restored translation without a real user edit.

Makes sense.

Pginer-WMF updated the task description. (Show Details)Sep 25 2018, 11:42 AM
  1. A published translation is restored (opened in the editor). No changes were done by translator. Some save happened, putting the translation into draft state.

This was a bug before, which was fixed for T193161. However, due to a regression, when draft is loaded, save can happen without user edit.

Since this patch, when block or inline transclusion node is rendered, update event is emitted. That gets treated as section change, which triggers saving. In effect, this means if saved draft has any transclusion nodes, the saving will be triggered even without a change.

I think we will need a new event there to indicate alignement and not a section change for save.

Change 461481 merged by Nikerabbit:
[mediawiki/extensions/ContentTranslation@master] Wait for autosave if needed while trying to publish

https://gerrit.wikimedia.org/r/461481

Etonkovidova closed this task as Resolved.Oct 22 2018, 4:19 PM
Etonkovidova added a subscriber: Etonkovidova.

Checked in cx2-testing.

For the case (1) - the fix seems to be in place. However, I haven't tried to test it on very large translations.
For the case (2) -without actual edit in loaded (published) translations, autosave is not triggered.

Case (2) is still a problem. Autosave can be triggered without actual user edit when draft contains some block or inline transclusion nodes.

That should be resolved as part of a new ticket - T207699.