tractorcow-farm / silverstripe-fluent

Multi-language translate module for Silverstripe, without having to manage separate site trees.
BSD 3-Clause "New" or "Revised" License
93 stars 111 forks source link

Fluent + Elemental #291

Closed thezenmonkey closed 6 years ago

thezenmonkey commented 7 years ago

I'm having trouble getting Fluent to play nice with Elemental. My Config look like this

Widget:
  extensions:
    - 'FluentExtension'
    - 'FluentFilteredExtension'

However on and SubClasses of BaseElement only Title shows correctly in the CMS. The translated Values write correctly to the database, but the CMS show the value for the default locale.

I tried extending FluentExtension and applying it to both my Subclassed Elements as well as Widget with no effect.

tractorcow commented 7 years ago

Unfortunately I don't have much experience with elemental. Does it work if you use a basic dataobject attached as a has_many to a parent page?

thezenmonkey commented 7 years ago

Yes. The way the Element works is that BaseElement extend Widget, it adds a couple of minor fields to control behaviour. All other Elements are subclasses of the BaseElement and add any fields needed for content.

tractorcow commented 7 years ago

I may need to have a look at the widget code to understand this issue, sorry. I appreciate the feedback thanks let me know if you find out anything in the mean time. :)

thezenmonkey commented 7 years ago

I'll keep plugging away. It's obviously something with when/where fluent gets injected. It writes to the correct field, just reads the incorrect one. The language portion of this project has been delayed as the client doesn't have bandwidth, so I have a bit of a reprieve.

robbieaverill commented 7 years ago

I'm not sure if it helps, but I've implemented fluent and elemental in an SS3 project before and had it working nicely. I can't remember whether we applied the FluentExtension to BaseElement or to Widget though

tractorcow commented 7 years ago

My recommendation is to always add the extension to the base class (direct subclass of DataObject); If you add it to a non-base you'll have trouble. :)

thezenmonkey commented 7 years ago

@tractorcow for more info Base Class is Widget do I've appoint Fluent to.

BaseElement extends Widget and has a VersionedDataObject applied as an Extension.

My Elements Extend BaseElement or each other.

I suspect it's the VersionedDataObject extension that's causing the problem since it forces the the CMS thread from stage. So it read MyField instead of MyField_de_DE or other language. I tried subclassing FluentExtension and Applying it my Element Subclass, but that didn't fix it either.

thezenmonkey commented 7 years ago

I think I've sorted it out. Though I'm not sure what caused the issue. My DB fields were named Content1, Content2, Content3. Not sure why it was an issue but renaming them ContentOne, ContentTwo, ContentThree seemed to fix it. I think I ran into a similar issue on a project 2 years ago. I'll have to remember it.

tractorcow commented 7 years ago

Oh I see, that might be a serious ORM issue if that's the case.

thezenmonkey commented 7 years ago

Yeah, I'll run some tests on clean install and add plugins and simple data objects to see if I can replicate consistently. It happened on MyElement which extends BaseElement, which has Versioned Applied. Base Element is an extension of Widget with has Fluent applied. and it's all being managed by a Versioned GridField.

MightyMerio commented 6 years ago

I am also attempting to combine Fluent and Elemental, on SS4. My approach currently is to try to translate the top level ElementAreaID, so that languages have their own set of blocks, rather than translating fields on each and every type of block. Elemental Page is my block holder and extends Page:

ElementalPage:
  extensions:
    - DNADesign\Elemental\Extensions\ElementalPageExtension
  translate:
    - 'ElementalAreaID'

My problem is that the Same ElementalAreaID copies over to the new translation, and I am unsure how to prevent that, and somehow get it to create a New ElementArea when the translation record is created. Is there a hook i can use in Fluent to catch this?

tractorcow commented 6 years ago

There's a solution for that. Will page the dev to get back to you. :)

mfendeksilverstripe commented 6 years ago

We have successfully created a well working setup of Fluent and Elemental modules on our project. Setup specification below:

Fluent

Note that Fluent is automatically applied to the SiteTree if you want the ability to block certain pages from specific locales you can use the FluentFilteredExtension (this is optional).

Elemental setup

You need to apply the ElementalPageExtension to a class, I recommend creating a new page type for example BlockPage. I strongly recommend avoiding applying the extension directly to the Page.

Fluent + Elemental setup

Initial setup will have the localized pages share the blocks, this is because by default the Fluent does not localize relation fields. Fortunately, there is a way how to configure this. The FluentExtension has following configuration options:

