Skip to content
This repository has been archived by the owner on Feb 21, 2023. It is now read-only.

Make fluent CMS 5 compatible #3

Merged
merged 3 commits into from
Jan 31, 2023
Merged

Make fluent CMS 5 compatible #3

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
155ef59
DEP Upgrade frontend build stack
GuySartorelli Jan 26, 2023
759c49e
FIX Don't use removed API
GuySartorelli Jan 26, 2023
5ab1826
MNT Fix broken unit tests
GuySartorelli Jan 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .nvmrc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
10
18
6 changes: 6 additions & 0 deletions babel.config.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"presets": [
"@babel/preset-env",
"@babel/preset-react"
]
}
2 changes: 1 addition & 1 deletion client/dist/js/fluent.js
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion client/dist/styles/fluent.css
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

23 changes: 15 additions & 8 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
{
"name": "tractorcow-fluent",
"version": "4.0.0",
"description": "Fluent localisation module for SilverStrip CMS",
"engines": {
"node": "^10.x"
"node": "^18.x"
},
"scripts": {
"build": "NODE_ENV=production webpack -p --bail --progress",
"build": "yarn && yarn lint && rm -rf client/dist/* && NODE_ENV=production webpack --mode production --bail --progress",
"dev": "NODE_ENV=development webpack --progress",
"watch": "NODE_ENV=development webpack --watch --progress",
"css": "WEBPACK_CHILD=css npm run build",
"lint": "eslint client/src && sass-lint -v"
Expand All @@ -27,12 +27,19 @@
},
"homepage": "https://github.com/tractorcow-farm/silverstripe-fluent",
"devDependencies": {
"@silverstripe/eslint-config": "^0.0.5",
"@silverstripe/webpack-config": "^0.4.1"
"@silverstripe/eslint-config": "^1.0.0-alpha6",
"@silverstripe/webpack-config": "^2.0.0-alpha6",
"webpack": "^5.74.0",
"webpack-cli": "^5.0.0"
},
"dependencies": {
"jquery": "^3.5.0",
"query-string": "^5.0.0",
"query-string": "^8.1.0",
"url": "^0.11.0"
}
},
"resolutions": {
"colors": "1.4.0"
},
"browserslist": [
"defaults"
]
}
2 changes: 1 addition & 1 deletion src/Extension/FluentVersionedExtension.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -949,7 +949,7 @@ protected function getCurrentVersionNumbers(string $class, string $stage, ?array
$results = DB::prepared_query($query, $params);
$versions = [];

while ($result = $results->next()) {
foreach ($results as $result) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixes this error:

Error: Call to undefined method SilverStripe\ORM\Connect\MySQLStatement::next()

$id = (int) $result['LatestID'];
$version = (int) $result['LatestVersion'];

Expand Down
15 changes: 10 additions & 5 deletions tests/php/Extension/FluentExtensionTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ public function testLocalisedMixSorting()

// Sort by the NonLocalisedSort field first then the LocalisedField second both in ascending order
// so the result will be opposite if the order of the columns is not maintained
$objects=MixedLocalisedSortObject::get()->sort(
$objects=MixedLocalisedSortObject::get()->orderBy(
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixes errors trying to run sql through sort() - a new method orderBy() was introduced to handle sorting data via raw SQL

'"FluentExtensionTest_MixedLocalisedSortObject"."LocalizedSort", '.
'"FluentExtensionTest_MixedLocalisedSortObject"."NonLocalizedSort", '.
'"FluentExtensionTest_MixedLocalisedSortObject"."Title"'
Expand Down Expand Up @@ -289,12 +289,16 @@ public function testLocalisedCopyClassNameChange(): void
* @param string[] $expected
* @group exclude-from-travis
*/
public function testLocalisedFieldsCanBeSorted($locale, array $sortArgs, $expected)
public function testLocalisedFieldsCanBeSorted($locale, array $sortArgs, $expected, $useOrderBy = false)
{
FluentState::singleton()->withState(function (FluentState $newState) use ($locale, $sortArgs, $expected) {
FluentState::singleton()->withState(function (FluentState $newState) use ($locale, $sortArgs, $expected, $useOrderBy) {
$newState->setLocale($locale);

$records = LocalisedParent::get()->sort(...$sortArgs);
if ($useOrderBy) {
$records = LocalisedParent::get()->orderBy(...$sortArgs);
} else {
$records = LocalisedParent::get()->sort(...$sortArgs);
}
$titles = $records->column('Title');
$this->assertEquals($expected, $titles);
});
Expand Down Expand Up @@ -411,6 +415,7 @@ public function sortRecordProvider()
'en_US',
['CONCAT((SELECT COUNT(*) FROM "FluentExtensionTest_LocalisedParent_Localised"), "FluentExtensionTest_LocalisedParent"."ID")'],
['A record', 'Read about things', 'Go for a run'],
true,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one test case needs to be run with orderBy() because it's using raw SQL

]
];
}
Expand All @@ -431,7 +436,7 @@ protected function hasLocalisedRecord(DataObject $record, $locale)
'"Locale"' => $locale,
])
->execute()
->first();
->record();
Comment on lines -434 to +439
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixes this error:

Error: Call to undefined method SilverStripe\ORM\Connect\MySQLStatement::first()


return !empty($result);
}
Expand Down
7 changes: 4 additions & 3 deletions tests/php/Extension/FluentSiteTreeExtensionTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use Page;
use SilverStripe\CMS\Forms\SiteTreeURLSegmentField;
use SilverStripe\CMS\Model\SiteTree;
use SilverStripe\Control\Controller;
use SilverStripe\Control\Director;
use SilverStripe\Core\Config\Config;
use SilverStripe\Dev\SapphireTest;
Expand Down Expand Up @@ -74,8 +75,8 @@ public function testGetLocaleInformation()
$this->assertEquals('English (New Zealand)', $result->getTitle());
$this->assertEquals('English', $result->getLanguageNative());
$this->assertEquals('en', $result->getLanguage());
$this->assertEquals('/newzealand/a-page/', $result->getLink());
$this->assertEquals('http://mocked/newzealand/a-page/', $result->getAbsoluteLink());
$this->assertEquals(Controller::normaliseTrailingSlash('/newzealand/a-page/'), $result->getLink());
$this->assertEquals(Controller::normaliseTrailingSlash('http://mocked/newzealand/a-page/'), $result->getAbsoluteLink());
Comment on lines -77 to +79
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixes failures which result from expecting a trailing slash when the default configuration is now to not include one.

Doing it this way instead of just removing the trailing slash means the test is robust against this change. The test should be that the URL is correct and should not explicitly care if there's a trailing slash or not.

$this->assertEquals('link', $result->getLinkingMode());
$this->assertEquals('newzealand', $result->getURLSegment());
});
Expand Down Expand Up @@ -179,7 +180,7 @@ function (FluentState $newState) use ($domain, $locale, $prefixDisabled, $pageNa

/** @var Page|FluentSiteTreeExtension $page */
$page = $this->objFromFixture(Page::class, $pageName);
$this->assertEquals($url, $page->Link());
$this->assertEquals(Controller::normaliseTrailingSlash($url), $page->Link());
}
);
}
Expand Down
13 changes: 7 additions & 6 deletions tests/php/Model/LocaleTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace TractorCow\Fluent\Tests\Model;

