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

Paginate continues even if null records are returned #370

Merged
merged 18 commits into from
Apr 28, 2022

Conversation

gweinjc
Copy link
Contributor

@gweinjc gweinjc commented Apr 27, 2022

Issues

  • SA-2427 - Paginate continues even if null records are returned

What does this solve?

If a system_group with a null system exists, the previous pagination would end once it came across the null system which would not return the full result set. The fix looks at the requests "x-total-count" to know how many results should be present

Import-JCCommand was having regex issues and that was resolved as well by making adjustments to the regex

Is there anything particularly tricky?

The only way to properly test this is by using an org that has a null system in a system group, which cannot be easily replicated and would have to involve database manipulation to test thoroughly

How should this be tested?

Import the module locally from the current branch
Run Get-JCSystemGroupMember or Get-JCUserGroupMember and validate that it is returning the correct number of results

Copy link
Contributor

@jworkmanjc jworkmanjc left a comment

Choose a reason for hiding this comment

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

I think this looks pretty good, it's nice to see some of the reusable code packaged up into a function. Nice work making this look clean and simple.

We still need to update the PublishToPSGallery param to be true in order for the module to release when we merge this in.

Copy link
Contributor

@kmaranionjc kmaranionjc 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. Awesome work!

@gweinjc gweinjc merged commit c8166fe into JumpCloudModule_1.20.1 Apr 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants