Revert "BP-761: Validation exception when sort by or sort direction are empty in the request."

[msljivic]

Should we revert #57?

Issue

Non-nullable reference fields are marked as Required by default.

Issue was suppressed on projects by setting the SuppressImplicitRequiredAttributeForNonNullableReferenceTypes = true

This suppress was not added to Blueprint template.

Request:

/Users?PageNumber=1&PageSize=20&SortBy=emailAddress&SortDirection=

[PublicAPI]
public class Request : PagedRequest<Response.Item>
{
    public string? Name { get; set; }
}

Error:

{
    "title": "One or more validation errors occurred.",
    "status": 400,
    "traceId": "00-22b69d3187027b178a5caff39b13172c-87bf17a84c5c6f2b-00",
    "errors": {
        "SortDirection": [
            "The SortDirection field is required."
        ]
    }
}

Exception occurs when query params in HTTP request contains empty/missing value sortBy/sortDirection.

Option 1: Make SortBy and SortDirection nullable fields.

Introduced by: #57

Potential breaking changes?

After packages are updated in a project, if SortBy or SortDirection are used directly in code e.g. var isPriceWithVatSort = request.SortBy.Equals("unitPriceWithVat") -> this would lead to compile time warning CS8602 Dereference of a possibly null reference

Option 2: SuppressImplicitRequiredAttributeForNonNullableReferenceTypes = true

Testing behavior of model validation and model binding, example:

[PublicAPI]
public class Request : PagedRequest<Response.Item>
{
    public string Name { get; set; } = String.Empty;
    public string? Email { get; set; } = String.Empty;
}

False (default): Non-nullable reference fields are marked as Required by default.

For request /Users?Name=&PageNumber=1&PageSize=20

public string Name { get; set; } = String.Empty; => Validation error 400: Name is required
public string? Name { get; set; } => OK: Model binding will set Name = null
public string? Name { get; set; } = String.Empty; => OK: Model binding will set Name = null (!?)
public string? Email { get; set; } = String.Empty; => OK: Model binding will set Email = "";

True: This required behavior is suppressed

For request /Users?Name=&PageNumber=1&PageSize=20

public string Name { get; set; } = String.Empty; => OK: Model binding will set Name = null (!!? seems hazardous)
public string? Name { get; set; } => OK: Model binding will set Name = null
public string? Name { get; set; } = String.Empty; => OK: Model binding will set Name = null (!?)
public string? Email { get; set; } = String.Empty; => OK: Model binding will set Email = "";

[vsljivic]

Should we revert #57?

In my view this is not a breaking change and it should not be reverted.
Paging request can come without SortBy and SortDirection and model binding would set these fields to null.
We just made it visible now, i.e. that these fields can be null.

[zsrdjan]

The root of the problem comes from how we construct api call from Angular to the Backend. The MVC erorr is valid. Non Null property is also valid. The problem is in the PagedQuery (https://github.com/enigmatry/entry-angular-building-blocks/pull/173/files). NSWag client does not have this problem since it is correctly constructing url.

E.g.

Please revert this since the alternative fix has been created which does not introduce breaking changes, nor requires changing default MVC setup (i.e. turning off SuppressImplicitRequiredAttributeForNonNullableReferenceTypes).