List Leaderboards Endpoint #249
Conversation
| @@ -51,11 +54,12 @@ public record LeaderboardViewModel | |||
| /// <summary> | |||
| /// A collection of `Category` entities for the `Leaderboard`. | |||
| /// </summary> | |||
| public required IList<CategoryViewModel> Categories { get; init; } | |||
| [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] | |||
There was a problem hiding this comment.
With this change though; we'll have to ensure we do the same thing for any future endpoint with possibly-null relations, for consistency. Imo it's fine to default to an empty array, anyway
There was a problem hiding this comment.
All this does is not serialize it when there are no cats to be returned.
There was a problem hiding this comment.
Ergo the field itself won't appear in the return object, right?
There was a problem hiding this comment.
That could be a problem if frontend is expecting it.
I'd ask the frontend people what they would expect here.
Anecdotally the frontend teams I generally work with would expect an empty array. It just helps avoid safe access checks cluttering up the code, and having to write a new type for leaderboards with and without the field. nvm, theres optionals
There was a problem hiding this comment.
The frontend code that actually deals with the models is autogenerated so it'll mark the field as potentially absent. If they try to access it without doing a null check, it'll produce a warning.
There was a problem hiding this comment.
Seems good to me as long as theres a tracking issue for paginating the full list.
Also I noticed that categories are on the view model, if these are returned during the list view can you test the expected behaviour of these returning and being absent? (this may be better in ViewModel tests if those exist; helps boilerplate down the road if there are special exceptions)
Categories are not implemented yet. We're gonna start worrying about them next "sprint" I believe so I'm just ignoring everything to do with them for now. |
This PR essentially splits the get leaderboards endpoint into two. If you provide IDs, it'll return only the Leaderboards with those IDs; otherwise, it'll return all non-deleted Leaderboards. Closes #227