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

SPIKE Review Fluent support for LinkFIeld #210

Closed
maxime-rainville opened this issue Feb 8, 2024 · 6 comments
Closed

SPIKE Review Fluent support for LinkFIeld #210

maxime-rainville opened this issue Feb 8, 2024 · 6 comments
Assignees

Comments

@maxime-rainville
Copy link

maxime-rainville commented Feb 8, 2024

We're not sure the LinkField's new implementation will work well with Fluent. The feedback we got from our bespoke devs that use LinkField v3 with Fluent was that in most cases the Link was attached to some parent object that was being translated. In practice, this meant that different language blocks would have different links for each language.

e.g.: Your create an elemental block with a link. The English block has a link pointing to an English website and the French Block has completely different link pointing to a French website.

Yet the scenario where the Link itself was the thing being translated was judged as desirable and would be considered a substantial shortcoming if it wasn't possible.

e.g. You create a Link pointing to a page. The English and French Link are joint together. User can choose to provide a different title for each Language.

Timebox

1 day

TODO

  • Validate that the "separate Link in translated block" approach still works as expected.
  • Validate if the LinkForm can be made to work with Fluent and allow the creating of translatable Link.
    • Is there a mechanims to copy Link content from one Local to an other.
    • Where a Link is pointing to specific page that has been translated, validate if the Link renders the correct title for the user chosen locale and if following the link URL uses the right locale.
    • If this approach is not working, document possible road block to its implementation.

Outcome

At the end of this SPIKE, we should:

  • know if Link can be translated with Fluent as-is,
  • have a rough idea of how much work would be needed to make Link translatable if they are not already,
  • be in a position to draft Fluent guidance for LinkField if Link can be translated.
@emteknetnz emteknetnz self-assigned this Feb 13, 2024
@emteknetnz
Copy link
Member

emteknetnz commented Feb 14, 2024

Summary:

Fluent seems to work fine for has_one LinkField's, however it is rough for has_many MutliLinkField's

Shortcomings:

Things work fairly well for the has_one LinkField, but seem to fall apart pretty badly for the has_many MutliLinkField. I'm not sure if that's because fluent seems to have not so great support for has_many (and many_many) relations, or if I just don't know how to properly use fluent

The only real shortcoming with the has_one was there's a 'Localisation' tab with a gridfield that's supposed to render, though because we're using a react modal there is no react gridfield that can be rendered. I need to add this to GridField.php to prevent a fatal exception:

// GridField.php
// ...
protected $schemaDataType = self::SCHEMA_DATA_TYPE_STRUCTURAL;

And it simply renders as nothing:

image

Possibly we could just manually remove the tab since its' non-functional

Otherwise things seemed to work as expected, based on my limited knowledge of fluent.

Has many was a different story unfortunately. Fluent docs indicate that you should use Locale based filter configuration for has_many and many_many, which means add the FluentFilteredExtension to Link.

I noticed the following issues:

  1. Neither the has_one and has_many links render in templates with the FluentFilteredExtension applied. This is completely busted an I cannot help thinking I'm doing something wrong here, however removing the extension means things work as expected i.e. visiting /en_US/about-us?stage=Stage shows my link relations as expected. Adding this to elemental BaseElement breaks ability to use elemental entirely. It seems like you simply cannot add this extension. Note I don't believe the following few points change whether or not the extension is added, so possibly this point is irrelevant.

  2. Adding a has_many link to one locale, will cause the link to be added to the other locale which is undesirable. This makes technical sense because the relation isn't actually localised. However because the Link is localised each version can still be edited independently. Publishing the has_many in another locale will not publish the version in the other locale. So other than the unintentionally created draft version this technically works. Note that this happens with elemental in general, though possibly I just had elemental with fluent setup sub-optimally.

  3. Sorting links is very bad. On one local if you sort you will get modified badges, on the other locale they'll sort, but they won't have modified badges, so it's sort straight to live? When clicking publish sometimes the sort order then changes again. There is a Sort column in LinkField_Link_Localised so they sorts should be independent. Possibly we we need to update a query in LinkFieldController to do something a bit more manual.

  4. There is also no purple 'Translatable field' icon added to the has_many field, because it is not localised. Note that I also seemed to get this with elemental in general, even though localisation was working on elemental.

