Skip to content

Conversation

@sathyapulse
Copy link

The zoninator_insert_zone and zoninator_update_zone hook sends the serialized description data and it requires additional code to unserialize, add additional fields and serialize data again. The PR addresses the problem by serializing the data after the apply_filters hook so that we can make sure the data is serialized always.

@philipjohn
Copy link
Contributor

Hi @sathyapulse, thanks for this.

Can you provide more detail about the issue this resolves please, including exact steps to reproduce?

@sathyapulse
Copy link
Author

@philipjohn It doesn't solve any issues but improvement. It's just to avoid additional code from the developers end as well as make sure the data is serialised while saving to database.

Let's say we are extending the fields with the action hook zoninator_post_zone_fields in here. We need to include the new field values while inserting and updating the zone with the filter hooks here and here. The data is serialised before apply_filters in insert and update functions. In the add_filter, developers needs to unserialise data, insert the new field values, serialise data again and return. The new code will send the array to apply_filters and developers can directly insert the data and plugin will make sure the data is serialised.

I think it will not create any issues with the existing code of developers since the we are using maybe_serialize function.

@GaryJones
Copy link
Contributor

I think it will not create any issues with the existing code of developers since the we are using maybe_serialize function.

I think you're right, but it still feels like this should be considered to have the potential to be a breaking change, and so would need to wait for a major release.

@GaryJones GaryJones added this to the Future milestone Aug 8, 2024
@GaryJones GaryJones changed the base branch from main to develop August 8, 2024 19:55
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.

4 participants