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

BUG: Localisation option is not applied to relation data #729

Open
mfendeksilverstripe opened this issue Oct 4, 2021 · 2 comments
Open

Comments

@mfendeksilverstripe
Copy link
Contributor

BUG: Localisation option is not applied to relation data

frontend_publish_required / cms_localisation_required can have the following values:

  • any - base record data is acceptable
  • fallback - localisation or fallback is required
  • exact - localisation is required

This configuration is applied to regular data lookups but not data lookups which contain relations.

Generic example

$dataList->filter(['RelationName.SomeField' => 'SomeValue'])

Localisation options of the related model are not applied so the model is treated as it always has any option.

Specific example

BusinessListingPage - localised with any option
SystemDeal - localised with fallback option
BusinessListingPage has many SystemDeal
SystemDeal has one BusinessListingPage

TaxonomyTerm - localised with any option
TravelAgent - localised with exact option
TravelAgent many many TaxonomyTerm
TaxonomyTerm belongs many many TravelAgent

Example 1

Inspecting the data list query shows that the system deal relation doesn't include segments representing fallback option.

$pages = BusinessListingPage::get();
$pages = $pages->filter([
    'SystemDeals.DisplayDateStart:LessThan' => DBDatetime::now()->Rfc2822(),
    'SystemDeals.DisplayDateEnd:GreaterThan' => DBDatetime::now()->Rfc2822(),
]);

Example 2

Inspecting the data list query shows that the system deal relation doesn't include segments representing exact option.

$terms = TaxonomyTerm::get();
$terms = $terms->filter([
    'TravelAgents.OID' => $oid,
]);

Bespoke solution

This solution is applied as extension but ideally this is solved within the Fluent module. We may need some extension points in some of the core modules first though.

<?php

namespace App\ORM\Filters;

use App\Extensions\Locale\LocalisationOrFallbackRequiredFluentExtension;
use App\Extensions\Locale\LocalisationRequiredFluentExtension;
use Exception;
use InvalidArgumentException;
use SilverStripe\Core\Extension;
use SilverStripe\ORM\DataList;
use SilverStripe\ORM\DataObject;
use SilverStripe\ORM\DataObjectSchema;
use SilverStripe\ORM\DataQuery;
use TractorCow\Fluent\Extension\FluentExtension;
use TractorCow\Fluent\Model\Locale;

/**
 * This extension provides temporary fix for localisation of ORM filters
 * This fix has limited scope as proper fix needs to be done on the Fluent module and also some other related modules
 * need to be updated (at least extension points need to be added)
 *
 * Limitation of scope:
 * - available only for filter() method
 * - doesn't support chained relation lookups
 *  (RelationName.Field is supported, RelationName1.RelationName2.Field is not)
 *
 * @property $this|DataList $owner
 */
class FiltersLocalisationExtension extends Extension
{
    /**
     * Apply localisation and use filter() method
     * Same input params as @see DataList::filter()
     *
     * @param mixed ...$arguments
     * @return DataList
     * @throws Exception
     */
    public function localisedFilter(...$arguments): DataList
    {
        // Validate and process arguments
        switch (sizeof($arguments)) {
            case 1:
                $filters = $arguments[0];

                break;
            case 2:
                $filters = [$arguments[0] => $arguments[1]];

                break;
            default:
                throw new InvalidArgumentException('Incorrect number of arguments passed to filter()');
        }

        $list = $this->applyLocalisedFilter($filters);

        return $list->filter($filters);
    }