Setup:

  • silverstripe/installer:5.x-dev
  • silverstripe/linkfield:4.x-dev
  • tractorcow/silverstripe-fluent:7.0.0

Add fluent support to the Link DataObject

---
Name: myfluent
---
SilverStripe\LinkField\Models\Link:
  extensions:
    - TractorCow\Fluent\Extension\FluentVersionedExtension
    # This is for has_many's, though breaks templates
    - TractorCow\Fluent\Extension\FluentFilteredExtension
  field_include:
    # need to manually include integer fields
    - OpenInNew
    - Sort
  field_exclude:
    # need to manually exclude this string field or is causes a mysql issue where it does not know which column to use
    - OwnerRelation

SilverStripe\LinkField\Models\FileLink:
  field_include:
    - FileID

SilverStripe\LinkField\Models\SiteTreeLink:
  field_include:
    - PageID

DNADesign\Elemental\Models\BaseElement:
  extensions:
    - TractorCow\Fluent\Extension\FluentVersionedExtension
    # This breaks elemental in the CMS
    # - TractorCow\Fluent\Extension\FluentFilteredExtension
  field_include:
    - Title

MyBlock:
  field_include:
    - MyLinkID
    - MyLinks

Fix required in silverstripe/framework so that we can submit the modal without getting a console warning. This gridfield will not render in a modal

// silverstripe/framework GridField.php
// ...
protected $schemaDataType = self::SCHEMA_DATA_TYPE_STRUCTURAL;
// app/src/Page.php
use SilverStripe\CMS\Model\SiteTree;
use SilverStripe\LinkField\Form\LinkField;
use SilverStripe\LinkField\Form\MultiLinkField;
use SilverStripe\LinkField\Models\Link;

class Page extends SiteTree
{
    private static $db = [];

    private static $has_one = [
        'MyLink' => Link::class,
    ];

    private static $has_many = [
        'MyLinks' => Link::class,
    ];

    private static $owns = [
        'MyLink',
        'MyLinks',
    ];

    private static $cascade_deletes = [
        'MyLink',
        'MyLinks',
    ];

    private static $cascade_duplicates = [
        'MyLink',
        'MyLinks',
    ];

    private static $translate = [
        'MyLinkID',
        // this does nothing:
        'MyLinks',
    ];

    public function getCMSFields()
    {
        // Fluent recommended way to add fields - this is so that we get the 'Translatable field' purple icons
        // https://github.com/tractorcow-farm/silverstripe-fluent/blob/master/docs/en/dataobjects.md#default-field-scaffolder)
        $this->beforeUpdateCMSFields(function($fields) {
            $fields->addFieldsToTab(
                'Root.Main',
                [
                    LinkField::create('MyLink'),
                    MultiLinkField::create('MyLinks')
                ]
            );
        });
        // need to ensure 'updateCMSFields` hook is called according to fluent docs
        $fields = parent::getCMSFields();
        return $fields;
    }
}
// app/src/MyBlock.php
use DNADesign\Elemental\Models\BaseElement;
use SilverStripe\LinkField\Models\Link;
use SilverStripe\LinkField\Form\LinkField;
use SilverStripe\LinkField\Form\MultiLinkField;
use TractorCow\Fluent\Extension\FluentVersionedExtension;
use TractorCow\Fluent\Extension\FluentFilteredExtension;

class MyBlock extends BaseElement
{
    private static $has_one = [
        'MyLink' => Link::class,
    ];

    private static $has_many = [
        'MyLinks' => Link::class,
    ];

    private static $owns = [
        'MyLink',
        'MyLinks',
    ];

