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

Can't handle non-comments as first post #7

Open
clarkwinkelmann opened this issue Feb 23, 2020 · 2 comments
Open

Can't handle non-comments as first post #7

clarkwinkelmann opened this issue Feb 23, 2020 · 2 comments

Comments

@clarkwinkelmann
Copy link
Member

This extension expects $discussion->firstPost to be a CommentPost. The template reads $post->content and expect it to be text.

In the case of my Premium Wordpress extension, the first post may be an EventPost whose content is a JSON object, returned as a PHP array.

The consequence is a PHP notice for Array to string conversion that's rendered in the email if notices are visible, and where the text should be I can read just Array.

I have implemented some workarounds for other extensions. For example the SEO extension calls $post->formatContent(), which I have been able to implement in a way that's compatible with CommentPost.

However I cannot re-implement $post->content, or it would break the functionality of the JSON payload.

In order for the two extensions to work together, Follow Tags should call another method where third-party extensions can provide custom implementations for event posts. Another option would be to ignore non-comment posts, which would remove the enigmatic Array from the email, but would prevent integration with extensions like mine.

My suggestion would be to call formatContent(), and inject the HTML in the email. It would both solve this issue and add formatting. But I'm sure there were some concerns adding the post HTML in the email.

I actually would like to open a proposal in core to add an interface for posts that can be rendered as HTML. That way extensions will be free to register new post types, and other extensions will be able to know whether they can output their content or not.

@clarkwinkelmann
Copy link
Member Author

Also affects new discussion notifications or fof/subscribed. Though in that other case it outputs a Warning: htmlspecialchars() expects parameter 1 to be string, array given.

Exact same issue though. It expects $post->content to be text.

@clarkwinkelmann
Copy link
Member Author

Version 1.5.0 of the Wordpress extension ships with a workaround for this issue.

WordpressSummaryPost::$content now returns an Illuminate\Contracts\Support\Htmlable object which can be read as a string.

The idea behind the use of Htmlable is to render the post as HTML in mails and as Markdown in other contexts. Unfortunately due to the use of {!! !!} in the mail template here it renders as Markdown, and the same passes down to Pretty Mail. It would be nice if Pretty Mail could somehow use the Htmlable::toHtml version of the content, but well...

I think it's worth keeping this open until we find a nice way of implementing "renderable" custom post types in Flarum, as the issue could affect other community extensions as well. But for now it seems the Wordpress extension is still the only one playing with fire.

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

No branches or pull requests

1 participant