Skip to content

Added correct counting of licenses seats through pagination #14099

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

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

Conversation

Godmartinz
Copy link
Member

Description

The lIcenses seat number would restart if the page was changed. Now licenses seats count continues through pagination

image

Fixes #14067

Type of change

Please delete options that are not relevant.

  • [ x] Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • Test A
  • Test B

Test Configuration:

  • PHP version:
  • MySQL version
  • Webserver version
  • OS version

Checklist:

Copy link

what-the-diff bot commented Jan 4, 2024

PR Summary

  • Introduction of page-based navigation in License Seats Controller
    The License Seats Controller, which oversees the management of license seats in the application, now can understand 'pages'. It basically means faster and more efficient browsing for users when going through their license seats data.

  • Changes in the logic for counting license seats
    A renewal of the method which counts the license seats was made in the related file. This change accommodates the addition of 'pages' and improves the accuracy of the count, especially when browsing through different pages of data. This ensures users always see correct numbers in their interface.

@marcusmoore
Copy link
Collaborator

marcusmoore commented Feb 6, 2024

Edit: See comment below


Previous comment:

I like this but it would be good to handle API requests from outside of our application as well.

For example, the requests that the table on the license show page fires has sort and order in it:

https://snipe-it.test/api/v1/licenses/1/seats?sort=name&order=asc&offset=10&limit=10

The sort=name doesn't really matter but order=asc does...if I fire a manual request using the example in the API docs, which does not include order=asc, the results don't match up:
mismatch

But if I add order=asc then the results line up:
in alignment

Should we always add order=asc to the query so the results are the same no matter the context?

@Godmartinz
Copy link
Member Author

@snipe if we want to only sort by ascending let me know, I'll edit this PR. But the seat number doesn't really matter for licenses right?

@marcusmoore
Copy link
Collaborator

@Godmartinz asked for clarification about this and I think my overall request makes sense but the implementation would be outside of the scope of this PR.

@snipe
Copy link
Member

snipe commented Apr 23, 2024

Gentle ping, @Godmartinz

@snipe
Copy link
Member

snipe commented May 6, 2024

Gentle ping again, @Godmartinz

@Godmartinz
Copy link
Member Author

whoops missed this, fixing...

@Godmartinz
Copy link
Member Author

@snipe a check on offset is performed prior to setting the $seat_count

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants