OrderBy sur de Guid

[nfaugout-lucca]

Le nouveau test plante à cause du .ToString()

Si on l'enlève alors le TU passe

Mais l'autre test, celui qui ne passe pas, correspond à un order by sur un Guid (uniqueidentifier en SQL) qui ne fonctionne pas en SQL, et pour cause, un "order by id asc" en SQL sur des GUID ne marche pas, même en faisant une req SQL directe.

D'où la nécessité du .ToString() en LINQ qui va convertir en string en SQL et le order by SQL fonctionne alors.

PS : si on peut corriger facilement tant mieux, sinon on peut mettre un gros warning comme quoi ça sert à rien d'essayer de trier sur des Guid => la classe OrderBy pourrait même planter explicitement si on lui demande de trier sur des Guid, c'est une alternative possible.

[nfaugout-lucca]

@Poltuu tu pourras regarder cette PR qui met en évidence un pb sur le OrderBy sur les Guid ?

Soit on les interdit, soit on corrige le pb (via le .ToString( ) ou autre alternative)

[rducom]

En fait ça n'a pas de sens de faire un order by sur des guids, car leur valeur est random.
Ce qu'il faut c'est lors de la génération d'une clause d'order by par défault, si le type Id est un Guid, alors lever une exception en demandant au dev d'override la méthode d'order by par default pour le type TEntity donné. Ainsi le dev pourra choisir une autre propriété.

[rducom]

Linked : #175

[Poltuu]

je ne vois pas l'intérêt d'interdire de trier par guid, je peux même imaginer des cas ou ça peut être utile.
Par exemple, si une foreign key est de type guid, ce ordre by fait office de groupby sur le retour d'api, et me permet de voir qui est associé à quoi da manière fort pratique (utile pour du debug donc).

Je vais surtout essayer de faire en sorte que le truc fonctionne en natif

[Poltuu]

j'ai fixé:

• la possibilité de rajouter un TOString() dans un orderby
• le orderby natif sur guid ou guid? qui rajoute automatiquement ce .ToString()

[codecov-io]

Codecov Report

Merging #183 into master will increase coverage by 0.2%.
The diff coverage is 93.75%.

@@            Coverage Diff            @@
##           master     #183     +/-   ##
=========================================
+ Coverage   71.65%   71.86%   +0.2%     
=========================================
  Files         113      113             
  Lines        2844     2851      +7     
  Branches      291      292      +1     
=========================================
+ Hits         2038     2049     +11     
+ Misses        682      678      -4     
  Partials      124      124

Impacted Files	Coverage Δ
Web/RDD.Web/Querying/OrderByParser.cs	10.34% <0%> (ø)	⬆️
Domain/RDD.Domain/Models/Querying/OrderBy.cs	100% <100%> (+5.55%)	⬆️
Infra/RDD.Infra/Storage/ReadOnlyRepository.cs	89.23% <0%> (+4.61%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update be94512...a2a04f9. Read the comment docs.

[rducom]

Par exemple, si une foreign key est de type guid, ce ordre by fait office de groupby sur le retour d'api, et me permet de voir qui est associé à quoi da manière fort pratique (utile pour du debug donc).

J'ai pas du tout mais alors pas du tout compris 😄

[Poltuu]

api/users?orderby=managerId,asc

pour voir en gros qui est managé par la même personne. Dans certains cas, c'est très pratique en debug

[nfaugout-lucca]

@Poltuu je te laisse prendre la main sur cette PR