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

Navigation - Fix serialization error #11107

Merged
merged 1 commit into from
Oct 12, 2017

Conversation

totten
Copy link
Member

@totten totten commented Oct 10, 2017

Overview

The navigation subsystem stores per-user data in the table
civicrm_setting. Most access to this table goes through
CRM_Core_BAO_Setting or Civi\Core\SettingsBag. However, certain
edge-cases produce a warning about a serialization error because
CRM_Core_BAO_Navigation does not participate.

Before

CRM_Core_BAO_Navigation::resetNavigation() generates malformed
records which cannot be read via CRM_Core_BAO_Setting or
Civi\Core\SettingsBag.

After

CRM_Core_BAO_Navigation::resetNavigation() generates well-formed
records which can be read via CRM_Core_BAO_Setting or
Civi\Core\SettingsBag.

Overview
--------

The navigation subsystem stores per-user data in the table
`civicrm_setting`.  Most access to this table goes through
`CRM_Core_BAO_Setting` or `Civi\Core\SettingsBag`.  However, certain
edge-cases produce a warning about a serialization error because
`CRM_Core_BAO_Navigation` does not participate.

Before
------
`CRM_Core_BAO_Navigation::resetNavigation()` generates malformed
records which cannot be read via `CRM_Core_BAO_Setting` or
`Civi\Core\SettingsBag`.

After
-----
`CRM_Core_BAO_Navigation::resetNavigation()` generates better records.
@eileenmcnaughton
Copy link
Contributor

SO we make this change & it still generates malformed messages?

@totten
Copy link
Member Author

totten commented Oct 11, 2017

@eileenmcnaughton Thanks, that was a mistake in the description. (Fixed now.) After the change, it should make well-formed records in civicrm_setting.

@aydun
Copy link
Contributor

aydun commented Oct 12, 2017

Tested & works for me.

@eileenmcnaughton
Copy link
Contributor

Merging based on @aydun testing, minor code change with constrained impact

@eileenmcnaughton eileenmcnaughton merged commit 5128639 into civicrm:master Oct 12, 2017
@eileenmcnaughton
Copy link
Contributor

But, I don't think this should have gone without a JIRA!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants