Remove deprecated CV/LCE params from ActivationKey and HostGroup by jeremylenz · Pull Request #1419 · SatelliteQE/nailgun

jeremylenz · 2026-05-21T18:51:38Z

Description of changes

Remove the deprecated separate content_view and environment fields from ActivationKey, replacing them with content_view_environment_ids to match the Katello 4.21+ API changes (Fixes #39330).

ActivationKey:

Remove content_view (OneToOneField) and environment (OneToOneField) from _fields
Add content_view_environment_ids (ListField) for create/update payloads
Override read() to populate content_view_environments from the API response
Add backward-compat @property for content_view and environment (read-only, extracts from first content view environment)

HostGroup:

Add content_view_environment_id (IntegerField) to _fields
Update create_payload() and update_payload() to strip legacy content_view_id/lifecycle_environment_id when content_view_environment_id is present

Upstream API documentation, plugin, or feature links

Katello PR: Katello/katello#11753 (branch sat-38100-remove-deprecated-cv-lce-params)
Robottelo PR: SatelliteQE/robottelo#21644

Functional demonstration

Callers update from:

ak = ActivationKey(content_view=cv, environment=lce, organization=org).create()

to:

ak = ActivationKey(content_view_environment_ids=[cvenv_id], organization=org).create()

Read-side backward compat is preserved:

ak = ActivationKey(id=123).read()
ak.content_view.id      # still works via @property
ak.environment.id        # still works via @property
ak.content_view_environments  # new: list of dicts with full CV/LCE data

Additional Information

Companion Robottelo PR will follow with test updates.

🤖 Generated with Claude Code

sourcery-ai

Hey - I've left some high level feedback:

In ActivationKey.read, you're mutating the ignore set passed in by the caller (ignore.add(...)); consider creating a new set (e.g., ignore = set(ignore or []) | {'content_view_environment_ids'}) to avoid surprising side effects when callers reuse the same set.
The content_view and environment properties assume the nested dicts always contain an 'id' key; it might be safer to use .get('id') and return None when it's missing to avoid a KeyError on malformed or partial API responses.
For HostGroup.create_payload/update_payload, you might want to check for a non-None content_view_environment_id rather than just key presence (e.g., if payload.get('content_view_environment_id') is not None:) so that you only strip the legacy IDs when a new CVE value is actually being sent.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- In `ActivationKey.read`, you're mutating the `ignore` set passed in by the caller (`ignore.add(...)`); consider creating a new set (e.g., `ignore = set(ignore or []) | {'content_view_environment_ids'}`) to avoid surprising side effects when callers reuse the same set.
- The `content_view` and `environment` properties assume the nested dicts always contain an `'id'` key; it might be safer to use `.get('id')` and return `None` when it's missing to avoid a `KeyError` on malformed or partial API responses.
- For `HostGroup.create_payload`/`update_payload`, you might want to check for a non-`None` `content_view_environment_id` rather than just key presence (e.g., `if payload.get('content_view_environment_id') is not None:`) so that you only strip the legacy IDs when a new CVE value is actually being sent.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

Copilot

Pull request overview

This PR updates Katello-facing entity models for the newer content view environment API shape, replacing deprecated separate content view / lifecycle environment parameters where applicable.

Changes:

Replaces ActivationKey create/update input with content_view_environment_ids.
Adds read-side ActivationKey compatibility helpers for content_view and environment.
Adds HostGroup content_view_environment_id payload support and removes legacy CV/LCE IDs when it is present.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+        if 'content_view_environment_id' in payload:
+            payload.pop('content_view_id', None)
+            payload.pop('lifecycle_environment_id', None)


LadislavVasina1

Changes look good to me, but I would like someone with more nailgun "skills" :D to also review.
Also I would merge only after we are close to delivering the Katello changes to the downstream snap.

jeremylenz · 2026-05-28T18:14:28Z

Katello change is merged now.

vsedmik

Looks good to me. The CI-CQ failure seems unrelated (raised #1421 to fix it).

Maybe we could add some unit test(s) for the new properties, just to satisfy the requirements.

synkd · 2026-06-02T14:33:25Z

@jeremylenz This looks good, but could you change the instances of cves to cvenvs?

vsedmik

LGTM, thank you for the update @jeremylenz!

sambible

ACK pending fixes for the failing unit tests.

ActivationKey: Replace content_view/environment fields with content_view_environment_ids for create/update. Add read() override to populate content_view_environments from the API response, and backward-compat properties for content_view/environment. HostGroup: Add content_view_environment_id field. Strip legacy content_view_id/lifecycle_environment_id from payloads when the new field is present. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

jeremylenz · 2026-06-02T19:43:09Z

latest commit is to fix Robottelo PRT failures.

This field is write-only — the API response does not include it, so read() must skip it to avoid a KeyError. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

jnagare-redhat

LGTM

jeremylenz mentioned this pull request May 21, 2026

Update tests for deprecated CV/LCE param removal SatelliteQE/robottelo#21644

Merged

LadislavVasina1 added No-CherryPick PR doesnt need CherryPick to previous branches Stream labels May 27, 2026

LadislavVasina1 requested a review from Copilot May 27, 2026 13:55

LadislavVasina1 self-assigned this May 27, 2026

Copilot started reviewing on behalf of LadislavVasina1 May 27, 2026 13:55 View session

jeremylenz marked this pull request as ready for review May 27, 2026 13:56

Satellite-QE assigned jeremylenz May 27, 2026

sourcery-ai Bot reviewed May 27, 2026

View reviewed changes

Copilot AI reviewed May 27, 2026

View reviewed changes

Comment thread nailgun/entities.py

Comment thread nailgun/entities.py

Comment on lines +4099 to +4101

if 'content_view_environment_id' in payload:

payload.pop('content_view_id', None)

payload.pop('lifecycle_environment_id', None)

LadislavVasina1 requested review from a team, ColeHiggins2, LadislavVasina1, jnagare-redhat and synkd May 27, 2026 14:36

LadislavVasina1 previously approved these changes May 28, 2026

View reviewed changes

LadislavVasina1 requested review from a team and vsedmik May 28, 2026 08:11

vsedmik reviewed Jun 2, 2026

View reviewed changes

synkd previously approved these changes Jun 2, 2026

View reviewed changes

jeremylenz dismissed stale reviews from synkd and LadislavVasina1 via e7ffa24 June 2, 2026 14:36

jeremylenz force-pushed the remove-deprecated-cv-lce-params branch from e7ffa24 to c928319 Compare June 2, 2026 14:37

vsedmik mentioned this pull request Jun 2, 2026

Add test coverage for SAT-45590 SatelliteQE/robottelo#21718

Merged

vsedmik previously approved these changes Jun 2, 2026

View reviewed changes

jeremylenz force-pushed the remove-deprecated-cv-lce-params branch from c928319 to dd22844 Compare June 2, 2026 16:57

jeremylenz dismissed vsedmik’s stale review via 6b0f708 June 2, 2026 19:38

sambible force-pushed the remove-deprecated-cv-lce-params branch from 6b0f708 to 77b11ab Compare June 2, 2026 19:38

sambible previously approved these changes Jun 2, 2026

View reviewed changes

jeremylenz force-pushed the remove-deprecated-cv-lce-params branch from 77b11ab to afaf8d6 Compare June 2, 2026 19:42

jeremylenz and others added 2 commits June 2, 2026 15:42

Rename cves to cvenvs and add unit tests for AK properties

d3a83a6

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

jeremylenz dismissed sambible’s stale review via 1d2d922 June 2, 2026 19:45

jeremylenz force-pushed the remove-deprecated-cv-lce-params branch 2 times, most recently from 1d2d922 to affbb03 Compare June 2, 2026 19:46

Ignore content_view_environment_id in HostGroup read()

affbb03

This field is write-only — the API response does not include it, so read() must skip it to avoid a KeyError. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

vsedmik approved these changes Jun 4, 2026

View reviewed changes

LadislavVasina1 approved these changes Jun 9, 2026

View reviewed changes

LadislavVasina1 requested review from sambible and synkd June 9, 2026 13:40

LadislavVasina1 added the PRT-Passed label Jun 9, 2026

synkd approved these changes Jun 9, 2026

View reviewed changes

jnagare-redhat approved these changes Jun 9, 2026

View reviewed changes

sambible approved these changes Jun 9, 2026

View reviewed changes

sambible merged commit 5e3e401 into SatelliteQE:master Jun 9, 2026
16 checks passed

Navigation Menu

Remove deprecated CV/LCE params from ActivationKey and HostGroup - #1419

Remove deprecated CV/LCE params from ActivationKey and HostGroup#1419
sambible merged 3 commits into
SatelliteQE:masterfrom
jeremylenz:remove-deprecated-cv-lce-params

jeremylenz commented May 21, 2026 •

edited

Loading

Uh oh!

sourcery-ai Bot left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

LadislavVasina1 left a comment

Uh oh!

jeremylenz commented May 28, 2026

Uh oh!

vsedmik left a comment

Uh oh!

synkd commented Jun 2, 2026

Uh oh!

vsedmik left a comment

Uh oh!

sambible left a comment

Uh oh!

jeremylenz commented Jun 2, 2026

Uh oh!

jnagare-redhat left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Conversation

jeremylenz commented May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of changes

Upstream API documentation, plugin, or feature links

Functional demonstration

Additional Information

Uh oh!

sourcery-ai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

LadislavVasina1 left a comment

Choose a reason for hiding this comment

Uh oh!

jeremylenz commented May 28, 2026

Uh oh!

vsedmik left a comment

Choose a reason for hiding this comment

Uh oh!

synkd commented Jun 2, 2026

Uh oh!

vsedmik left a comment

Choose a reason for hiding this comment

Uh oh!

sambible left a comment

Choose a reason for hiding this comment

Uh oh!

jeremylenz commented Jun 2, 2026

Uh oh!

jnagare-redhat left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

jeremylenz commented May 21, 2026 •

edited

Loading