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

Code Review: Review editor and publishing logic realted to local-only posts. #3978

Closed
aerych opened this issue Jul 6, 2015 · 5 comments
Closed

Comments

@aerych
Copy link
Member

aerych commented Jul 6, 2015

All local-only posts have a postID of -1. This is potentially a problem anywhere reference a post by postID since for local-only post it is not unique.

Theory: Some of the bug reports regarding missing or vanishing drafts are related to non-unique -1 postID.

Scrub the publish logic for problems and investigate other options for local-only postIDs.

cc @bummytime

@diegoreymendez
Copy link
Contributor

@aerych - Do you happen to have any of those reports handy? Or a link?

@aerych
Copy link
Member Author

aerych commented Mar 17, 2016

@diegoreymendez I was probably referring to the occasional reports in the iOS forums about lost drafts/edits. I don't recall a specific thread tho.

@diegoreymendez
Copy link
Contributor

No worries just wondering. I'll investigate this after we merge #4952.

@mchowning
Copy link
Contributor

Seems this is likely still an issue we should keep open since we still have a default postId of -1.

@kean
Copy link
Contributor

kean commented May 1, 2024

The default value being -1 is not problem in itself.

The entire flow has been re-design in the scope of pcdRpT-6vS-p2. For example, these problematic queries are gone:

let query =
            // Existing draft/pending posts
            "(statusAfterSync = status AND status IN (%@))"
            // Existing draft/pending posts transitioned to another status but not uploaded yet
            + " OR (statusAfterSync != status AND statusAfterSync IN (%@))"
            // Posts existing only on the device with statuses defined in `statusesForLocalDrafts`.
            + " OR (postID = %i AND status IN (%@))"
            // Include other existing draft/pending posts with `nil` `statusAfterSync`. This is
            // unlikely but this ensures that those posts will show up somewhere.
            + " OR (postID > %i AND statusAfterSync = nil AND status IN (%@))"
``

It's now just: 

"status == draft"


Other areas have also been updated and are fully covered by integration tests. I think it's fair to close it.

@kean kean closed this as completed May 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants