Skip to content

Issue/3855 plans not serializable#3857

Merged
daniloercoli merged 2 commits intodevelopfrom
issue/3855-plans-not-serializable
Mar 14, 2016
Merged

Issue/3855 plans not serializable#3857
daniloercoli merged 2 commits intodevelopfrom
issue/3855-plans-not-serializable

Conversation

@nbradbury
Copy link
Copy Markdown
Contributor

Fixes #3855

To test: Open plans, then background the app. Prior to this fix, the app would crash with a NotSerializableException.

The problem was caused by serializing the Plan object (mPlanDetails) when saving the fragment state because the Plan contains a "PlanFeaturesHighlightSection" object which didn't implement Serializable.

Rather than resolve this by implementing Serializable in PlanFeaturesHighlightSection, I chose to stop serializing the Plan object. When the SitePlan is restored, it takes care of loading the correct plan. I decided this was a safer approach due to the downsides of serializing (namely, it leads to issue like this one!).

Needs review: @daniloercoli

@daniloercoli
Copy link
Copy Markdown
Contributor

:shipit:

This is probably going to be changed soon, since the new version (1.3) of the site/plans/ endpoint should contain all the info required to build the UI. Global plans should not be needed anymore.

daniloercoli added a commit that referenced this pull request Mar 14, 2016
…erializable

Issue/3855 plans not serializable
@daniloercoli daniloercoli merged commit c1b7eae into develop Mar 14, 2016
@daniloercoli daniloercoli deleted the issue/3855-plans-not-serializable branch March 14, 2016 09:32
@nbradbury
Copy link
Copy Markdown
Contributor Author

the new version (1.3) of the site/plans/ endpoint should contain all the info required to build the UI. Global plans should not be needed anymore.

💯

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.

Plans NotSerializableException

2 participants