Added fields as query parameter #199
Conversation
|
Also I have no idea why that one test failed. It failed about 2 times on my machine as well but now it is working fine. |
| /// <inheritdoc/> | ||
| public override string ToString() | ||
| { | ||
| return $"filter[{Type}]={string.Join(",", Fields)}"; |
There was a problem hiding this comment.
should say fields instead of filter?
There was a problem hiding this comment.
Fixed in 79c9a86
Tests/JsonApiSerializerTests.cs
Outdated
| @@ -68,6 +68,22 @@ public void ConditionallyAppliesFilters() | |||
|
|
|||
| } | |||
|
|
|||
| [Fact(DisplayName = "Uses query fieldset expressions if specified")] | |||
| public void UsesQueryFieldsetExpressions() | |||
There was a problem hiding this comment.
it looks like tabs and spaces got mixed in this file?
There was a problem hiding this comment.
Having some trouble detecting those on my machine I hope they got fixed with a0ee858
Tests/JsonApiSerializerTests.cs
Outdated
| @@ -68,6 +68,22 @@ public void ConditionallyAppliesFilters() | |||
|
|
|||
| } | |||
|
|
|||
| [Fact(DisplayName = "Uses query fieldset expressions if specified")] | |||
There was a problem hiding this comment.
I think we need a test where we ask for a field that's not on the model (should get nothing in that case).
There was a problem hiding this comment.
Added the test in a0ee858. I think only the attributes should be empty as id and type field are still returned which I think would be correct behaviour at this point.
|
I guess the pre-release is held up by the failing test? Should I take a look at that as well? ~Cheers |
|
@PhyberApex you're right; but it's nothing to do with your PR. I'll fix it and publish the release. Thanks! |
|
Hey @joukevandermaas sorry to bother you again. Just realized that there does not seem to be a release yet. Any estimate on this one? Would be greatly appriciated! Thanks! ~Cheers Edit: |
|
@PhyberApex it is indeed a beta release. The betas are usually pretty stable; in this specific case I think it only contains the changes from this PR (compared to stable). Before creating a stable release I'll need to (at least) add some docs for this feature. |
|
If you need any help with that from me be sure to let me know. Would gladly take over any work related to this. Also thanks for always being so responsive, that's not a given! ~Cheers |
This is really a first throw of me to check if you are okay with this or if you have anything against it. I also fixed all tests and added one more test. As I am not that fluent in C# and WebAPI tho I can't really tell if this is enough.
Be sure to get back to me with any comments you have and what you need changed to make a PR possible.
~Cheers