    /**
     * @param array $filters
     * @return DataList
     * @throws Exception
     */
    private function applyLocalisedFilter(array $filters): DataList
    {
        $list = $this->owner;
        $locale = Locale::getCurrentLocale();

        if ($locale === null) {
            return $list;
        }

        // Keep track of tables we've joined so far, so we avoid redundant query conditions
        $processedTables = [];

        foreach ($filters as $condition => $value) {
            $relatedClass = $this->getRelatedClassFromCondition($list->dataClass(), $condition);

            if ($relatedClass === null) {
                // This is not a relation filter - bail out
                continue;
            }

            /** @var DataObject|FluentExtension $singleton */
            $singleton = DataObject::singleton($relatedClass);

            if (!$singleton->hasExtension(FluentExtension::class)) {
                // Model not localised - bail out as there is no need to localise filters
                continue;
            }

            if (!$singleton->hasExtension(LocalisationRequiredFluentExtension::class)
                && !$singleton->hasExtension(LocalisationOrFallbackRequiredFluentExtension::class)) {
                // Model not strictly localised - bail out as the fallback to base record is acceptable
                continue;
            }

            // Remove any filter modifiers
            $relationField = explode(':', $condition);
            $relationField = array_shift($relationField);

            // Apply the relation, so we can get a relation table alias
            // Note that this does not produce redundant query join segments
            // as relations is internally cached within the core functionality
            $list = $list->applyRelation($relationField, $columnName);

            // Extract the table name which is used to join the relation
            $relationTable = explode('.', $columnName);
            $relationTable = array_shift($relationTable);
            $relationTable = trim($relationTable, '"');

            if (in_array($relationTable, $processedTables)) {
                // We've already handled this relation, so we can bail out
                continue;
            }

            // Mark table as processed
            $processedTables[] = $relationTable;

            $list = $list->alterDataQuery(
                static function (DataQuery $query) use ($singleton, $locale, $relationTable): void {
                    // Determine localised table that we need to join
                    $baseTable = $singleton->baseTable();
                    $localisedTable = $singleton->getLocalisedTable($baseTable);

                    if ($singleton->hasExtension(LocalisationRequiredFluentExtension::class)) {
                        // Localisation required
                        $joinAlias = $singleton->getLocalisedTable($baseTable, $locale->Locale);
                        $conditions = sprintf('"%s"."ID" IS NOT NULL', $joinAlias);
                        $joinCondition = sprintf(
                            '"%1$s"."ID" = "%2$s"."RecordID" AND "%2$s"."Locale" = ?',
                            $relationTable,
                            $joinAlias
                        );

                        $query->leftJoin(
                            $localisedTable,
                            $joinCondition,
                            $joinAlias,
                            20,
                            [$locale->Locale]
                        );

                        $query->where($conditions);
                    } elseif ($singleton->hasExtension(LocalisationOrFallbackRequiredFluentExtension::class)) {
                        // Localisation or fallback required
                        $localeChain = $locale->getChain();
                        $conditions = [];

                        foreach ($localeChain as $joinLocale) {
                            $joinAlias = $singleton->getLocalisedTable($baseTable, $joinLocale->Locale);
                            $conditions[] = sprintf('"%s"."ID" IS NOT NULL', $joinAlias);
                            $joinCondition = sprintf(
                                '"%1$s"."ID" = "%2$s"."RecordID" AND "%2$s"."Locale" = ?',
                                $relationTable,
                                $joinAlias
                            );
                            $query->leftJoin(
                                $localisedTable,
                                $joinCondition,
                                $joinAlias,
                                20,
                                [$joinLocale->Locale]
                            );
                        }

                        $query->whereAny($conditions);
                    }
                }
            );
        }

        return $list;
    }

    /**
     * Determine class of the related model from relation condition
     *
     * @param string $class
     * @param string $condition
     * @return string|null
     */
    private function getRelatedClassFromCondition(string $class, string $condition): ?string
    {
        if (mb_strpos($condition, '.') === false) {
            return null;
        }

        $relation = explode('.', $condition);
        $relation = array_shift($relation);
        $schema = DataObjectSchema::singleton();

        // Check has_one
        $component = $schema->hasOneComponent($class, $relation);

        if ($component) {
            return $component;
        }

        // Check has_many
        $component = $schema->hasManyComponent($class, $relation);

        if ($component) {
            return $component;
        }

        // check belongs_to
        $component = $schema->belongsToComponent($class, $relation);

        if ($component) {
            return $component;
        }

        // Check many_many
        $component = $schema->manyManyComponent($class, $relation);

        if ($component) {
            return $component['childClass'];
        }

        return null;
    }
}

Unit tests

<?php

namespace App\Tests\ORM\Filters;

use App\Extensions\Locale\LocaleDefaultRecordsExtension;
use App\ORM\Filters\FiltersLocalisationExtension;
use App\Pages\BusinessListingPage;
use Exception;
use SilverStripe\Dev\SapphireTest;
use SilverStripe\ORM\DataList;
use SilverStripe\ORM\FieldType\DBDatetime;
use SilverStripe\ORM\ValidationException;
use SilverStripe\Taxonomy\TaxonomyTerm;
use TractorCow\Fluent\State\FluentState;

