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

sort() is not working on translated fields #208

Closed
Friksel opened this issue May 29, 2016 · 16 comments
Closed

sort() is not working on translated fields #208

Friksel opened this issue May 29, 2016 · 16 comments

Comments

@Friksel
Copy link

Friksel commented May 29, 2016

Sorting of translated fields is not working:

$this->Tags()->sort('Title', 'ASC');
Does only work on default locale, because sort is ALWAYS sorting Tags on default locale instead of current locale.

$Loc_TitleField = Fluent::db_field_for_locale('Title', $this->CurrentLocale());
$this->Tags()->sort($Loc_TitleField, 'ASC');

Is not working, because Tags that have no textual difference in translation, are null in the database and therefore mess up the sort.

@tractorcow
Copy link
Collaborator

tractorcow commented Jun 6, 2016

Todo: Implement translatable sorting. :D

It'll probably need another line in augmentSQL() to ensure each field is sorted.

A "simple" rewrite could be ORDER BY "Tag"."Title_en_NZ" ASC, "TAG"."Title" ASC, but I guess a more complete solution would need to be a sort on expression with a COALESCE.

@robbieaverill robbieaverill self-assigned this May 18, 2018
@robbieaverill
Copy link
Contributor

Tests to prove this is a bug: https://github.com/tractorcow/silverstripe-fluent/pull/415

Currently passing for the default locale sorting but failing for the non-default (as noted in the original issue)

@tractorcow
Copy link
Collaborator

@robbieaverill
Copy link
Contributor

Hey @tractorcow, I've made a WIP pull request at #415 but think I need your help to get it over the line from here

@tractorcow
Copy link
Collaborator

Thanks I'll try to make some time to look at it soon. :) I also want to see this fixed.

@tractorcow
Copy link
Collaborator

The bug in the test is that ORM doesn't support ->sort(['"somecolumn" ASC', '"anothercolumn" ASC']);. If you read the phpdocs, you're ment to key the array by column and value is the direction.

@tractorcow
Copy link
Collaborator

Updated the PR.

@tractorcow
Copy link
Collaborator

@robbieaverill you can update the core if you want to fix the above.

@tractorcow
Copy link
Collaborator

Fixed with https://github.com/tractorcow/silverstripe-fluent/pull/415

@robbieaverill
Copy link
Contributor

Awesome!

@tractorcow
Copy link
Collaborator

tractorcow commented Jul 17, 2018

@robbieaverill can you follow up on the minor issues I noted on the PR, namely the ->sort(['field asc', 'anotherfield desc']) syntax not working.

We may need to verify #257 doesn't still exist in new fluent.

@robbieaverill
Copy link
Contributor

It's on the todo list =)

@sunnysideup
Copy link

sunnysideup commented Jul 19, 2018

We are still seeing this issue making a simple lookup for a gridfield fail:

SELECT DISTINCT "SiteTree"."ClassName", "SiteTree"."LastEdited", "SiteTree"."Created", "SiteTree"."URLSegment", CASE
	WHEN "SiteTree_Localised_en_NZ"."ID" IS NOT NULL THEN "SiteTree_Localised_en_NZ"."Title"
	ELSE "SiteTree"."Title" END
 AS "Title", CASE
	WHEN "SiteTree_Localised_en_NZ"."ID" IS NOT NULL THEN "SiteTree_Localised_en_NZ"."MenuTitle"
	ELSE "SiteTree"."MenuTitle" END
 AS "MenuTitle", CASE
	WHEN "SiteTree_Localised_en_NZ"."ID" IS NOT NULL THEN "SiteTree_Localised_en_NZ"."Content"
	ELSE "SiteTree"."Content" END
 AS "Content", CASE
	WHEN "SiteTree_Localised_en_NZ"."ID" IS NOT NULL THEN "SiteTree_Localised_en_NZ"."MetaDescription"
	ELSE "SiteTree"."MetaDescription" END
 AS "MetaDescription", "SiteTree"."ExtraMeta", "SiteTree"."ShowInMenus", "SiteTree"."ShowInSearch", "SiteTree"."Sort", "SiteTree"."HasBrokenFile", "SiteTree"."HasBrokenLink", CASE
	WHEN "SiteTree_Localised_en_NZ"."ID" IS NOT NULL THEN "SiteTree_Localised_en_NZ"."ReportClass"
	ELSE "SiteTree"."ReportClass" END
 AS "ReportClass", "SiteTree"."Version", "SiteTree"."CanViewType", "SiteTree"."CanEditType", "SiteTree"."ShowPageUtilities", "SiteTree"."ContentReviewType", "SiteTree"."ReviewPeriodDays", "SiteTree"."NextReviewDate", CASE
	WHEN "SiteTree_Localised_en_NZ"."ID" IS NOT NULL THEN "SiteTree_Localised_en_NZ"."LastEditedByName"
	ELSE "SiteTree"."LastEditedByName" END
 AS "LastEditedByName", CASE
	WHEN "SiteTree_Localised_en_NZ"."ID" IS NOT NULL THEN "SiteTree_Localised_en_NZ"."OwnerNames"
	ELSE "SiteTree"."OwnerNames" END
 AS "OwnerNames", CASE
	WHEN "SiteTree_Localised_en_NZ"."ID" IS NOT NULL THEN "SiteTree_Localised_en_NZ"."ShareTokenSalt"
	ELSE "SiteTree"."ShareTokenSalt" END
 AS "ShareTokenSalt", "SiteTree"."ParentID", "SiteTree"."WorkflowDefinitionID", CASE
	WHEN "Page_Localised_en_NZ"."ID" IS NOT NULL THEN "Page_Localised_en_NZ"."Intro"
	ELSE "Page"."Intro" END
 AS "Intro", "Page"."ShowInFooterMenu", "Page"."MainLinkID", "Page"."SecondaryLink1ID", "Page"."SecondaryLink2ID", "Page"."VideoHolderID", "SiteTree"."ID", 
			CASE WHEN "SiteTree"."ClassName" IS NOT NULL THEN "SiteTree"."ClassName"
			ELSE 'SilverStripe\\CMS\\Model\\SiteTree' END AS "RecordClassName", 'en_NZ' AS "Locale", CASE 	WHEN "Page_Localised_en_NZ"."ID" IS NOT NULL THEN 'en_NZ' 
ELSE NULL END AS "SourceLocale"

FROM "SiteTree"
LEFT JOIN "Page" ON "Page"."ID" = "SiteTree"."ID"
LEFT JOIN "SiteTree_Localised" AS "SiteTree_Localised_en_NZ" ON "SiteTree"."ID" = "SiteTree_Localised_en_NZ"."RecordID" AND "SiteTree_Localised_en_NZ"."Locale" = ?
LEFT JOIN "Page_Localised" AS "Page_Localised_en_NZ" ON "Page"."ID" = "Page_Localised_en_NZ"."RecordID" AND "Page_Localised_en_NZ"."Locale" = ?

WHERE ("SiteTree"."ID" NOT IN 
SELECT DISTINCT "SiteTree"."ID"

FROM "SiteTree"
LEFT JOIN "Page" ON "Page"."ID" = "SiteTree"."ID"
INNER JOIN "HouseCountTerm_DoNotAnnotateOn" ON "HouseCountTerm_DoNotAnnotateOn"."PageID" = "SiteTree"."ID"
LEFT JOIN "SiteTree_Localised" AS "SiteTree_Localised_en_NZ" ON "SiteTree"."ID" = "SiteTree_Localised_en_NZ"."RecordID" AND "SiteTree_Localised_en_NZ"."Locale" = ?
LEFT JOIN "Page_Localised" AS "Page_Localised_en_NZ" ON "Page"."ID" = "Page_Localised_en_NZ"."RecordID" AND "Page_Localised_en_NZ"."Locale" = ?

WHERE ("HouseCountTerm_DoNotAnnotateOn"."HouseCountTermID" = ?)
 AND ("SiteTree"."ClassName" IN (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?))))
 AND ((COALESCE("SiteTree_Localised_en_NZ"."Title", "SiteTree"."Title") LIKE ?)
 OR (COALESCE("SiteTree_Localised_en_NZ"."Content", "SiteTree"."Content") LIKE ?))
 AND ("SiteTree"."ClassName" IN (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?))

ORDER BY "SiteTree"."Title" ASC

It is sorted by SiteTree.Title, but this is not in the select.

Does anyone know a workaround?

@sunnysideup
Copy link

sunnysideup commented Jul 19, 2018

HACK:

SilverStripe\ORM\DB::query("SET SESSION sql_mode='REAL_AS_FLOAT,PIPES_AS_CONCAT,ANSI_QUOTES,IGNORE_SPACE';");

ansi minus the group thingy

@lerni
Copy link

lerni commented Aug 30, 2018

If I got it right, this issue is solved in 4 but not in 3.x. Is a backport planed?

There are workarounds for 3.x like https://github.com/SpliffSplendor/silverstripe-mysqlfixer https://github.com/lerni/silverstripe3-mysql57-fluent

@sminnee thinks it should be fixed in 3.x as well. Would this be a separate issue or reopening this one?

@sminnee
Copy link

sminnee commented Sep 3, 2018

The support for 3.x is much more limited these days so if "upgrade to SS4" is a fix to this issue, that's probably the one I'd recommend ;-)

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

7 participants