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

Automatically upload local drafts and locally changed posts and pages #10177

Closed
wants to merge 4 commits into from

Conversation

shiki
Copy link
Member

@shiki shiki commented Jul 8, 2019

Closes #10174.

Summary

When the LocalDraftUploadStarter was created in #9774, it was only uploading local draft posts, and later on, pages. The reasons for this were:

  • I wanted to test out the implementation first and see how it works.
  • I wasn't sure whether we should support this for other statuses. There were still some discussions going on.

Today, it looks like we can now safely do this. This change makes it so that all local drafts and all posts and pages that are locally changed will be automatically uploaded. Based on some testing, this is what iOS does today.

Testing

  1. Go offline.
  2. Create or edit posts and pages to end up with different statuses:
    • Local draft
    • Existing draft with changes
    • Existing uploaded post/page with changes
  3. Go online.

Confirm that all are automatically uploaded.

Reviewing

Only 1 reviewer is needed but anyone can review.

Release Notes

  • If there are user-facing changes, I have added an item to RELEASE-NOTES.txt.

Other Tasks

shiki added 3 commits July 8, 2019 09:54
This is for clarity when UploadStarter will auto-upload all pending post types, not just local drafts.
This was because of the recent changes in UploadStarter.
@shiki shiki requested review from maxme and malinajirka July 8, 2019 20:22
@shiki shiki marked this pull request as ready for review July 8, 2019 20:22
@malinajirka
Copy link
Contributor

malinajirka commented Jul 9, 2019

Thanks @shiki for the changes!

The code seems to work as intended. However, I'm a bit confused whether this is the behavior we wanted. I thought we'd publish changes only after user's explicit confirmation, but the app publishes all the changes, right?

The behavior I expected is following

  1. Local drafts and locally changed drafts -> upload automatically
  2. Published post with local changes - user did not click on "publish/update" -> automatically auto save
  3. Published post with local changes - user clicked on "publish/update" -> upload automatically
  4. I'm not sure what was our final decision for scheduled posts

Another thing I'm wondering about is if we need to somehow make sure the post we are about to upload isn't being edited at the moment. I'm not sure about this, I just wanted to bring it to the table so we can think about it.

Let me know your thoughts ;)

@malinajirka
Copy link
Contributor

Btw I believe this task shouldn't be merged without #10207. We might want to merge it into a working branch. Wdyt?

@shiki
Copy link
Member Author

shiki commented Jul 11, 2019

@malinajirka That sounds right. I was planning to split this into multiple PRs so we're more careful of what should get automatically uploaded. I haven't thought more about it yet. I'll let you know.

@malinajirka
Copy link
Contributor

I'm not sure where to note this so we don't forget about it. I'll just post it here.You might be planning or have already fixed this, but just to be sure.

I believe we'll need to modify this statement when we start publishing posts automatically so we bump the tracks when a post isn't uploaded because the device is offline and is uploaded automatically later.

@shiki shiki removed this from the 12.9 milestone Jul 12, 2019
@shiki
Copy link
Member Author

shiki commented Jul 12, 2019

Will just close this for now as this will most probably be split into multiple PRs. There will be upcoming discussions for this too.

@shiki shiki closed this Jul 12, 2019
Offline Support: Posting [Android] automation moved this from In Review to Done (PRs) Jul 12, 2019
@peril-wordpress-mobile
Copy link

Warnings
⚠️ PR is not assigned to a milestone.

Generated by 🚫 dangerJS

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Automatically upload posts with local changes
2 participants