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

Extend notification model with optional fields (redone) #63

Draft
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

raaaahman
Copy link
Collaborator

@raaaahman raaaahman commented Jul 20, 2021

Before

The WP_Notify_Base_Notification model was not up-to-date with the optional fields defined in PR #21

See #60

Changes

  • Introduces a 'composite notification' model that can take additional fields
    • FIELD_TITLE
    • FIELD_IMAGE
    • FIELD_ACTION_LINK
  • Serializes the notifications with the optional fields into JSON
  • Unserializes notifications with optional fields from JSON

Replaces #61

TODO

  • Test serializing and unserializing notifications with several of the optional fields
  • Refactor WP_Notify_Composite_Notification::json_unserialize() to not encode and decode JSON data many times
  • (Maybe) Merge the WP_Notify_Composite_Notification with the WP_Notify_Base_Notification model

@raaaahman raaaahman added [Scope] Model Definition of the data used in the WP Notify project [Type] Feature Describe a feature to be added to the project labels Jul 20, 2021
@raaaahman raaaahman marked this pull request as draft July 20, 2021 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Scope] Model Definition of the data used in the WP Notify project [Type] Feature Describe a feature to be added to the project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant