Issue 1: Send method does too many things (render + sending process enforcement). Complicating customization in each of them due to code duplication.
Issue 2: The method postSend method does too many things (decides to save or not, assigns rendered output to fields based on configuration, performs the actual saving of the message to DB). Complicating customization in each of them due to code duplication.
issue 3: On successful delivery and missing fields, an exception is thrown, preventing the save of the message (even if it should be saved). If there is an exception, it would have been better to not deliver the message or silence the error when the message was already delivered. This way some tracking could have been left in the DB for auditing / debugging.
issue 4:
Exposing the use of entity metadata wrappers, due to insufficient validation of whether the field is present on the particular message instance type. When there is a field in the system and it is configured to be used, but it is not part of the particular message instance's type, EntityMetadataWrapper exception is thrown instead of MessageNotifyException.
Proposals:
1. Add a designated render method that will handle that and just call it on send. (implemented in PR)
2. Move the sub-actions to separate methods and use them in postSend. (implemented in PR)
3.1. Expose the exception triggering, so custom plugins can decide to silence them in a way. (implemented in PR)
3.2. Move the fields assignment before the delivery of the message.
4. Add some more validation there and throw our exception. (implemented in PR)
Discussion for the API changes proposed.
Review the PR in github (https://github.com/Gizra/message_notify/pull/36).
Patch (if needed).
None.
API addition for anyone that wants to implement custom notifiers.
None.
Closed: outdated
2.0
Code
Enhances developer experience.
Not all content is available!
It's likely this issue predates Contrib.social: some issue and comment data are missing.