    private static $cascade_deletes = [
        'MyLink',
        'MyLinks',
    ];

    private static $cascade_duplicates = [
        'MyLink',
        'MyLinks',
    ];

    private static $table_name = 'MyBlock';

    private static $singular_name = 'My Block';

    private static $plural_name = 'My Blocks';

    private static $description = 'This is my block';

    private static $icon = 'font-icon-block-content';

    public function getType()
    {
        return 'My Block';
    }

    public function getCMSFields()
    {
        $this->beforeUpdateCMSFields(function($fields) {
            $fields->addFieldsToTab(
                'Root.Main',
                [
                    LinkField::create('MyLink'),
                    MultiLinkField::create('MyLinks')
                ]
            );
        });
        $fields = parent::getCMSFields();
        $fields->removeByName('MyLinkID');
        return $fields;
    }
}

@emteknetnz emteknetnz removed their assignment Feb 14, 2024
@GuySartorelli
Copy link
Member

FluentFilteredExtension is not for use cases that make sense with Link I think, so that's probably just a red herring.

Adding a has_many link to one locale, will cause the link to be added to the other locale which is undesirable.

I wonder if this is related to the multi-relational has_one? Might be that we need to add special support for that relation type in fluent.

The other has_many issues might be related to that - or at least probably all share some root cause whatever that cause is.

@maxime-rainville
Copy link
Author

From reading the comments so far, I don't know that we would be able to scope a card to provide Fluent support or prowide good guidance on how to enable.

Do we feel that spending more time analysing this would allow us to reach more solid conclusions? Or this just a case of us needing to knock our collective head against the wall until we find solution?

Might be worth getting some terraformers eyes on this.

@mfendeksilverstripe
Copy link
Collaborator

Hey @maxime-rainville @emteknetnz , thanks for the write-up 😄 , looks like you had quite a journey with Fluent. Here are some of my thoughts on this:

Fluent customised CMS UI with React components

Fluent customisation generally doesn't work with React components as it heavily relies on GridField which does not work in React components. We have similar issue with Fluent CMS UI on the asset admin.

I'm not sure what's the best solution for this one but it seems we should aim for some common extensibility interface in React components as the GridField used to provide as things like localisation badges and button label customisations are likely to be very similar between different areas of the CMS (asset admin, linkfield...).

Has-many many-many fluent support & sorting

Completely agree that Fluent support of these features is very limited. I did try to set this up in a few different ways but I couldn't achieve good results. I think it is possible to set up something functional with the combination of localised many many through models and FluentFilteredExtension, however the outcome is not very content author friendly.

FluentFilteredExtension is poorly documented but the idea behind it is to provide model instance level of localisation control.

  • global level control (covered by Locale setup)
  • model level control (covered by inheritance mode config)
  • model instance level control (covered by FluentFilteredExtension)

This extension will prevent any content from showing on the frontend when it's applied so we expect content authors to maintain an allow list of locales where the content is allowed to be shown for this particular model instance (this is currently achieved via GridField which is probably not showing up in the LinkField).

This is technically functional but the moment you have to maintain more than 10 model instances it gets super tedious and unsustainable. This is the reason why I usually recommend the holder approach for localisation (for example Elemental area for blocks) as it's much easier to set up and maintain.

Overall, sorting capability requires sort data consistency within each locale. If links are individually localised they can have individual data states which breaks the consistency (for example link 1 has localised content and link 2 has inherited content).

I can see cases for localised links using has one as those would not need sorting capability though.

@maxime-rainville
Copy link
Author

Given the feedback provided here, I think we'll put "full blown Fluent support" in the too hard basket for now.

We'll still have a card to "document simple linkfield-fluent scenarios" but there's an expectation that it won't work for the more complex configurations.

I've created a follow up card to address the underlying problems:

@maxime-rainville
Copy link
Author

#237 is the follow up card.

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

No branches or pull requests

4 participants