Skip to content

Add better tests for GraphQL filtering and lookup #15569

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
arthanson opened this issue Mar 28, 2024 · 5 comments
Open

Add better tests for GraphQL filtering and lookup #15569

arthanson opened this issue Mar 28, 2024 · 5 comments
Labels
netbox status: backlog Awaiting selection for work topic: GraphQL type: housekeeping Changes to the application which do not directly impact the end user

Comments

@arthanson
Copy link
Collaborator

Proposed Changes

The GraphQL tests currently test all model fields, but are very light on testing any filtering of objects or sub-objects. We should add tests for checking different primary filters:

{
  site_list(filters: {name: {i_contains:"DM"}}) {
    id
    name
  }
}

and sub-object filtering:

{
  site_list {
    id
    name
    tags(filters: {name: {i_starts_with: "P"}}) {
      name
    }
  }
}

Justification

The filtering logic for Strawberry is undergoing some major refactors and we could easily miss issues with filtering without tests in-place.

@arthanson arthanson added the type: housekeeping Changes to the application which do not directly impact the end user label Mar 28, 2024
@arthanson arthanson added status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation beta Concerns a bug/feature in a beta release labels Apr 25, 2024
@jeremystretch jeremystretch removed the beta Concerns a bug/feature in a beta release label May 6, 2024
Copy link
Contributor

github-actions bot commented Aug 5, 2024

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. NetBox is governed by a small group of core maintainers which means not all opened issues may receive direct feedback. Do not attempt to circumvent this process by "bumping" the issue; doing so will result in its immediate closure and you may be barred from participating in any future discussions. Please see our contributing guide.

@github-actions github-actions bot added the pending closure Requires immediate attention to avoid being closed for inactivity label Aug 5, 2024
@jeremystretch jeremystretch added status: backlog Awaiting selection for work and removed status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation pending closure Requires immediate attention to avoid being closed for inactivity labels Aug 21, 2024
@loulecrivain
Copy link

Heya, just a heads up from a Netbox-using organization regarding this proposed change.

I've seen and experienced some issues/regressions regarding GraphQL filters following latest release and refactors.
Now I know GraphQL isn't used much internally in Netbox itself. However for lots of people writing exporters, crawlers, you name it, GraphQL API is actually the go-to.

In my opinion, if we want to be on par with the level of quality of the REST API, ensuring filters are working as expected should be a priority. +1 for automated testing here

@jeremystretch
Copy link
Member

@loulecrivain thanks for your interest. Would you be willing to volunteer to contribute this work?

@loulecrivain
Copy link

@jeremystretch I get that the Netbox team may be busy right now with latest releases too. Unfortunately I currently do not really have the time to work on this, as I'm busy with other projects (and also not sure I'm familiar enough with Netbox codebase, as I haven't contributed lots).

So I'll have to politely decline 😉

@jeremystretch jeremystretch added the netbox label Nov 1, 2024 — with Linear
@loulecrivain
Copy link

Commenting on this issue again, since we had another regression / breaking change in a minor release update (4.1.x -> 4.2.x).

#18433 broke in-production software depending on the GraphQL API. I'm going to be honest, I get the need for making the models evolve according to users needs, but why do so in a minor release?

I urge you to consider prioritizing better end-to-end tests for the GraphQL API. IMO if it's not as stable as the REST API, then there's no point in having this feature available for the users.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
netbox status: backlog Awaiting selection for work topic: GraphQL type: housekeeping Changes to the application which do not directly impact the end user
Projects
None yet
Development

No branches or pull requests

4 participants