In our case we need to localize the ElementalAreaID in the BlockPage. We added this code below to the BlockPage

    /**
     * Fluent specific configuration
     * @var array
     */
    private static $field_include = [
        'ElementalAreaID',
    ];

If you run dev/build you should see that a localized table for BlockPage has been created and it contains the column ElementalAreaID. In our case the table name would be BlockPage_Localised.

Now we have the necessary structure but the problem is that when a localized page is created the newly created page will inherit the field values from the parent locale. It is not an issue for basic fields like text field as it is expected the CMS user to change the text field value later. For relation fields, this is a problem because the two pages will share the relation and thus all blocks.

To remedy the situation we need to add following code to the BlockPage:

    public function onBeforeWrite()
    {
        parent::onBeforeWrite();

        // detect top level locale
        $currentLocale = Locale::getCurrentLocale();
        $isTopLevelLocale = (is_null($currentLocale) || $currentLocale->getChain()->count() == 1);

        // if localised page instance is being created proceed with recursive duplicate of elemental data
        if (!$this->isDraftedInLocale() && !$isTopLevelLocale) {
            // clone elemental area

            /** @var $elementalAreaNew ElementalArea  */
            $elementalAreaNew = $this->ElementalArea()->duplicate();
            $elementalAreaOld = $this->ElementalArea();
            $this->ElementalAreaID = $elementalAreaNew->ID;

            // clone elements

            /** @var $elements HasManyList */
            $elements = $elementalAreaNew->Elements();
            foreach ($elementalAreaOld->Elements() as $elementOld) {
                /** @var $elementOld BaseElement */
                $elementNew = $elementOld->duplicate();
                $elements->add($elementNew);
            }
        }
    }

The code above will duplicate the ElementalArea for the newly created localized page. As a result, the blocks are no longer shared between those two pages.

Furthermore, we also want to disable the inheritance for blocks. The Fluent module provides multiple extension points, one of them being the updateLocaliseSelect. We need to create an Extension with the following code and apply it to the BlockPage:

    /**
     * Override default Fluent fallback
     * @param string $query
     * @param string $table
     * @param string $field
     * @param Locale $locale
     */
    public function updateLocaliseSelect(&$query, $table, $field, Locale $locale)
    {
        // disallow elemental data inheritance in the case that published localised page instance already exists
        if ($field == 'ElementalAreaID' && $this->owner->isPublishedInLocale()) {
            $query = '"' . $table . '_Localised_' . $locale->getLocale() . '"."' . $field . '"';
        }
    }

That should be all required setup. I hope this helps :)

MightyMerio commented 6 years ago

This. Is. Fantastic!

Thank you so much :-) Exactly what i was hoping for!

The logic and available variables for the onBeforeWrite is where I was swimming blindly. This is amazing, thank you.

madmatt commented 6 years ago

Hey @mfendeksilverstripe, you mentioned the following:

I strongly recommend avoiding applying the extension directly to the Page.

Could you clarify what you mean here? Is there a particular problem with applying it to a psuedo-top level class like Page - or is it just recommended to ensure that you can create other objects that don't include automatic fluent mapping? Is it something about fluent and elemental together that makes you say that?

Just curious :)

robbieaverill commented 6 years ago

This is one reason not to apply directly to Page: https://github.com/dnadesign/silverstripe-elemental-userforms/issues/10

mfendeksilverstripe commented 6 years ago

@madmatt The reason is quite simple - you may want to have pages without blocks someday and if you do, it's much easier to add them. Also, there is no downside. The effort to create a BlockPage at the start of the project is quite small :).

bummzack commented 6 years ago

Just want to note that applying the fluent to BaseElement works fine as well, if you want the same blocks for each locale (with translated contents).

MightyMerio commented 6 years ago

I did consider translating blocks content as well, but i run into issues such as blocks that have dependent dataobjects, occasionally an image field needs to be different for a language, and what about the alt tags on that particular image field, etc. Keeping the entire block set different per lang feels much less error prone, and clean :-)

madmatt commented 6 years ago

@mfendeksilverstripe, @robbieaverill: Awesome, both good reasons. Thanks! :)

MightyMerio commented 6 years ago

@mfendeksilverstripe , there is one method i am not able to resolve:

if (!$this->isDraftedInLocale() && !$isTopLevelLocale) { ...

isDraftedInLocale() does not exist. I was not able to find this method in any of the Fluent classes.. Am I missing something?

isPublishedInLocale()

Also can not be found. Where do these methods exist?

Many thanks

mfendeksilverstripe commented 6 years ago

@MightyMerio

It's in here:

https://github.com/tractorcow/silverstripe-fluent/blob/master/src/Extension/FluentVersionedExtension.php

MightyMerio commented 6 years ago

@mfendeksilverstripe - ah, I think it the version i am using.. I have composer set to "tractorcow/silverstripe-fluent": "^4",

What branch should I be using?

mfendeksilverstripe commented 6 years ago

@MightyMerio I'm using "tractorcow/silverstripe-fluent": "dev-master"

MightyMerio commented 6 years ago

@mfendeksilverstripe , are you on the SilverStripe slack channel? I feel like I almost had this working. The actual logic was working well on the previous version of fluent, even though I got that method not found error. Now with the more recent version of Fluent, this approach is not creating the duplicate blockarea any longer. I would love to get a little more help if possible, but not clog up the github thread :-)

mfendeksilverstripe commented 6 years ago

@MightyMerio I do use Slack, just look me up mfendek@silverstripe.com.

tractorcow commented 6 years ago

Just going to step in to say that the recommendation for fluent is the opposite to the recommendation for elemental; You should ONLY apply fluent to the base class, never a sub-class. E.g. SiteTree, BaseElement, but never Page / BlogPage / BlogElement.

tractorcow commented 6 years ago

@mfendeksilverstripe @MightyMerio I've tagged 4.0.0-beta1 of fluent which has the necessary functionality.

https://github.com/tractorcow/silverstripe-fluent/releases/tag/4.0.0-beta1

You can use ^4@beta as a dependency constraint to get this pulled in.

robbieaverill commented 6 years ago

I think that we might need some integration modules for elemental and other modules that deal with content fields, such as userforms. The goal would be that the "other modules" could still do whatever they want to do, but that the integration module would transfer the impact from the page to the content element (for example), and the other goal would be that you can and should apply elemental directly to Page or SiteTree

tractorcow commented 6 years ago

I'm going to close this as I think most of the concerns raised in this issue are generally met... please ping me if anything in particular is still broken.

lerni commented 6 years ago

@bummzack - is there a reason you didn't apply fluent to Widget? -> /dev/tasks/FluentValidateTask

bummzack commented 6 years ago

@lerni There's no widget in SS4, as far as I know.

thezenmonkey commented 6 years ago

@tractorcow sorry to Necro this, but I'm having trouble with it again in SS3 with versioned DataObejcts (Elemental). It seems to only effect HTMLText. Both in the CMS and Front End I only see the Unlocalized Values. Checking the Database I see the correct values in the Localized Columns. All other translated fields appear to be behaving correctly.

wernerkrauss commented 6 years ago

@thezenmonkey same here. The problem exists only onPublish, when VersionedDataObject wants to copy the record from Versioned table to Live table. Then fluent again wants to augment some fields and reverts the translation to the original.

this happens to fields that are not in the base class and are lazy loaded. One fix is, to modify VersionedDataObject's publish method like:

    public function publish($fromStage, $toStage, $createNewVersion = false)
    {
            \FluentExtension::set_enable_write_augmentation(false);
        parent::publish($fromStage, $toStage, $createNewVersion);
        $this->owner->extend('onAfterVersionedPublish', $fromStage, $toStage, $createNewVersion);
        \FluentExtension::set_enable_write_augmentation(true);
    }
thezenmonkey commented 6 years ago

@wernerkrauss after some more testing I noticed this is only happening on one field on a DataObject and all is extended DOs. Changing the name of field didn't have an en effect. The issue seams to be with the first HTMLText field DO.

wernerkrauss commented 6 years ago

@thezenmonkey does it write to Stage and not to Live table?

I got it now working with this extension:

<?php
/**
 * Apply this class to each DataObject that has VersionedDataObject and Fluent applied.
 *
 * @property Widget $owner
 */

class FluentVersionedDataObjectExtension extends Extension
{
    public function onBeforeVersionedPublish($fromStage, $toStage, $createNewVersion)
    {
        \FluentExtension::set_enable_write_augmentation(false);
    }

    public function onAfterVersionedPublish($fromStage, $toStage, $createNewVersion)
    {
        \FluentExtension::set_enable_write_augmentation(true);
    }

}

and this config:

Widget:
  extensions:
    - FluentExtension
    - FluentFilteredExtension
    - FluentVersionedDataObjectExtension