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

Conversation

GuySartorelli
Copy link
Member

Do not squash!

There are three distinct commits which all do different things towards making this module CMS 5 compatible.

Parent issue

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()

@@ -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

Comment on lines -77 to +79
$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());
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.

@@ -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

Comment on lines -434 to +439
->first();
->record();
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()

@@ -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.

Copy link
Member

@emteknetnz emteknetnz left a comment

Choose a reason for hiding this comment

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

Smoke tested locally

@emteknetnz emteknetnz merged commit 043f149 into silverstripe:7 Jan 31, 2023
@emteknetnz emteknetnz deleted the pulls/7/cms5-compat branch January 31, 2023 22:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants