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

Add support for transOrderBy #516

Merged
merged 9 commits into from
Feb 28, 2020
Merged

Conversation

aurelien-roy
Copy link
Contributor

These changes add a new method enabling models to be sorted by an indexed translatable field.

Use case : Print a list of items sorted by their name where name is a translatable field. Of course, sorting won't be the same from one language to another.

Example :

Product::transOrderBy('name')->get();

If a record hasn't be translated, the value from default locale will be used for sorting.

@LukeTowers
Copy link
Contributor

@mjauvin @osmanzeki @datune could you guys review this please?

@mjauvin once it's been reviewed and is deemed good please let me know so I can do a final review before merging this.

@LukeTowers LukeTowers requested review from bennothommo and w20k October 3, 2019 16:16
@LukeTowers
Copy link
Contributor

@mjauvin is on vacation for the next few weeks so unless someone else reviews it it'll take a few weeks to get to this.

@w20k
Copy link

w20k commented Oct 24, 2019

@aurelien-roy sorry took me a while, but could you add unit test for TranslatableModel.php?

@aurelien-roy
Copy link
Contributor Author

@aurelien-roy sorry took me a while, but could you add unit test for TranslatableModel.php?

Done :)

@mjauvin
Copy link
Contributor

mjauvin commented Oct 30, 2019

@aurelien-roy since you modified scopeTransWhere(), could you also add a unit test for this as well?

@mjauvin mjauvin added this to the 1.6.3 milestone Nov 15, 2019
@mjauvin mjauvin self-assigned this Nov 30, 2019
@mjauvin mjauvin modified the milestones: 1.6.3, 1.7.0 Nov 30, 2019
@mjauvin mjauvin modified the milestones: 1.7.0, 1.6.4 Dec 13, 2019
@mjauvin
Copy link
Contributor

mjauvin commented Jan 10, 2020

@LukeTowers looks good to me!

@daftspunk daftspunk modified the milestones: 1.6.4, 1.6.5 Jan 11, 2020
@LukeTowers
Copy link
Contributor

@mjauvin I don't see anything that stands out to me so if you've tested it and it works well feel free to merge when appropriate

@daftspunk daftspunk modified the milestones: 1.6.5, 1.6.6 Jan 20, 2020
@bennothommo bennothommo modified the milestones: 1.6.7, 1.6.8 Feb 10, 2020
@mjauvin
Copy link
Contributor

mjauvin commented Feb 27, 2020

@LukeTowers @bennothommo I've tested this with the following OctoDock instance and it's working fine as far as I can see:

https://d3f3b84cf746.octodock.com/backend

I also ran the phpunit tests within the instance without issues.

I'm ready to merge this if you have nothing further to add.

@bennothommo
Copy link
Contributor

@mjauvin Looks fine to me. It's new functionality, so its introduction shouldn't break anything.

@mjauvin mjauvin merged commit e88a3f7 into rainlab:master Feb 28, 2020
@aurelien-roy
Copy link
Contributor Author

Thank you all for your time reviewing it!

@@ -33,7 +33,7 @@ public function __construct($model)

/**
* Applies a translatable index to a basic query. This scope will join the index
* table and cannot be executed more than once.
* table and can be executed neither more than once, nor with scopeTransOrder.
Copy link
Contributor

Choose a reason for hiding this comment

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

@mjauvin what does this mean?

Copy link
Contributor

Choose a reason for hiding this comment

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

Indeed... @aurelien-roy can you fix this comment so it makes sense?

mjauvin added a commit that referenced this pull request Mar 20, 2020
* runs params value through htmlspecialchars() to escape html content

* add note about escaped content in translated messages

* use Html::clean() instead of e()

* Revert "add note about escaped content in translated messages"

* escape params when translating messages; introduce transRaw method for legacy usage

* Fix for #376 (#559)

Fix for #376

* Add support for transOrderBy (#516)

Add support for transOrderBy

* update version file for 1.6.8 release

* Disable safe mode checks for ML Static Pages.

Fixes rainlab/pages-plugin#434. Refs: rainlab/pages-plugin#174, rainlab/pages-plugin@6b6b061

* Clear RainLab.Pages caches when saving a static page

Fixes rainlab/pages-plugin#404

* Register asset bundle (#560)

* make sure multi-lingual input form controls have padding-right of 44px
* register asset bundle to process less files into css files
* reposition language selector above textarea box
* fix language selector position when commentAbove is defined

* Update version file for 1.6.9 release

* Fix error with casts fields default locale value (#556)

* only call setLocale() if locale has changed (#561)

* remove unused module

Co-authored-by: Siarhei Karavai <[email protected]>
Co-authored-by: Aurélien Roy <[email protected]>
Co-authored-by: Ben Thomson <[email protected]>
Co-authored-by: Luke Towers <[email protected]>
Co-authored-by: Trysystems <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants