Wordpress-ios: Analytics event `editor_draft_saved` fires as `editor_post_published`

[chipsnyder]

Expected behavior

After creating a new page or post and backing out via the X and save as draft flow, the editor should fire the analytics event editor_draft_saved

Actual behavior

Editor fires the analytics event editor_post_published

Steps to reproduce the behavior

1. Create a new Page or Post
2. Make a few edits Set title and content
3. Select "X" in the top left-hand corner
   
   
   
   • Expect: To see a dialog with "You have unsaved changes." and "Save Draft"
   
   
4. Select Save

Expected The editor should fire the analytics event editor_draft_saved
Actual The editor fires the analytics event editor_post_published

Tested on iPhone 11, iOS 13.3, WPiOS 14.5

Additional info

This was discovered as part of https://github.com/wordpress-mobile/WordPress-iOS/issues/13527. We might want to combine this work with another ticket to adjust the way we fire the draft quicksave as well https://github.com/wordpress-mobile/WordPress-iOS/issues/13527#issuecomment-603743549

[elibud]

@hypest @pinarol can you please have someone on your team to look into this as it may be affecting your analysis?

[guarani]

Thanks for reporting, @chipsnyder! I can reproduce it consistently.

The method that performs the save in this scenario is publishPost(action:dismissWhenDone:analyticsStat:) and is called here:
https://github.com/wordpress-mobile/WordPress-iOS/blob/d798535dc663d24d3231caef236791f322c0c82e/WordPress/Classes/ViewRelated/Post/PostEditor%2BPublish.swift#L309

Notice that action is passed in explicitly _and_ is also a property of postEditorStateContext – this seems like two sources of truth for the action the user is performing, which is odd.

Going back to this scenario, I see that action == .saveAsDraft while self.postEditorStateContext.action == .publish. I'd say the former is correct but the latter is wrong – both should be .saveAsDraft. The reason it's publish in this scenario seems to be because publish is the default and is never updated. See:
https://github.com/wordpress-mobile/WordPress-iOS/blob/e3cd86b3cf984ff576bbef61a17df38fa7f06b20/WordPress/Classes/ViewRelated/Post/PostEditorState.swift#L152

One solution is to directly update self.postEditorStateContext.action to saveAsDraft when the "Save Draft" button is pressed. I then noticed that the mapping from action to analytic event is not correct for what we're doing here (quick save is the save you can perform without leaving the editor), so I'm probably looking for .save instead:
https://github.com/wordpress-mobile/WordPress-iOS/blob/e3cd86b3cf984ff576bbef61a17df38fa7f06b20/WordPress/Classes/ViewRelated/Post/PostEditorState.swift#L123-L128

This feels a bit hacky so I'm looking for any comments you might have. I'll add a draft PR for now, and keep looking into this (maybe the Android implementation will have clues).

[chipsnyder]

(maybe the Android implementation will have clues).

IIRC Android was a bit different for this flow as they don't have the same options that iOS does when you back out so I don't believe they handle the logic here.

I took a look at your PR and I believe that's similar to what another call is doing around that same area. I remember trying that briefly but it affected another call from my test cases in https://github.com/wordpress-mobile/WordPress-iOS/issues/13527. I'll take some time this week and see if I'm remembering correctly and add the notes here.

[guarani]

I took a look at your PR and I believe that's similar to what another call is doing around that same area. I remember trying that briefly but it affected another call from my test cases in #13527. I'll take some time this week and see if I'm remembering correctly and add the notes here.

Ok, I'll take a look at your issue and see if I can find any problems too.

[guarani]

Hey @chipsnyder—I ran through the test cases you provided here https://github.com/wordpress-mobile/WordPress-iOS/pull/13708 and I didn't notice any issues.

I remember trying that briefly but it affected another call from my test cases in #13527.

Do you recall any other details about this? In any case, I've marked the PR as ready for review and added you as a reviewer 🙇 .

Test case results

Scenarios that fire editor_post_created:

• [x] Create using FAB button
• [x] Create via the Page List
• [x] Create via the Post List
• [x] Create via Adding Media
• [x] Create via the App Icon
• [x] Create via Universal Links

Scenarios that fire editor_post_published

• [x] Create a Page or a Post

Scenarios that fire editor_post_updated (the event is actually editor_post_update):

• [x] Update via Update Button
• [x] Update via Backing out

Scenarios that fire editor_post_scheduled:

• [x] Create a Page or a Post

Scenarios that fire editor_quick_draft_saved:

• [x] Create a Page or a Post

[chipsnyder]

Thanks for testing @guarani I didn't get a chance yet to get back to testing these flows but I'll take a look here in the next day or so.

[chipsnyder]

Hey @guarani, I did find that case I was thinking about and noted it here: https://github.com/wordpress-mobile/WordPress-iOS/pull/14274#issuecomment-645665694