Skip to content

Fix null site model in PostListActivity#10826

Merged
malinajirka merged 4 commits intodevelopfrom
fix/null_site_model
Nov 27, 2019
Merged

Fix null site model in PostListActivity#10826
malinajirka merged 4 commits intodevelopfrom
fix/null_site_model

Conversation

@planarvoid
Copy link
Copy Markdown
Contributor

Fixes #8363

We're still passing nullable SiteModel to the intent when creating activities. This PR should fix that when creating the PostListActivity. All the places should now be checked for non-null SiteModel. I don't know which solution is the best.

viewCurrentBlogPages had already NonNull parameter but we were still passing a nullable SiteModel. That's why is my solution different than the one with viewCurrentBlogPosts.

To test:

PR submission checklist:

  • I have considered adding unit tests where possible.

  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@planarvoid planarvoid added this to the 13.8 milestone Nov 21, 2019
@planarvoid planarvoid self-assigned this Nov 21, 2019
@peril-wordpress-mobile
Copy link
Copy Markdown

peril-wordpress-mobile bot commented Nov 21, 2019

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

@planarvoid planarvoid marked this pull request as ready for review November 22, 2019 09:29
Copy link
Copy Markdown
Contributor

@malinajirka malinajirka left a comment

Choose a reason for hiding this comment

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

Thanks @planarvoid! I think I found one bug, but it looks good otherwise.

ActivityLauncher.viewCurrentBlogPages(requireActivity(), getSelectedSite());
SiteModel selectedSite = getSelectedSite();
if (selectedSite != null) {
ToastUtils.showToast(getActivity(), R.string.site_cannot_be_loaded);
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.

Shouldn't this line be in an else clause? Wdyt?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch, I've fixed it in 60bcddc

@planarvoid
Copy link
Copy Markdown
Contributor Author

@malinajirka thanks for the check! I've fixed what you found and it should be ready for another check.

Copy link
Copy Markdown
Contributor

@malinajirka malinajirka left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

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.

Rare NPE crash in ActivityLogListFragment

2 participants