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

Fix: Null Pointer Exception in PublishSettingsViewModel #20745

Merged
merged 4 commits into from
May 2, 2024

Conversation

zwarm
Copy link
Contributor

@zwarm zwarm commented May 1, 2024

Fixes #20272

This PR fixes a NPE when accessing EditPostRepository.dateCreated. The getter for dateCreated uses a post!! which will throw a NPE if the post is null. This PR replaces the use of let with takeIf { it.hasPost() } to ensure we don't call dateCreated when there is no post.


To Test:

I was unable to recreate the issue, but did write a unit test to validate the new takeIf logic.
Please verify that all unit tests pass CI


Regression Notes

  1. Potential unintended areas of impact
    Continue to experience the NPE crash

  2. What I did to test those areas of impact (or what existing automated tests I relied on)
    Add unit test to EditPostPublishSettingsViewModelTest

  3. What automated tests I added (or what prevented me from doing so)
    N/A


PR Submission Checklist:

  • I have completed the Regression Notes.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

Testing Checklist (strike-out the not-applying and unnecessary ones): N/A

@zwarm zwarm added this to the 24.9 milestone May 1, 2024
@zwarm zwarm requested a review from pantstamp May 1, 2024 15:02
@zwarm zwarm self-assigned this May 1, 2024
@wpmobilebot
Copy link
Contributor

wpmobilebot commented May 1, 2024

Jetpack📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.
App NameJetpack Jetpack
FlavorJalapeno
Build TypeDebug
Versionpr20745-980cb73
Commit980cb73
Direct Downloadjetpack-prototype-build-pr20745-980cb73.apk
Note: Google Login is not supported on these builds.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented May 1, 2024

WordPress📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.
App NameWordPress WordPress
FlavorJalapeno
Build TypeDebug
Versionpr20745-980cb73
Commit980cb73
Direct Downloadwordpress-prototype-build-pr20745-980cb73.apk
Note: Google Login is not supported on these builds.

Copy link

codecov bot commented May 1, 2024

Codecov Report

Attention: Patch coverage is 66.66667% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 40.62%. Comparing base (3be32da) to head (96460b9).

❗ Current head 96460b9 differs from pull request most recent head 980cb73. Consider uploading reports for the commit 980cb73 to get more accurate results

Files Patch % Lines
...press/android/ui/posts/PublishSettingsViewModel.kt 66.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##            trunk   #20745   +/-   ##
=======================================
  Coverage   40.62%   40.62%           
=======================================
  Files        1489     1489           
  Lines       68531    68532    +1     
  Branches    11326    11326           
=======================================
+ Hits        27841    27842    +1     
  Misses      38161    38161           
  Partials     2529     2529           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@AjeshRPai AjeshRPai self-requested a review May 2, 2024 04:52
Copy link
Contributor

@AjeshRPai AjeshRPai left a comment

Choose a reason for hiding this comment

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

Hey @zwarm

Changes LGTM, I couldn't reproduce the crash as well but your changes makes sense. Did a minor formatting in commit 980cb73

@AjeshRPai AjeshRPai enabled auto-merge May 2, 2024 04:56
Copy link

sonarqubecloud bot commented May 2, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@AjeshRPai AjeshRPai merged commit 5837010 into trunk May 2, 2024
20 checks passed
@AjeshRPai AjeshRPai deleted the issue/20272-npe-publish-settings branch May 2, 2024 05:27
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.

NullPointerException
3 participants