Skip to content

Conversation

@despadam
Copy link
Member

@despadam despadam commented Apr 29, 2025

Description

GET/jobs and fullQuery/jobs need to apply CASL rules after retrieving the jobs because these rules depend on each job configuration. This was causing inconsistencies in the results of endpoints because the filters and limits of the search where applied first, and the CASL rules afterwards. These rules would exclude jobs that the user does not have access to, causing discrepancies if the results where being paginated.

For example, when applying limits like so: { "where": { "type": "archive" }, "limits": { "skip": 0, "limit": 10 } } one would expect the first page to contain 10 jobs, and similarly the rest of the pages, until we reach the end of the list. However, since the execution order was 1. where + limits, 2. CASL rules, there were cases where a page would contain less than 10 jobs even if the remaining number of jobs was more than that.

Motivation

Return consistent number of jobs per page, when pagination is applied.

Changes:

Instead of using findAll and findquery from jobs.service, introduce findByFilters and applyFilterLimits, so that they can be applied separarately and the execution order can change to 1. where, 2. CASL rules, 3. limits. This way the pagination will be done after all filtering has been applied. Now GET/jobs, fullQuery/jobs as well as fullFacet/jobs have been updated to always return consistent results.

Tests included

  • Included for each change/fix?
  • Passing?

Documentation

  • swagger documentation updated (required for API changes)
  • official documentation updated

official documentation info

@despadam despadam self-assigned this Apr 29, 2025
@despadam despadam marked this pull request as ready for review May 5, 2025 10:12
@despadam despadam requested review from nitrosx, sbliven and sofyalaski May 5, 2025 10:16
Copy link
Member

@sofyalaski sofyalaski left a comment

Choose a reason for hiding this comment

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

Looks good to me! The only thing maybe is that we should also provide an example for SwaggerUI similar as we do in all other methods?

@despadam despadam merged commit df94798 into master May 20, 2025
14 checks passed
@despadam despadam deleted the fix-jobs-get-endpoints branch May 20, 2025 22:17
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.

3 participants