class FiltersLocalisationExtensionTest extends SapphireTest
{
    /**
     * @var string
     */
    protected static $fixture_file = 'FiltersLocalisationExtensionTest.yml';

    protected function setUp(): void
    {
        FluentState::singleton()->withState(function (FluentState $state): void {
            $state->setLocale(LocaleDefaultRecordsExtension::LOCALE_INTERNATIONAL_ENGLISH);

            parent::setUp();
        });
    }

    /**
     * @param string $writeLocale
     * @param string $targetLocale
     * @param string $now
     * @param int $expected
     * @throws ValidationException
     * @throws Exception
     * @dataProvider localisationOrFallbackRequiredDataProvider
     */
    public function testLocalisationOrFallbackRequired(
        string $writeLocale,
        string $targetLocale,
        string $now,
        int $expected
    ): void {
        DBDatetime::set_mock_now($now);

        // Fetch the model from source locale
        /** @var BusinessListingPage $page */
        $page = FluentState::singleton()->withState(function (FluentState $state): BusinessListingPage {
            $state->setLocale(LocaleDefaultRecordsExtension::LOCALE_INTERNATIONAL_ENGLISH);

            /** @var BusinessListingPage $page */
            $page = $this->objFromFixture(BusinessListingPage::class, 'page1');

            return $page;
        });

        // Localise the model to specified locale
        FluentState::singleton()->withState(static function (FluentState $state) use ($page, $writeLocale): void {
            $state->setLocale($writeLocale);
            $page->write();
        });

        // Assert localised filters
        FluentState::singleton()->withState(function (FluentState $state) use ($page, $targetLocale, $expected): void {
            $state->setLocale($targetLocale);

            /** @var BusinessListingPage $page */
            $page = BusinessListingPage::get()->byID($page->ID);
            $this->assertNotNull($page);

            /** @var DataList|FiltersLocalisationExtension $pages */
            $pages = BusinessListingPage::get();
            $pages = $pages->localisedFilter([
                'SystemDeals.DisplayDateStart:LessThan' => DBDatetime::now()->Rfc2822(),
                'SystemDeals.DisplayDateEnd:GreaterThan' => DBDatetime::now()->Rfc2822(),
            ]);

            $this->assertCount($expected, $pages);
        });
    }

    public function localisationOrFallbackRequiredDataProvider(): array
    {
        return [
            'Related model localised / matches filter criteria' => [
                LocaleDefaultRecordsExtension::LOCALE_INTERNATIONAL_ENGLISH,
                LocaleDefaultRecordsExtension::LOCALE_INTERNATIONAL_ENGLISH,
                '2020-01-01 01:00:00',
                1,
            ],
            'Related model localised / no matches for filter criteria' => [
                LocaleDefaultRecordsExtension::LOCALE_INTERNATIONAL_ENGLISH,
                LocaleDefaultRecordsExtension::LOCALE_INTERNATIONAL_ENGLISH,
                '2020-01-03 01:00:00',
                0,
            ],
            'Related model not localised but with fallback / matches filter criteria' => [
                LocaleDefaultRecordsExtension::LOCALE_INTERNATIONAL_ENGLISH,
                LocaleDefaultRecordsExtension::LOCALE_UNITED_STATES,
                '2020-01-01 01:00:00',
                1,
            ],
            'Related model not localised but with fallback / no matches for filter criteria' => [
                LocaleDefaultRecordsExtension::LOCALE_INTERNATIONAL_ENGLISH,
                LocaleDefaultRecordsExtension::LOCALE_UNITED_STATES,
                '2020-01-03 01:00:00',
                0,
            ],
            'Related model not localised / matches filter criteria' => [
                LocaleDefaultRecordsExtension::LOCALE_JAPAN,
                LocaleDefaultRecordsExtension::LOCALE_JAPAN,
                '2020-01-01 01:00:00',
                0,
            ],
        ];
    }

