Fixing issue where total-records was 0 on POST and PATCH calls.

[nwise]

Closes #307

BUG FIX

• reproduce issue in tests
• fix issue
• bump package version

[jaredcnance]

I feel like this changes the semantics of TotalRecords a bit. For GET requests, TotalRecords indicates the number of records in the database and not the number of records in the response, this would change the meaning of the field for POST and PATCH requests.

Do you find this field useful for create/update requests? I wonder if it should simply be excluded.

[nwise]

Yeah, that makes sense. I'd prefer that, if the total-records value is included, it should be accurate. What do you think about simply not adding it on PATCH/POST responses? I think that would be simplest and also not require an extra call to the DB.

[jaredcnance]

Yeah I 100% agree. I think we should exclude it in those cases.

[milosloub]

I think this is good idea. Property TotalRecords corresponds to amount of returned data in case of filtering. So in case of POST and PATCH is good idea to set TotalRecords : 1. It trully returns only one record.

[jaredcnance]

@milosloub the use case TotalRecords is intended to support is pagination.

corresponds to amount of returned data in case of filtering

This is not entirely accurate. It should return the number of records in the database that meet the specified query before pagination, not the number of results in the response payload. (I recognize that our documentation is a bit lacking here)

I am failing to see a use case for including TotalRecords in POST/PATCH requests. Can you elaborate on how this might be used?

[milosloub]

I'm sorry. I didn't notice

Yeah I 100% agree. I think we should exclude it in those cases.

I think this is better solution to exlude in all irrelevant cases. My suggestion was for "total-record" always presented

[jaredcnance]

I think we can simply make TotalRecords int? and change the following line from:

JsonApiDotNetCore/src/JsonApiDotNetCore/Builders/DocumentBuilder.cs

Lines 78 to 79 in 8f7f0f7

	if (_jsonApiContext.Options.IncludeTotalRecordCount)
	builder.Add("total-records", _jsonApiContext.PageManager.TotalRecords);

to:

if (_jsonApiContext.Options.IncludeTotalRecordCount && _jsonApiContext.PageManager.TotalRecords != null) 
  builder.Add( /* ... */ )

What this implies is that it will only be included if we have explicitly set it earlier in the pipeline.

[jaredcnance]

Hey guys, take a look at the changes and let me know what you think. I've modified it so even if the payload returns empty, we will still include the total-records meta attribute. This is so that a client application that uses paging doesn't have to first check to make sure total-records is actually set.

Example:

GET /api/articles HTTP/1.1
Accept: application/vnd.api+json

HTTP/1.1 200 OK
Content-Type: application/vnd.api+json

{ "data": [], "meta": { "total-records":0 } }

[nwise]

I think this looks good. I like that it's always included on empty results too. It will make coding clients against it simpler. Thanks!