Page MenuHomePhabricator

Verify editing flow events are firing properly
Open, Needs TriagePublic

Description

Instructions

In the mobile visual editor, perform each of the actions listed in the "Action and Events" section below and verify each "Action" listed in the table fires the correct event (listed in the "Event that should fire" column of the table).

These events should fire in all sessions (not just a sample).

The easiest way to monitor events is by enabling the in-site monitoring UI (see instructions at https://www.mediawiki.org/wiki/Extension:EventLogging/Programming#Monitoring_events).

Actions and Events

ActionEvent that should fireDid event fire? Y/NNotes
tap the “>” in the toolbar to advance to "Save your changes"EditAttemptStep: saveIntentYes
tap “<” in “Save your changes” screenVisualEditorFeatureUse: mwSave.dialog-abortYes, but the event name that appears in the console is dialog-abort not Save.dialog-abort
tap to "Review your changes" in "Save your changes" screenVisualEditorFeatureUse: mwSave.dialog-review/previewyes, but it's dialog-review not Save.dialog-review/preview
tap "Visual" or "Wikitext" toggle in “Review your changes” viewVisualEditorFeatureUse: mwSave.review-switch-source / mwSave.review-switch-visualSame for these ones too. the save part is not there
tap “<” in “Review your changes” screenVisualEditorFeatureUse: mwSave.dialog-reviewNope, the event that does get fired is dialog-approve
tap "This is a minor edit" checkbox in the "Save your changes" screenVisualEditorFeatureUse: mwSave.checkbox-wpMinoreditYes, but it's getting fired twice
tap the "Watch this page" checkbox in the "Save your changes" screenVisualEditorFeatureUse: mwSave.checkbox-wpWatchthisYes, but it's getting fired twice

"Done"

  • We know which of the actions listed in the "Actions and Events" section of this task are firing logging events in the way they should be.

Details

Related Gerrit Patches:

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 12 2019, 10:22 PM
ppelberg updated the task description. (Show Details)Sep 19 2019, 9:21 PM
ppelberg updated the task description. (Show Details)
ppelberg updated the task description. (Show Details)

Updated the task description to include the events we need to test and the instructions for testing them.

@DLynch before handing this over to @Ryasmeen, can you confirm the "Actions and events" section is accurate and exhaustive? I was largely pulling from the table you posted in T229079#5475532 (this was helpful – thank you).

ppelberg updated the task description. (Show Details)Sep 19 2019, 9:26 PM

Updating the "Instructions" portion of the task description to not make mention of particular schemas.

Ryasmeen updated the task description. (Show Details)Sep 24 2019, 10:27 PM
Ryasmeen added a comment.EditedSep 24 2019, 10:30 PM

@ppelberg: I went ahead and gave it a round of testing. I have noted my observations. The only thing is, for all events the word save. is not appended. And the last two events are firing twice.

If these issues are expected, I will mark it as verified :)

It should all be mwSave as the feature and then the dialog-whatever as the action.

ppelberg updated the task description. (Show Details)Oct 1 2019, 5:01 PM
ppelberg updated the task description. (Show Details)Oct 1 2019, 5:04 PM

@ppelberg: I went ahead and gave it a round of testing. I have noted my observations. The only thing is, for all events the word save. is not appended. And the last two events are firing twice.
If these issues are expected, I will mark it as verified :)

Thanks, Rummana. As to whether the observation you made are expected or not, I'm going to ask David to confirm...

@DLynch, I've updated the events in the "Event that should fire" column in the task description to follow the naming pattern you shared in T232790#5521207: for each row in the table in the task description, can you confirm whether what Rummana experienced is expected or not? And if not, what she should expect to see?

ppelberg reassigned this task from ppelberg to DLynch.Oct 1 2019, 5:07 PM
DLynch updated the task description. (Show Details)Oct 3 2019, 5:25 PM
DLynch added a comment.Oct 3 2019, 5:30 PM

Everything Rummana saw looks correct, it's just a confusion about what the event name is. For the VisualEditorFeatureUse events it's two-part, there's a feature and action component, with the name being written here as [feature].[action].

e.g. this console log shows the mwSave.checkbox-wpWatchthis event:

That said, the "Yes, but it's getting fired twice" shouldn't be firing twice. But I can't reproduce that myself, so I might need more information.

DLynch added a comment.Oct 3 2019, 5:38 PM

That said, the "Yes, but it's getting fired twice" shouldn't be firing twice. But I can't reproduce that myself, so I might need more information.

Never mind, worked it out, patch incoming.

Change 540628 had a related patch set uploaded (by DLynch; owner: DLynch):
[mediawiki/extensions/VisualEditor@master] ui.MWSaveDialog: checkbox tracking was getting duplicated on every setup

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

Change 540628 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] ui.MWSaveDialog: checkbox tracking was getting duplicated on every setup

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

Everything Rummana saw looks correct, it's just a confusion about what the event name is. For the VisualEditorFeatureUse events it's two-part, there's a feature and action component, with the name being written here as [feature].[action].
e.g. this console log shows the mwSave.checkbox-wpWatchthis event:

Got it. Thank you for demonstrating this and for updating the task description accordingly, David.

That said, the "Yes, but it's getting fired twice" shouldn't be firing twice. But I can't reproduce that myself, so I might need more information.

Never mind, worked it out, patch incoming.

Excellent.

Next steps...

  • @Ryasmeen, all that's left now is to test the two actions in the table below that you correctly identified as being fired twice in your initial testing:
ActionEvent that should fireDid event fire? Y/NNotes
tap "This is a minor edit" checkbox in the "Save your changes" screenVisualEditorFeatureUse: mwSave.checkbox-wpMinoreditYes, but it's getting fired twice
tap the "Watch this page" checkbox in the "Save your changes" screenVisualEditorFeatureUse: mwSave.checkbox-wpWatchthisYes, but it's getting fired twice
ppelberg reassigned this task from DLynch to Ryasmeen.Oct 3 2019, 10:17 PM
ppelberg moved this task from Stalled/Waiting to QA on the VisualEditor (Current work) board.
ppelberg added a project: Editing QA.

@ppelberg and @DLynch : I am working on the post deployment data checks and had a few queries.

  1. Can you please confirm if this scenario is working as expected ?
ActionEvent that should fireDid event fire? Y/N
tap “<” in “Review your changes” screenVisualEditorFeatureUse: mwSave.dialog-reviewNope, the event that does get fired is dialog-approve
  1. Can you please also clarify what do we expect to get here ?
ActionEvent that should fireDid event fire? Y/N
tap to "Review your changes" in "Save your changes" screenVisualEditorFeatureUse: mwSave.dialog-review/previewyes, but it's dialog-review not Save.dialog-review/preview

When does the dialog-review vs dialog-preview get fired ?

Everything Rummana saw looks correct, it's just a confusion about what the event name is. For the VisualEditorFeatureUse events it's two-part, there's a feature and action component, with the name being written here as [feature].[action].
e.g. this console log shows the mwSave.checkbox-wpWatchthis event:

Got it. Thank you for demonstrating this and for updating the task description accordingly, David.

That said, the "Yes, but it's getting fired twice" shouldn't be firing twice. But I can't reproduce that myself, so I might need more information.

Never mind, worked it out, patch incoming.

Excellent.

Next steps...

  • @Ryasmeen, all that's left now is to test the two actions in the table below that you correctly identified as being fired twice in your initial testing:
ActionEvent that should fireDid event fire? Y/NNotes
tap "This is a minor edit" checkbox in the "Save your changes" screenVisualEditorFeatureUse: mwSave.checkbox-wpMinoreditYes, but it's getting fired twice
tap the "Watch this page" checkbox in the "Save your changes" screenVisualEditorFeatureUse: mwSave.checkbox-wpWatchthisYes, but it's getting fired twice

@ppelberg: Checked just now, both the events are now firing once as expected :)