    /**
     * @param string $writeLocale
     * @param string $targetLocale
     * @param int $oid
     * @param int $expected
     * @throws ValidationException
     * @throws Exception
     * @dataProvider localisationRequiredDataProvider
     */
    public function testLocalisationRequired(string $writeLocale, string $targetLocale, int $oid, int $expected): void
    {
        // Fetch the model from source locale
        /** @var TaxonomyTerm $term */
        $term = FluentState::singleton()->withState(function (FluentState $state): TaxonomyTerm {
            $state->setLocale(LocaleDefaultRecordsExtension::LOCALE_INTERNATIONAL_ENGLISH);

            /** @var TaxonomyTerm $term */
            $term = $this->objFromFixture(TaxonomyTerm::class, 'term1');

            return $term;
        });

        // Localise the model to specified locale
        FluentState::singleton()->withState(static function (FluentState $state) use ($term, $writeLocale): void {
            $state->setLocale($writeLocale);
            $term->write();
        });

        // Assert localised filters
        FluentState::singleton()->withState(
            function (FluentState $state) use ($term, $targetLocale, $oid, $expected): void {
                $state->setLocale($targetLocale);

                /** @var TaxonomyTerm $term */
                $term = TaxonomyTerm::get()->byID($term->ID);
                $this->assertNotNull($term);

                /** @var DataList|FiltersLocalisationExtension $terms */
                $terms = TaxonomyTerm::get();
                $terms = $terms->localisedFilter([
                    'TravelAgents.OID' => $oid,
                ]);

                $this->assertCount($expected, $terms);
            }
        );
    }

    public function localisationRequiredDataProvider(): array
    {
        return [
            'Related model localised / matches filter criteria' => [
                LocaleDefaultRecordsExtension::LOCALE_INTERNATIONAL_ENGLISH,
                LocaleDefaultRecordsExtension::LOCALE_INTERNATIONAL_ENGLISH,
                123,
                1,
            ],
            'Related model localised / no matches for filter criteria' => [
                LocaleDefaultRecordsExtension::LOCALE_INTERNATIONAL_ENGLISH,
                LocaleDefaultRecordsExtension::LOCALE_INTERNATIONAL_ENGLISH,
                124,
                0,
            ],
            'Related model localised but with fallback / matches filter criteria' => [
                LocaleDefaultRecordsExtension::LOCALE_INTERNATIONAL_ENGLISH,
                LocaleDefaultRecordsExtension::LOCALE_UNITED_STATES,
                123,
                0,
            ],
            'Related model not localised / matches filter criteria' => [
                LocaleDefaultRecordsExtension::LOCALE_JAPAN,
                LocaleDefaultRecordsExtension::LOCALE_JAPAN,
                123,
                0,
            ],
            'Related model not localised / no matches for filter criteria' => [
                LocaleDefaultRecordsExtension::LOCALE_JAPAN,
                LocaleDefaultRecordsExtension::LOCALE_JAPAN,
                124,
                0,
            ],
        ];
    }
}
TractorCow\Fluent\Model\Locale:
  en:
    Locale: en_001
    Title: 'International'
    URLSegment: int
  us:
    Locale: en_US
    Title: 'English (US)'
    URLSegment: usa
    Fallbacks:
      - =>TractorCow\Fluent\Model\Locale.en
  jp:
    Locale: ja_JP
    Title: 'Japan'
    URLSegment: jp

App\Models\Deals\SystemDeal:
  deal1:
    TitleLineOne: Deal1
    DisplayDateStart: '2020-01-01 00:00:00'
    DisplayDateEnd: '2020-01-02 00:00:00'

App\Pages\BusinessListingPage:
  page1:
    Title: Page1
    URLSegment: page1
    SystemDeals:
      - =>App\Models\Deals\SystemDeal.deal1

App\Models\TravelAgent:
  travel-agent1:
    FirstName: 'TravelAgent1'
    OID: 123

SilverStripe\Taxonomy\TaxonomyTerm:
  term1:
    Name: 'Term1'
    TravelAgents:
      - =>App\Models\TravelAgent.travel-agent1
@tractorcow
Copy link
Collaborator

Nice solution.

Another way of doing it without an explicit method is to localise all joins. We'd have to detect all joins in a query, then reverse lookup the table to the class, and checking if those classes are localised. :)

It's very hard to do it in such a way that you don't break complex and custom filters / joins though.

To be honest, I don't really have much of the motivation to do the kind of huge refactor that would provide this kind of functionality.

We could just look at dropping in your solution, but instead of the ->hasExtension checks, add more extension points. I think that's what you want right?

You'd probably want to do a bit of work to make sure fallback is respected too.

@mfendeksilverstripe
Copy link
Contributor Author

I completely agree with your assessment 👍 . I'll have a look if I can just integrate this solution without doing any major changes :).

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

2 participants