fix: handle static DynamicGroups in job filter and repo sync

[xtinalang]

Why:

• Nautobot supports two DynamicGroup types: dynamic filter (query-based)
  and static (manually assigned members)
• generate_query() raises RuntimeError on static groups because they
  have no filter expression to evaluate
• This caused ALL Golden Config jobs to crash when any GoldenConfigSetting
  was linked to a static DynamicGroup

What changed:

• jobs.py (get_refreshed_repos): replaced generate_query() with
  group.members.values_list("pk", flat=True) to safely check device
  membership for both group types
• helper.py (get_job_filter): same fix applied where the base device
  queryset is built from all GoldenConfigSettings
• Added regression tests for both code paths

Fixes #1029

Closes: #

What was changed

• jobs.py (get_refreshed_repos): replaced generate_query() with
  group.members.values_list("pk", flat=True) — works correctly for both
  static and dynamic group types
• helper.py (get_job_filter): same fix applied where the base device
  queryset is built across all GoldenConfigSettings

To Do

• Explanation of Change(s)
• Added change log fragment(s)
• Attached Screenshots, Payload Example
• Unit, Integration Tests
• Documentation Updates (when adding/changing features)
• Outline Remaining Work, Constraints from Design

[github-actions]

Coverage report

Click to see where and how coverage changed

File	Statements	Missing	Coverage	Coverage (new stmts)	Lines missing
nautobot_golden_config
jobs.py
nautobot_golden_config/utilities
helper.py
Project Total

This report was generated by python-coverage-comment-action

[itdependsnetworks]

I see your point... but perhaps this should be fixed in core or core may already have a method that handles this already, @glennmatthews any thoughts?

[glennmatthews]

Checking against .members instead of .generate_query seems correct in general to me, as .members already returns a queryset; I'm not familiar enough with the two specific uses here to understand exactly what they're trying to do, and be able to say whether they could be implemented more efficiently.

[xtinalang]

I initially marked this as a draft because I wasn't confident the implementation was complete or correct — I wanted to validate the approach before requesting a full review. After testing and getting feedback, I've updated the implementation to avoid .members entirely. Instead of looping through GC settings and checking .members, I'm now using the existing GoldenConfigSettingManager.get_for_device() method which, as I understand it, traverses device.dynamic_groups at the ORM level — a single DB query per device regardless of how many GC settings exist. I'm hoping this handles both static and filter-based groups correctly and more efficiently. Happy to take a different approach if needed.

[itdependsnetworks]

This would essentially undo #941

[itdependsnetworks]

Checking against .members instead of .generate_query seems correct in general to me, as .members already returns a queryset; I'm not familiar enough with the two specific uses here to understand exactly what they're trying to do, and be able to say whether they could be implemented more efficiently.

@glennmatthews the crux of the question is should the API for the DG be different if it is using static? Specfically, in regards to generate_query. It would seem likely that we can just have the static method respect generate_query and return a queryset, and the rest of the workflow would be fine.

[glennmatthews]

Maybe I'm missing something but it seems to me at a glance that members and generate_query are if anything a bit redundant with one another.

[xtinalang]

Thank you @glennmatthews and @itdependsnetworks. I am grateful for the time you both took to look at this and for your knowledge of Nautobot. You pointed me in the right direction, and I missed the real intent. You were right, and I was wrong. I am humbled by this experience.

Going back to look at DynamicGroup in Core more carefully, .members does work for both static and dynamic-filter groups. The implementation is the same in 3.0, 3.1, and develop. The RuntimeError was coming from a direct call to obj.dynamic_group.generate_query(), not .members.

I pushed a better refactor (834fdfd), and the helper is gone. Hopefully this is right. The helper is gone, and get_job_filter reduces to a one-liner using obj.dynamic_group.members.values_list("pk", flat=True). The app should no longer have its own logic for static vs dynamic filter groups. Like you both were saying, the app does not need to handle the static and dynamic-filter groups differently. If there is a better way, please let me know.

Again, I am humbled -- big lesson learned. It is great having you guys here to work with as I learn the ropes. Note to self: When I am working on a plugin like Golden Config, I need to read the core source for the APIs I am using, not just react to the symptom in the app. Thank you for walking me through this.

This is still a work in progress and I am still open to suggestions or if I am going in the wrong direction.

[itdependsnetworks]

In general we can likely close this out, depending on how urgent this is (see work around below for more details.) We can/should update generate_query() to be part of the static group assignment in core. It does not seem like the issue with settings is actually a challenge, but at a min, should not be looped in the way it was being suggested.

If it is urgent, this should work in the two places that it would be relevant.

        for obj in models.GoldenConfigSetting.objects.all():
            if dynamic_group__group_type=DynamicGroupTypeChoices.TYPE_STATIC:
                raw_qs = raw_qs | Q(pk__in=obj.dynamic_group.members.values_list("pk", flat=True))
            else:
                raw_qs = raw_qs | obj.dynamic_group.generate_query()