Get rid of the necessity to override two loader's methods `ApplyNaturalOrderBy` and `ApplyNaturalThenBy`
- Dominant language
- C#
- Stars
- 23
- Forks
- 6
- PR merge metrics
- No merged PRs in 30d
Description
### Description
Current approach for ordering query results for connection: developer have to override `ApplyNaturalOrderBy` and `ApplyNaturalThenBy` methods for every loader:
1. These methods are abstract, a developer has to override them even if connection for this loader won't be used.
2. It is error prune because they are overridden very similarly, the only difference is a LINQ `ThenBy` call instead of `OrderBy`:
```c#
public override IOrderedQueryable ApplyNaturalOrderBy(IQueryable query) =>
query.OrderBy(x => x.FullName);
public override IOrderedQueryable ApplyNaturalThenBy(IOrderedQueryable query) =>
query.ThenBy(x => x.FullName);
```
### Describe the solution you'd like
- [x] `Connection<...>()`/`FromLoader<...>(...).AsConnection()` should have overloaded versions with an argument of type `Expression, IOrderedQueryable>>`; `OrderedLoaderField` should use this argument for query ordering (if it is supplied) instead of `ApplyNaturalOrderBy`/`ApplyNaturalThenBy` calls
- [x] If this argument is not supplied, `OrderedLoaderField` should check if `ApplyNaturalOrderBy`/`ApplyNaturalThenBy` returns the same query and throw, otherwise; `OrderedLoaderField` should call `ApplyNaturalOrderBy`/`ApplyNaturalThenBy` for query ordering
- [x] Make `ApplyNaturalOrderBy`/`ApplyNaturalThenBy` virtual, not abstract. Default implementation should return passed query.
- [x] Deprecate `ApplyNaturalOrderBy`/`ApplyNaturalThenBy`
- [ ] Deprecate old versions of `Connection<...>()`/`FromLoader<...>(...).AsConnection()`
- [ ] After final removing of deprecated methods above, for `IdentifiableLoader`s overloaded versions of `Connection`/`AsConnection` should be implemented without arguments which should borrow id expression of that loader for ordering connection.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start by tracing the Connection, FromLoader, and AsConnection entry points, then inspect OrderedLoaderField and the ApplyNaturalOrderBy/ApplyNaturalThenBy methods. Compare the checked solution requirements with the remaining unchecked deprecation and IdentifiableLoader work; done means the overloads, defaults, deprecations, ordering behavior, and final cleanup are addressed.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp
- Domain
- backend-api-design
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 28/100