You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I create this issue to discuss possible ways to implement custom post types that contain text.
Where is the data used ?
$post->content is used in emails
The content might be used in web notifications but I don't think that's the case currently
$post->formatContent() is used by the SEO extension
Other extensions might use the data for custom notifications or indexing
Which posts are accessed ?
Most impacted emails are triggered via the Posted event, which is only dispatched for CommentPost by Flarum, but an extension might dispatch it with another post type. The type hinting on the event allows any post type
The SEO extension and FoF Follow tags access $discussion->firstPost. They both currently don't check that the post is a CommentPost before reading the content, but checking for that class explicitly would break the ability to do what this issue is about
A post might be renderable as text or HTML, but it might not use the TextFormatter for that
Custom post types must be able to declare whether they can be rendered as text or HTML
At the moment, because there's almost no checks for CommentPost, Flarum and extensions will happily call ->formatContent() on any post. This has the benefit of allowing custom post types to use it, but also throws errors if another post type ends up there
The use of ->content for the plain text version conflicts with the access to the ->content column of the database. It's not an issue for comment posts because there's nothing else to access, but because ->content is used for the JSON data of custom post types, re-declaring getContentAttribute() prevents sending the full payload via the serializer
Using ->content for plain text and still being able to access JSON would be possible, but I think re-naming the accessor to something that says "text" or "plain" will be much clearer
Use cases
My premium Wordpress extension registers a custom post type that contains an excerpt of the linked Wordpress post. That post stores a title, html body and link. This content can be dynamically rendered as a single HTML payload for notifications and SEO. It's expected that other extensions could access this content and treat it like a comment post
Imagined example: an automated script pushes content from a feed into a discussion. Each post contains a source url and content. You might want to receive posting notifications from the Subscription extension in that thread
Proposals
Easy but not satisfactory solution
We could add explicit checks for CommentPost everywhere before passing it to an email template or reading the content. This would mean every third-party extension that has such a custom post type would need to define their own logic for everything, including checking the other extension (like Subscription) is enabled and compose their own email template.
Pros: very little change to Flarum, we just prevent things from breaking and let extension developers manage everything.
Cons: compatibility between third-party extensions is complicated or impossible. An extension cannot simply register a new post type and expect that another notification extensions will pick it up.
Concrete solution
Add a new interface for posts. For example:
interface Renderable {
public contentPlain(ServerRequestInterface$request = null) {}
public contentHtml(ServerRequestInterface$request = null) {}
}
And update core extensions that handle email to check for that interface before trying to read the content.
One question regards the "plain text" version. If we used HTML in the emails we could completely skip that from the interface and only have an HTML output.
Another question is the naming. I don't really like the current names. I would like something that clearly says "whatever the post payload is it can be rendered as a single text". The intent of that interface would be to be used by default for everything notification or indexing related unless additional logic has been written for a specific post type.
This solution only covers the PHP side. Rendering the content in the javascript frontend would still require custom code for those special post types. But I don't think there are any major issues there, the frontend is sufficiently extensible for the developer to choose how such a post will be rendered.
The text was updated successfully, but these errors were encountered:
I create this issue to discuss possible ways to implement custom post types that contain text.
Where is the data used ?
$post->content
is used in emails$post->formatContent()
is used by the SEO extensionWhich posts are accessed ?
Posted
event, which is only dispatched forCommentPost
by Flarum, but an extension might dispatch it with another post type. The type hinting on the event allows any post type$discussion->firstPost
. They both currently don't check that the post is aCommentPost
before reading the content, but checking for that class explicitly would break the ability to do what this issue is aboutConcerns
$post
object can be rendered as text or HTML. Related Can't handle non-comments as first post FriendsOfFlarum/follow-tags#7CommentPost
, Flarum and extensions will happily call->formatContent()
on any post. This has the benefit of allowing custom post types to use it, but also throws errors if another post type ends up there->content
for the plain text version conflicts with the access to the->content
column of the database. It's not an issue for comment posts because there's nothing else to access, but because->content
is used for the JSON data of custom post types, re-declaringgetContentAttribute()
prevents sending the full payload via the serializer->content
for plain text and still being able to access JSON would be possible, but I think re-naming the accessor to something that says "text" or "plain" will be much clearerUse cases
Proposals
Easy but not satisfactory solution
We could add explicit checks for
CommentPost
everywhere before passing it to an email template or reading the content. This would mean every third-party extension that has such a custom post type would need to define their own logic for everything, including checking the other extension (like Subscription) is enabled and compose their own email template.Pros: very little change to Flarum, we just prevent things from breaking and let extension developers manage everything.
Cons: compatibility between third-party extensions is complicated or impossible. An extension cannot simply register a new post type and expect that another notification extensions will pick it up.
Concrete solution
Add a new interface for posts. For example:
And update core extensions that handle email to check for that interface before trying to read the content.
One question regards the "plain text" version. If we used HTML in the emails we could completely skip that from the interface and only have an HTML output.
Another question is the naming. I don't really like the current names. I would like something that clearly says "whatever the post payload is it can be rendered as a single text". The intent of that interface would be to be used by default for everything notification or indexing related unless additional logic has been written for a specific post type.
This solution only covers the PHP side. Rendering the content in the javascript frontend would still require custom code for those special post types. But I don't think there are any major issues there, the frontend is sufficiently extensible for the developer to choose how such a post will be rendered.
The text was updated successfully, but these errors were encountered: