MagicContentTrait DX improvements about core HOT 25 CLOSED

I (personally) don't really like that. It is too big a break from what we've used before, and it looks less clean i think..

{{ record.link() }} -> is the link of a record
{{ record|link() }} -> is a record processed into a link.

I'll ask my colleagues what they think as well!

from core.

I like it - even if it changes stuff.. Bolt 4.x will not be 100% compatible with Bolt 3.x anyway.

So {{ record.link }} gives me the link field in the record and {{ record|link() }} generates a link for the record based on the record content and maybe some more information in the configuration.

Contrary to what @sbonardt says I like the part where {{ record.link }} gives me a value and {{ record|link() }} does something with a record to give me something (which is what a filter does).
Stuff like {{ record|title()|trim('/') }} still works and is problably more what twig expects than the dot notation.

Whith filters you can also make variations like {{ record|link('fqdn') }} generate https://www.example.com/record/slug and {{ record|link('relative') }} generate /record/slug without much semantic overhead for front-end devs.

This will make life for front-end devs who work in the twig templates much easier compared to bolt 3 where the magic is getting in the way sometimes.

For people who create the back-end templates the JSON probably needs changes, but the whole back-end is new anyway so it does not really matter if we change it to one option or the other.

I would choose the version that makes life better for the front-end devs ({{ record|link() }}) at this point, because they are many and the rest of the devs are few.

from core.

I like the |link() solution, it's (technically) clear what it does. Only thing is that it initially may not be clear to someone who knows a bit less about Bolt ("What's the difference? Why?"). But the distinction will be:

• a contenttypes.yml defined field, use .fieldname
• every internal/magic function is |link or |link()

My reasons:

• I want to be able to use field names like link
• I don't want some random magictitle derivate values in my instances
• I think a Content object should be really stupid (less magic, more predictable)

For API/JSON context, I think there needs to be a function that transforms a Content instance to proper JSON. E.g. for JSON API, you need something like this:

{
    "data": [{
        "links": {
            "self: {{ record|link() }}"
        },
        "attributes": {
            "title": "{{ record.title }}",
            "foo": "{{ record.foo }}",
            ...
        }
    }]
}

Do note that in this case, you need to think of a different way to generate titles/links in PHP.

from core.

Ok this is what I would propose:

1. Move "magic" methods: link, editLink, excerpt, iimage, previous, next - to Twig filters, so they become:

• {{ content | link }} or {{ content | link() }} or {{ content | link(absolute=false) }} gives relative link
• {{ content | link(absolute=true) }} or {{ content | link(true) }} gives absolute link
• {{ content | edit_link }} will work the same (relative/absolute) but it COULD be restricted to work only for logged-in users
• {{ content | magic_image }} useful in Backend (MAYBE we could add here {{ content | magic_image(field=another_image_field) }} ?)
• {{ content | magic_title }} useful in Backend
• {{ content | excerpt }} or {{ content | excerpt(length=300) }} useful in Backend
• {{ content | next_record }} and {{ content | previous_record }}

2. Let's think about API output for get_content endpoint, how could it look like with magics included? I would recommend including only link and editLink as the others are more needed in backend than frontend. So it can become something like:

{
    "id": 3,
    "slug": "random-slug"
    "contentType": "blogpost"
    "fields": {
        "title": "Random title"
        "teaser": "...",
        "image": {
            "url": "..."
            "alt": "..."
        }
    }
    "extras": {
        "link": "magicLink here"
        "editLink": "magicEditLink here"
    }    
}

It's just an example. Due to API Platform elastic mechanisms it can look just like frontend devs would love it :)

3. Same thing applies to serialized Content object. I would recommend to make it look same (or almost same) to what API returns.

4. Last but not least, all those magic domain logic will be separated from Model (Doctrine entities) and View (Twig filters will be just aliases to services, API views will just serialize domain models here). Also since we will get specialized services for those tasks then will be unit testable.

from core.

Also in MediaFactory:

• can you replace it with an \Exception and add @todo? At least for getting trace where it happened :) done

from core.

I really don't like all those "magic" methods in ContentMagicTrait. Can we at least rename them to be less magical? :)

In Bolt 3 we also had these, only they weren't named as such. And IMHO that gave a bigger problem. When you'd get title, you'd either get the magic title-like result, or the actual field named title. It was a source of confusion.

I've named them like this to make it explicit.. You can now fetch what you need, and you'll know what to expect as the result.
What would you change it to, instead?

if we could make $this->fields a name-indexed array, using this method be much faster (simple isset instead of foreaching).

Yes, certainly.. What we have now is basically the generated piece of code from make:entity.
I was thinking about turning it into a Collection, too.

rename to get to getField, has to hasField

Will do.

can you replace it with an \Exception and add @todo? At least for getting trace where it happened :)

Allright, allright.. ;-)

from core.

What would you change it to, instead?

I would like to get rid of them, completely. I don't feel they are really needed (at least inside an entity, they can be moved to some service).

from core.

I don't feel they are really needed

The are needed on the overview pages to show titles, excerpts, thumbnails. They are needed in the sidebar menu to link the 'last 5 items'.. In the "frontend" they're used for listing pages as well as 'generic content pages'.

I'm open to renaming them, but they are most certainly required. :-)

from core.

if we could make $this->fields a name-indexed array, using this method be much faster (simple isset instead of foreaching).

Over the weekend i worked on getLocalisedFields() and related methods. These return named fields, as a Collection.

from core.

I know they are used here and there. But an Entity is not a good place for logic like this. That's why I'd vote for moving them away. It breaks SRP, Law of Demeter and probably a few other OOP rules.

OR

Add tests to cover every use case of those service-alike entities ;)

from core.

That's why I'd vote for moving them away.

Not opposed to that. How would you do this? To me, it's very important that it doesn't detract from the theme/site implementors ease of use that can now simply use {{ record.excerpt }} and know they'll get an excerpt.

Add tests to cover every use case of those service-alike entities

Excellent point. We absolutely need to get started on adding unit tests too. :-)

from core.

See #168

from core.

How would you do this?

Entity Decorator with logic needed for things like getLink or Excerpt.

OR

use twig filters instead:
{{ record | excerpt(200) }}
{{ record | link(absolute=true) }}

I would vote on the second as it will be still easy for frontend devs. @bobdenotter how about you?

from core.

And then we could get rid off service container inside an entity

from core.

Additionally, for the sake of conversation: Let's say we make all the current "magic" methods into filters.. How would they be usable in an API / JSON context?

from core.

As a frontenddeveloper (and colleague of Bob, who asked me to take a look at this as well), I am in favour of less magic.

I don't know any technical implications, but I'd prefer to get a field when I ask for a record.link, and a generated thing when I ask for record | link().

I sometimes stumble upon code where Bolt magically tries to guess what I want, when I think i clearly ask for a field, so I like @JarJak s solution. :)

from core.

I'm in favour of no new custom filters. Also I feel using the '.' with () gives it a more of function approach/feel to it.You have a record and .link() gives you the link to it.

A record where you |filter the link doesn't feel right to me somehow.
I filter stuff off of values, like record.title|trim('/')

So I'm with team .link()

from core.

@anketwokings The way it was "magic" in Bolt 3 is mostly solved because the current way in the prototype makes it a lot more consistent.

What you describe as your problem is a result of a faulty implementation. With the correct implementation of it, I think it's more elegant than a bunch of filters.

from core.

@bobdenotter Had to dig, but I encountered it only one time that I can remember; Old issue alert -> bolt/bolt#6798

from core.

@bobdenotter API output will not be a problem. It can include and exclude anything or be something totally custom.

Doctrine's Entity is tightly coupled with the Persistence Layer. So it should stay as simple as possible. Then we can make another (domain) layer where we can put any logic we want. Then we go to Presentation layer - what API actually outputs. I would recommend avoid mixing those layers (call them MVC, ADR, CQS, whatever).

from core.

Another thing I have noticed: ContentmagicTrait uses Twig_Markup so we are mixing M with V layers there.

from core.

bolt/bolt#6985 similar discussion from a year ago

from core.

Ok, if the consensus is that |filter is a good solution, and we can keep them in the "extras" in JSON / API, let's roll with it!

One small remark: magic_image, magic_title and excerpt are used in the frontend as well:

• In the "out of box" experience, where it "just works" when you install it, and have some random content
• In listings like search results, where we display different types of content mixed together, regardless of which fields they have
• In "RAD", because it's easy to just use the 'record.twig' at first, and replace it with a custom template later.

So, we should just make them available in the frontend as well, and i'm all good. 👍

from core.

@bobdenotter ok but do you include headless frontend?

from core.

Headless frontend is considered JSON / API, afaic. So, if we can include them in the "extras" for those usecases, that's great. :-)

from core.