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.

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

Fix the pagination of recent jobs list #1366

Merged
merged 1 commit into from
Jan 15, 2024

Conversation

kirba
Copy link
Contributor

@kirba kirba commented Jan 14, 2024

There is an issue with navigating to the next / previous pages in recent jobs pages (pending, completed & silenced).

To summarize, a job at index 50 will be skipped when navigating to the 2nd page and a job at index 0 will be skipped when navigating back to the first page.

https://github.com/laravel/horizon/blob/5.x/src/Repositories/RedisJobRepository.php#L244-L251

This function is responsible for pulling the list of jobs from Redis

 protected function getJobsByType($type, $afterIndex)
  {
      $afterIndex = $afterIndex === null ? -1 : $afterIndex;
  
      return $this->getJobs($this->connection()->zrange(
          $type, $afterIndex + 1, $afterIndex + 50
      ), $afterIndex + 1);
  }

what this function does is it takes the $afterIndex add 1 to it for the starting index and adds 50 for the ending.

That works for the first page, we will send -1 and it will load jobs between 0 and 49. But when we navigate to the second page, as a starting index we are sending 50 instead of 49 (since in the Vue component we would multiple the current page (1) with the number of records we load per page (50)). And then what happens in the Repo class is that we load jobs starting at index 51 until index 100. By doing this we have skipped one job. Further pages would be fine because they will follow the same principle. (101-150 etc)

And when going back to the first page we multiply current page minus 2 (0) with number of records we load per page and we get 0. Then we are loading jobs from 1 to 50 and we are skipping the very first job at the zero index. In this case the issue will resolve itself in 3 seconds as we are auto refreshing with the starting value of -1 while on the first page.

The fix is very easy, for the starting value we should always send a value decremented by 1 after doing computation that involves multiplying the current page number with number of records. That way we would sent -1, 49, 99, etc as starting values and we would not skip a job.

@taylorotwell taylorotwell merged commit ec5510f into laravel:5.x Jan 15, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants