epam / epam/epam-graphql

Get rid of the necessity to override two loader's methods `ApplyNaturalOrderBy` and `ApplyNaturalThenBy`

Open
#5 1 comment 0 reactions 0 assignees View on GitHub
enhancement
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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.