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

Only overwrite Posts that don't have local changes #1064

Merged
merged 5 commits into from
Jan 14, 2019

Conversation

mzorz
Copy link
Contributor

@mzorz mzorz commented Jan 10, 2019

This PR does two things:

  1. when retrieving the Posts list, only dispatch fetch action for posts that don't have local changes.
    2. also when listening to completed FETCH_ACTION, only overwrite Posts that don't have local changes updated as per convo in in Possible content loss when a draft is published from elsewhere WordPress-Android#5984 (comment)

Otherwise, local changes were lost for good.

Thanks @oguzkocer for pointing out where to make the changes!

…hen listening to completed FETCH_ACTION, only overwrite Posts that don't have local changes
Copy link
Contributor

@oguzkocer oguzkocer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good @mzorz! I have left a couple nitpick comments, let me know what you think about them.

P.S: I know I was the one who brought up the changes in handleFetchSinglePostCompleted but I have to say, I am a little bit worried about the potential side effects of it. For one thing, each fetch request will take slightly longer and when a blog post list is visited for the very first time, there will be a lot of these, since we'll fetch each post one by one. Having said that, I can't think of an actual problem with this change, so I am hoping that it's fine :) I just wanted to add my thoughts here in case it proves useful later.

Copy link
Contributor

@oguzkocer oguzkocer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good :shipit:

I'll merge once Travis is done 🤞

@oguzkocer oguzkocer merged commit 8808e6d into develop Jan 14, 2019
@oguzkocer oguzkocer deleted the fix/avoid-update-posts-if-locally-changed-take2 branch January 14, 2019 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants