Skip to content
This repository was archived by the owner on Feb 4, 2025. It is now read-only.

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 subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants