Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(api): Add sortOrder to API_GetComments #3156

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

feat(api): Add sortOrder to API_GetComments #3156

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b4ccd46
Allow Submitted as fillable in Comment model
ioslife Jan 30, 2025
efcbc10
Add tests for Comment sortOrder param
ioslife Jan 30, 2025
81a3b27
Add sortOrder query param for Comments
ioslife Jan 30, 2025
cf537ce
Merge branch 'master' into api/get-comments-descending
ioslife Jan 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions app/Models/Comment.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ class Comment extends BaseModel
'ArticleID',
'Payload',
'user_id',
'Submitted',
];

protected static function newFactory(): CommentFactory
Expand Down
14 changes: 11 additions & 3 deletions public/API/API_GetComments.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
* t : 1 = game, 2 = achievement, 3 = user
* o : offset - number of entries to skip (default: 0)
* c : count - number of entries to return (default: 100, max: 500)
* s : sortOrder - sort comments. 0 = ascending, 1 = descending (default: 0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be too ambiguous in terms of what the orderBy() is. I'm also thinking integers as possible values may be inflexible if this pattern is extended to other API endpoints.

I think we may want to change this to use JSON:API-like enum values, ie: createdAt and -createdAt.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do have a few public APIs where filter parameters are effectively booleans (0=all,1=filtered), and some that are direct mapping of enums (3=core,5=unofficial).

But as this is not an enum, and it's not being implemented as a boolean (s: sort descending (0=no,1=yes)), I agree that it makes sense to have it be more free-form.

*
* int Count number of comment records returned in the response
* int Total number of comment records the game/achievement/user actually has overall
Expand Down Expand Up @@ -43,12 +44,14 @@
],
'o' => ['sometimes', 'integer', 'min:0', 'nullable'],
'c' => ['sometimes', 'integer', 'min:1', 'max:500', 'nullable'],
's' => ['sometimes', 'integer', 'min:0', 'max:1', 'nullable'],
];

$input = Validator::validate(Arr::wrap($query), $rules);

$offset = $input['o'] ?? 0;
$count = $input['c'] ?? 100;
$sortOrder = $input['s'] ?? 0;

$username = null;
$gameOrAchievementId = 0;
Expand All @@ -75,7 +78,7 @@

$articleId = $user ? $user->ID : $gameOrAchievementId;

$comments = Comment::withTrashed()
$commentsQuery = Comment::withTrashed()
->with('user')
->where('ArticleType', $commentType)
->where('ArticleID', $articleId)
Expand All @@ -84,8 +87,13 @@
$query->whereNull('banned_at');
})
->offset($offset)
->limit($count)
->get();
->limit($count);

if ($sortOrder == 1) {
$commentsQuery->orderBy('Submitted', 'DESC');
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll probably want an explicit orderBy for ascending to. It's unsafe to assume that an unordered query returns sorted data.


$comments = $commentsQuery->get();

$totalComments = Comment::withTrashed()
->where('ArticleType', $commentType)
Expand Down
61 changes: 61 additions & 0 deletions tests/Feature/Api/V1/CommentsTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,12 @@ public function testItValidates(): void
$this->get($this->apiUrl('GetComments', ['i' => 'not-an-integer', 't' => 1]))
->assertJsonValidationErrors(['i']);

$this->get($this->apiUrl('GetComments', ['i' => 1, 't' => 1, 's' => 'not-an-integer']))
->assertJsonValidationErrors(['s']);

$this->get($this->apiUrl('GetComments', ['i' => 1, 't' => 1, 's' => 3]))
->assertJsonValidationErrors(['s']);

$this->get($this->apiUrl('GetComments', ['i' => null, 't' => 2]))
->assertJsonValidationErrors(['i']);
}
Expand Down Expand Up @@ -100,6 +106,61 @@ public function testGetCommentsForAchievement(): void
]);
}

public function testGetCommentsForAchievementDescendingOrder(): void
{
// Arrange
$system = System::factory()->create();
$game = Game::factory()->create(['ConsoleID' => $system->ID]);
$user1 = User::factory()->create();
$user2 = User::factory()->create();
$bannedUser = User::factory()->create(['ID' => 309, 'banned_at' => Carbon::now()->subDay()]);

$achievement = Achievement::factory()->create(['GameID' => $game->ID, 'user_id' => $user1->ID]);
$comment1 = Comment::factory()->create([
'ArticleID' => $achievement->ID,
'ArticleType' => 2,
'user_id' => $user1->ID,
'Payload' => 'This is a great achievement!',
'Submitted' => "2024-01-18T15:01:04.000000Z",
]);
$comment2 = Comment::factory()->create([
'ArticleID' => $achievement->ID,
'ArticleType' => 2,
'user_id' => $user2->ID,
'Payload' => 'I agree, this is awesome!',
'Submitted' => "2024-12-18T15:01:04.000000Z",
]);
$comment3 = Comment::factory()->create([
'ArticleID' => $achievement->ID,
'ArticleType' => 2,
'user_id' => $bannedUser->ID,
'Payload' => 'This comment is from a banned user!',
]);

// Act
$response = $this->get($this->apiUrl('GetComments', ['i' => $achievement->ID, 't' => 2, 's' => 1]))
->assertSuccessful();

// Assert
$response->assertStatus(200);
$response->assertJson([
'Count' => 2,
'Total' => 2,
'Results' => [
[
'User' => $user2->User,
'Submitted' => $comment2->Submitted->toISOString(),
'CommentText' => $comment2->Payload,
],
[
'User' => $user1->User,
'Submitted' => $comment1->Submitted->toISOString(),
'CommentText' => $comment1->Payload,
],
],
]);
}

public function testGetCommentsForGame(): void
{
// Arrange
Expand Down