Skip to content

Offline Support : Post List - Fix Upload Status Cache issue - #11472

Merged
malinajirka merged 5 commits into
feature/master-pages-offline-supportfrom
issue-11471/post_list_upload_status_tracker
Mar 19, 2020
Merged

Offline Support : Post List - Fix Upload Status Cache issue#11472
malinajirka merged 5 commits into
feature/master-pages-offline-supportfrom
issue-11471/post_list_upload_status_tracker

Conversation

@jd-alexander

Copy link
Copy Markdown
Contributor

Fixes #11471

Findings

A new instance of PostModelUploadStatusTracker was being created in multiple components which resulted in out of sync caches.

Solution

Similarly to #11315, A single instance of PostModelUploadStatusTracker is being injected into PostListMainViewModel and all child VMs are utilizing it. This resolves the scoping issue and ensures the cache for all components are up to date.

Testing

  1. Open Post list
  2. Turn on airplane mode
  3. Open/Create a post and add a media item
  4. Click on Update/Publish
  5. Find the post in the post list
  6. Turn airplane mode off.
  7. Ensure the media upload progress is being shown.

  1. Open Post list
  2. Turn on airplane mode
  3. Open a post and make changes
  4. Click on Update/Publish
  5. Find the post in the post list
  6. Click on the "Cancel" action
  7. Make sure the Cancel action disappears

Reviewing

Only 1 reviewer is needed but anyone can review.

Submitter Checklist

  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.
  • I have considered adding accessibility improvements for my changes.
  • If it's feasible, I have added unit tests.

@peril-wordpress-mobile

peril-wordpress-mobile Bot commented Mar 19, 2020

Copy link
Copy Markdown

You can test the changes on this Pull Request by downloading the APK here.

@malinajirka malinajirka left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good catch @jd-alexander! Thank you for the fix. I've tested the changes and it works as expected.

I think we might want to consider making the uploadStatusTracker private and passing the tracker within the postListViewModelConnector for consistency. I didn't want to push commits into your PR and I wanted to speed things up. I tried this approach in move-tracker-to-connector branch. If you agree with the changes, feel free to merge them into issue-11471/post_list_upload_status_tracker.

I'm approving this PR. Feel free to merge it. I'm leaving the decision if you want to merge my branch to you.

Comment thread WordPress/src/main/java/org/wordpress/android/ui/posts/PostListFragment.kt Outdated
@malinajirka

Copy link
Copy Markdown
Contributor

The connected test has been failing even on other branches so I think we shouldn't block this PR on it -> 🚢

@malinajirka
malinajirka merged commit d7e9cb0 into feature/master-pages-offline-support Mar 19, 2020
@malinajirka malinajirka modified the milestones: 14.5, 14.6 Mar 23, 2020
@jkmassel
jkmassel deleted the issue-11471/post_list_upload_status_tracker branch October 17, 2024 18:53
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.

2 participants