contentHtml attribute on Post

matteocontrini

Was the contentHtml attribute on a Post removed in beta9? It was previously available but it seems missing now.

This affected the SEO extension by @JasperVriends as well, as seen in this fix commit.

I relied on that for generating HTML emails instead of plaintext ones, but it's gone 😱

clarkwinkelmann

matteocontrini nothing has changed on the serializer, the API was returning and still returns a contentHtml property.

However you're correct, the attribute was renamed on the Post model in PHP. It was previously registered as an Eloquent property via getContentHtmlAttribute and could be accessed via $model->content_html or $model->contentHtml. Now it has been replaced with a method is called formattedContent that takes the request as a parameter.

This is the commit that contains the changes

The SEO extension could probably use the new formattedContent method.

We are working on adding a developer changelog for the next releases.

matteocontrini

I see, thank you. Is the request parameter required when calling the formatContent method? Would it work without it? I can imagine that there might be situations where there's no request and one "simply" wants an HTML version of the post content.

clarkwinkelmann

matteocontrini I think the $request parameter is currently non-nullable. You might want to open an issue about this.

It's of course always possible to instanciate the post formatter from an extension, then parse the text directly without going through the formatContent method. This is what the SEO extension seems to be doing.

matteocontrini

clarkwinkelmann this might be a dumb question, but how should the code access the current request? Should it be passed somehow by the "framework" or can it be retrieved with something like app(i_dont_know) in any context? For example, can the notification blueprints (like this one) retrive the current request? In that context having the post HTML content is useful if you want to generate an HTML email/notification.

Before opening an issue I'd like to see if this is actually a problem or if it's just easier to pass the current request.

Thanks for your time, as always.

clarkwinkelmann

matteocontrini I think there is a way to access the request via the app container, and therefore via app(), but this is something we're trying to avoid. Not 100% sure what the reasons behind this architecture are because in Laravel it's pretty much standard to access all global objects out of the container. We have an issue about removing global helpers here flarum/core1796

The request object is usually available in the events where it makes sense, usually around the HTTP/API parts.

I'm not sure why the post formatter need access to the request, it doesn't seem logical. Given it doesn't interact with the HTTP stack... I have not dived into the source code to check that yet.

matteocontrini

clarkwinkelmann I'm not sure why the post formatter need access to the request, it doesn't seem logical. Given it doesn't interact with the HTTP stack..

The commit you pointed to above says:

This is important because extensions may wish to render post content
differently depending on Request factors such as the actor. For example,
an attachments extension might wish to hide attachments from guests.

But unless I'm worse than I thought at PHP, the $request parameter is optional an defaults to null, doesn't it?

flarum/core0ab9fac#diff-609ad82b0ebea8caea9b874de0f99d29R45

Right. The correct link.

EDIT: I think I see what you mean, the formatContent method requires the parameter as non-null

clarkwinkelmann

matteocontrini correct.

But the formatedContent method doesn't have = null so I think it won't accept null values in the Post class. We should probably add null as default there as well if we want extensions to use that method in contexts where the request doesn't make sense.

matteocontrini

Opened an issue: flarum/core1840

I tested the change on my installation and everything's fine.