use SilverStripe\Control\Controller;
use SilverStripe\Core\Config\Config;
use SilverStripe\Dev\SapphireTest;
use SilverStripe\Forms\CheckboxField;
Expand Down Expand Up @@ -101,25 +102,25 @@ public function testGetBaseURLContainsDomainAndURLSegmentForNonDefaultLocale()
// es_ES has a domain but is not the default locale for that domain
$result = Locale::getByLocale('es_ES')->getBaseURL();
$this->assertStringContainsString('fluent.es', $result, "Locale's domain is in the URL");
$this->assertStringContainsString('/es/', $result, 'URL segment for non-default locale is in the URL');
$this->assertStringEndsWith(Controller::normaliseTrailingSlash('/es/'), $result, 'URL segment for non-default locale is in the URL');
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use endsWith instead of contains - it shouldn't just be arbitrarily somewhere in the url, it should be specifically at the end.


// Turning off domain mode removes domain but not prefix
FluentState::singleton()->setIsDomainMode(false);
$result = Locale::getByLocale('es_ES')->getBaseURL();
$this->assertStringNotContainsString('fluent.es', $result, "Locale's domain is in the URL");
$this->assertStringContainsString('/es/', $result, 'URL segment for non-default locale is in the URL');
$this->assertStringEndsWith(Controller::normaliseTrailingSlash('/es/'), $result, 'URL segment for non-default locale is in the URL');
}

public function testBaseURLPrefixDisabled()
{
// Default base url includes the default url segment
$result = Locale::getDefault()->getBaseURL();
$this->assertStringContainsString('/au/', $result);
$this->assertStringEndsWith(Controller::normaliseTrailingSlash('/au/'), $result);

// Default base url shortens the default locale url base by excluding the locale's url segment
Config::inst()->set(FluentDirectorExtension::class, 'disable_default_prefix', true);
$result = Locale::getDefault()->getBaseURL();
$this->assertStringNotContainsString('/au/', $result);
$this->assertStringEndsNotWith(Controller::normaliseTrailingSlash('/au/'), $result);
}

public function testGetBaseURLOnlyContainsDomainForPrefixDisabledDefaultLocale()
Expand All @@ -129,13 +130,13 @@ public function testGetBaseURLOnlyContainsDomainForPrefixDisabledDefaultLocale()
// es_US has a domain and is the default
$result = Locale::getByLocale('es_US')->getBaseURL();
$this->assertStringContainsString('fluent.es', $result, "Locale's domain is in the URL");
$this->assertStringNotContainsString('/es-usa/', $result, 'URL segment is not in the URL for default locales');
$this->assertStringEndsNotWith(Controller::normaliseTrailingSlash('/es-usa/'), $result, 'URL segment is not in the URL for default locales');

// When domain mode is turned off, prefix is now necessary
FluentState::singleton()->setIsDomainMode(false);
$result = Locale::getByLocale('es_US')->getBaseURL();
$this->assertStringNotContainsString('fluent.es', $result, "Domain not used");
$this->assertStringContainsString('/es-usa/', $result, 'URL Segment necessary for non-global default');
$this->assertStringEndsWith(Controller::normaliseTrailingSlash('/es-usa/'), $result, 'URL Segment necessary for non-global default');
}

public function testGetSiblings()
Expand Down
Loading