mirror of
https://github.com/solidtime-io/solidtime.git
synced 2026-08-08 00:02:15 +01:00
add unique id tiebreaker to paginated index endpoints to make pagination
stable (#1138)
This commit is contained in:
@@ -43,7 +43,8 @@ class ClientController extends Controller
|
|||||||
|
|
||||||
$clientsQuery = Client::query()
|
$clientsQuery = Client::query()
|
||||||
->whereBelongsTo($organization, 'organization')
|
->whereBelongsTo($organization, 'organization')
|
||||||
->orderBy('created_at', 'desc');
|
->orderBy('created_at', 'desc')
|
||||||
|
->orderBy('id');
|
||||||
|
|
||||||
if (! $canViewAllClients) {
|
if (! $canViewAllClients) {
|
||||||
$clientsQuery->visibleByEmployee($user);
|
$clientsQuery->visibleByEmployee($user);
|
||||||
|
|||||||
@@ -42,6 +42,7 @@ class InvitationController extends Controller
|
|||||||
|
|
||||||
$invitations = $organization->organizationInvitations()
|
$invitations = $organization->organizationInvitations()
|
||||||
->orderBy('created_at', 'desc')
|
->orderBy('created_at', 'desc')
|
||||||
|
->orderBy('id')
|
||||||
->paginate(config('app.pagination_per_page_default'));
|
->paginate(config('app.pagination_per_page_default'));
|
||||||
|
|
||||||
return InvitationCollection::make($invitations);
|
return InvitationCollection::make($invitations);
|
||||||
|
|||||||
@@ -61,6 +61,7 @@ class MemberController extends Controller
|
|||||||
->whereBelongsTo($organization, 'organization')
|
->whereBelongsTo($organization, 'organization')
|
||||||
->with(['user'])
|
->with(['user'])
|
||||||
->orderBy('created_at', 'desc')
|
->orderBy('created_at', 'desc')
|
||||||
|
->orderBy('id')
|
||||||
->paginate(config('app.pagination_per_page_default'));
|
->paginate(config('app.pagination_per_page_default'));
|
||||||
|
|
||||||
return MemberCollection::make($members);
|
return MemberCollection::make($members);
|
||||||
|
|||||||
@@ -62,6 +62,7 @@ class ProjectController extends Controller
|
|||||||
|
|
||||||
$projects = $projectsQuery
|
$projects = $projectsQuery
|
||||||
->orderBy('created_at', 'desc')
|
->orderBy('created_at', 'desc')
|
||||||
|
->orderBy('id')
|
||||||
->paginate(config('app.pagination_per_page_default'));
|
->paginate(config('app.pagination_per_page_default'));
|
||||||
|
|
||||||
$showBillableRate = $this->member($organization)->role !== Role::Employee->value || $organization->employees_can_see_billable_rates;
|
$showBillableRate = $this->member($organization)->role !== Role::Employee->value || $organization->employees_can_see_billable_rates;
|
||||||
|
|||||||
@@ -49,6 +49,7 @@ class ProjectMemberController extends Controller
|
|||||||
$projectMembers = ProjectMember::query()
|
$projectMembers = ProjectMember::query()
|
||||||
->whereBelongsTo($project, 'project')
|
->whereBelongsTo($project, 'project')
|
||||||
->orderBy('created_at', 'desc')
|
->orderBy('created_at', 'desc')
|
||||||
|
->orderBy('id')
|
||||||
->paginate(config('app.pagination_per_page_default'));
|
->paginate(config('app.pagination_per_page_default'));
|
||||||
|
|
||||||
return new ProjectMemberCollection($projectMembers);
|
return new ProjectMemberCollection($projectMembers);
|
||||||
|
|||||||
@@ -47,6 +47,7 @@ class ReportController extends Controller
|
|||||||
|
|
||||||
$reports = Report::query()
|
$reports = Report::query()
|
||||||
->orderBy('created_at', 'desc')
|
->orderBy('created_at', 'desc')
|
||||||
|
->orderBy('id')
|
||||||
->whereBelongsTo($organization, 'organization')
|
->whereBelongsTo($organization, 'organization')
|
||||||
->paginate(config('app.pagination_per_page_default'));
|
->paginate(config('app.pagination_per_page_default'));
|
||||||
|
|
||||||
|
|||||||
@@ -42,6 +42,7 @@ class TagController extends Controller
|
|||||||
$tags = Tag::query()
|
$tags = Tag::query()
|
||||||
->whereBelongsTo($organization, 'organization')
|
->whereBelongsTo($organization, 'organization')
|
||||||
->orderBy('created_at', 'desc')
|
->orderBy('created_at', 'desc')
|
||||||
|
->orderBy('id')
|
||||||
->paginate(config('app.pagination_per_page_default'));
|
->paginate(config('app.pagination_per_page_default'));
|
||||||
|
|
||||||
return new TagCollection($tags);
|
return new TagCollection($tags);
|
||||||
|
|||||||
@@ -84,6 +84,7 @@ class TaskController extends Controller
|
|||||||
|
|
||||||
$tasks = $query
|
$tasks = $query
|
||||||
->orderBy('created_at', 'desc')
|
->orderBy('created_at', 'desc')
|
||||||
|
->orderBy('id')
|
||||||
->paginate(config('app.pagination_per_page_default'));
|
->paginate(config('app.pagination_per_page_default'));
|
||||||
|
|
||||||
return new TaskCollection($tasks);
|
return new TaskCollection($tasks);
|
||||||
|
|||||||
@@ -194,7 +194,8 @@ class TimeEntryController extends Controller
|
|||||||
$timeEntriesQuery = TimeEntry::query()
|
$timeEntriesQuery = TimeEntry::query()
|
||||||
->whereBelongsTo($organization, 'organization')
|
->whereBelongsTo($organization, 'organization')
|
||||||
->select($select)
|
->select($select)
|
||||||
->orderBy('start', 'desc');
|
->orderBy('time_entries.start', 'desc')
|
||||||
|
->orderBy('time_entries.id');
|
||||||
|
|
||||||
$filter = new TimeEntryFilter($timeEntriesQuery);
|
$filter = new TimeEntryFilter($timeEntriesQuery);
|
||||||
$filter->addStartFilter($request->input('start'));
|
$filter->addStartFilter($request->input('start'));
|
||||||
|
|||||||
@@ -13,6 +13,8 @@ use App\Models\ProjectMember;
|
|||||||
use App\Models\Task;
|
use App\Models\Task;
|
||||||
use App\Models\TimeEntry;
|
use App\Models\TimeEntry;
|
||||||
use App\Service\BillableRateService;
|
use App\Service\BillableRateService;
|
||||||
|
use Illuminate\Support\Facades\DB;
|
||||||
|
use Illuminate\Support\Str;
|
||||||
use Illuminate\Testing\Fluent\AssertableJson;
|
use Illuminate\Testing\Fluent\AssertableJson;
|
||||||
use Laravel\Passport\Passport;
|
use Laravel\Passport\Passport;
|
||||||
use Mockery\MockInterface;
|
use Mockery\MockInterface;
|
||||||
@@ -81,6 +83,49 @@ class ProjectEndpointTest extends ApiEndpointTestAbstract
|
|||||||
$this->assertSame([$projectNewest->getKey(), $projectMiddle->getKey(), $projectOldest->getKey()], $ids);
|
$this->assertSame([$projectNewest->getKey(), $projectMiddle->getKey(), $projectOldest->getKey()], $ids);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function test_index_endpoint_pagination_returns_every_project_exactly_once_when_they_share_created_at(): void
|
||||||
|
{
|
||||||
|
// Arrange
|
||||||
|
$data = $this->createUserWithPermission([
|
||||||
|
'projects:view',
|
||||||
|
'projects:view:all',
|
||||||
|
]);
|
||||||
|
config(['app.pagination_per_page_default' => 15]);
|
||||||
|
|
||||||
|
// Bulk import: 300 projects that all share the exact same created_at.
|
||||||
|
$sharedCreatedAt = now()->subDay()->startOfSecond();
|
||||||
|
$rows = [];
|
||||||
|
for ($i = 0; $i < 300; $i++) {
|
||||||
|
$rows[] = [
|
||||||
|
'id' => (string) Str::uuid(),
|
||||||
|
'name' => 'Project '.$i,
|
||||||
|
'color' => '#000000',
|
||||||
|
'is_billable' => false,
|
||||||
|
'is_public' => false,
|
||||||
|
'organization_id' => $data->organization->getKey(),
|
||||||
|
'created_at' => $sharedCreatedAt,
|
||||||
|
'updated_at' => $sharedCreatedAt,
|
||||||
|
];
|
||||||
|
}
|
||||||
|
DB::table('projects')->insert($rows);
|
||||||
|
Passport::actingAs($data->user);
|
||||||
|
|
||||||
|
// Act - walk every page like resources/js/utils/fetchAllPages.ts does.
|
||||||
|
$orgId = $data->organization->getKey();
|
||||||
|
$first = $this->getJson(route('api.v1.projects.index', [$orgId]).'?page=1');
|
||||||
|
$this->assertResponseCode($first, 200);
|
||||||
|
$lastPage = $first->json('meta.last_page');
|
||||||
|
$collected = collect($first->json('data.*.id'));
|
||||||
|
for ($page = 2; $page <= $lastPage; $page++) {
|
||||||
|
$response = $this->getJson(route('api.v1.projects.index', [$orgId]).'?page='.$page);
|
||||||
|
$this->assertResponseCode($response, 200);
|
||||||
|
$collected = $collected->concat($response->json('data.*.id'));
|
||||||
|
}
|
||||||
|
|
||||||
|
// Assert - every project appears exactly once, none duplicated or missing.
|
||||||
|
$this->assertEqualsCanonicalizing(array_column($rows, 'id'), $collected->all(), 'Some projects were duplicated or missing across pages');
|
||||||
|
}
|
||||||
|
|
||||||
public function test_index_endpoint_without_filter_archived_returns_only_non_archived_projects(): void
|
public function test_index_endpoint_without_filter_archived_returns_only_non_archived_projects(): void
|
||||||
{
|
{
|
||||||
// Arrange
|
// Arrange
|
||||||
|
|||||||
@@ -24,6 +24,7 @@ use App\Models\User;
|
|||||||
use App\Service\TimeEntryFilter;
|
use App\Service\TimeEntryFilter;
|
||||||
use Illuminate\Support\Carbon;
|
use Illuminate\Support\Carbon;
|
||||||
use Illuminate\Support\Facades\Config;
|
use Illuminate\Support\Facades\Config;
|
||||||
|
use Illuminate\Support\Facades\DB;
|
||||||
use Illuminate\Support\Facades\Log;
|
use Illuminate\Support\Facades\Log;
|
||||||
use Illuminate\Support\Facades\Queue;
|
use Illuminate\Support\Facades\Queue;
|
||||||
use Illuminate\Support\Facades\Storage;
|
use Illuminate\Support\Facades\Storage;
|
||||||
@@ -392,6 +393,59 @@ class TimeEntryEndpointTest extends ApiEndpointTestAbstract
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function test_index_endpoint_pagination_returns_every_time_entry_exactly_once_with_rounding(): void
|
||||||
|
{
|
||||||
|
// Arrange
|
||||||
|
$data = $this->createUserWithPermission([
|
||||||
|
'time-entries:view:own',
|
||||||
|
]);
|
||||||
|
|
||||||
|
// Bulk import: 300 time entries that all share the exact same start.
|
||||||
|
$sharedStart = Carbon::createFromFormat('Y-m-d H:i:s', '2020-01-01 00:00:07');
|
||||||
|
$rows = [];
|
||||||
|
for ($i = 0; $i < 300; $i++) {
|
||||||
|
$rows[] = [
|
||||||
|
'id' => (string) Str::uuid(),
|
||||||
|
'description' => 'Entry '.$i,
|
||||||
|
'start' => $sharedStart,
|
||||||
|
'end' => $sharedStart,
|
||||||
|
'billable' => false,
|
||||||
|
'is_imported' => true,
|
||||||
|
'user_id' => $data->member->user_id,
|
||||||
|
'member_id' => $data->member->getKey(),
|
||||||
|
'organization_id' => $data->organization->getKey(),
|
||||||
|
'created_at' => $sharedStart,
|
||||||
|
'updated_at' => $sharedStart,
|
||||||
|
];
|
||||||
|
}
|
||||||
|
DB::table('time_entries')->insert($rows);
|
||||||
|
$this->actAsOrganizationWithSubscription();
|
||||||
|
Passport::actingAs($data->user);
|
||||||
|
|
||||||
|
// Act - walk every page like the client does (limit/offset), with rounding enabled.
|
||||||
|
$orgId = $data->organization->getKey();
|
||||||
|
$limit = 15;
|
||||||
|
$collected = collect();
|
||||||
|
$offset = 0;
|
||||||
|
do {
|
||||||
|
$response = $this->getJson(route('api.v1.time-entries.index', [
|
||||||
|
$orgId,
|
||||||
|
'member_id' => $data->member->getKey(),
|
||||||
|
'rounding_type' => TimeEntryRoundingType::Nearest,
|
||||||
|
'rounding_minutes' => 6,
|
||||||
|
'limit' => $limit,
|
||||||
|
'offset' => $offset,
|
||||||
|
]));
|
||||||
|
$this->assertResponseCode($response, 200);
|
||||||
|
$ids = $response->json('data.*.id');
|
||||||
|
$collected = $collected->concat($ids);
|
||||||
|
$offset += $limit;
|
||||||
|
} while (count($ids) === $limit);
|
||||||
|
|
||||||
|
// Assert - every time entry appears exactly once, none duplicated or missing.
|
||||||
|
$this->assertEqualsCanonicalizing(array_column($rows, 'id'), $collected->all(), 'Some time entries were duplicated or missing across pages');
|
||||||
|
}
|
||||||
|
|
||||||
public function test_index_endpoint_can_round_up(): void
|
public function test_index_endpoint_can_round_up(): void
|
||||||
{
|
{
|
||||||
// Arrange
|
// Arrange
|
||||||
|
|||||||
Reference in New Issue
Block a user