SAT-28731 - Do not assign taxonomies to filters #19899

adamruzicka · 2025-10-07T13:39:43Z

A recent change in Foreman dropped the option to assign (and override) organizations and locations on filters as well as the unlimited and override fields from api responses.

Problem Statement

Some tests use no-longer-available code paths, some tests don't make sense anymore.

Solution

Change the tests that make sense to the new way (aka orgs/locs on roles only), drop the ones that aren't relevant anymore.

Related Issues

https://issues.redhat.com/browse/SAT-28731
theforeman/foreman#10370
theforeman/hammer-cli-foreman#645

Requires SatelliteQE/nailgun#1363 and SatelliteQE/airgun#2141

adamruzicka · 2025-10-08T09:32:49Z

trigger: test-robottelo
pytest: tests/foreman/api/test_contentview.py tests/foreman/api/test_role.py tests/foreman/cli/test_contentview.py tests/foreman/cli/test_filter.py tests/foreman/ui/test_activationkey.py tests/foreman/ui/test_errata.py tests/foreman/ui/test_role.py

adamruzicka · 2025-10-08T14:33:15Z

trigger: test-robottelo
pytest: tests/foreman/api/test_contentview.py

Satellite-QE · 2025-10-08T15:49:05Z

PRT Result

Build Number: 13077
Build Status: UNSTABLE
PRT Comment: pytest tests/foreman/api/test_contentview.py --external-logging
Test Result : === 13 failed, 73 passed, 11 deselected, 1686 warnings in 3806.15s (1:03:26) ===

Satellite-QE · 2025-10-08T18:28:36Z

PRT Result

Build Number: 13068
Build Status: UNSTABLE
PRT Comment: pytest tests/foreman/api/test_contentview.py tests/foreman/api/test_role.py tests/foreman/cli/test_contentview.py tests/foreman/cli/test_filter.py tests/foreman/ui/test_activationkey.py tests/foreman/ui/test_errata.py tests/foreman/ui/test_role.py --external-logging
Test Result : == 30 failed, 320 passed, 18 deselected, 8055 warnings in 31392.21s (8:43:12) ==

adamruzicka · 2025-10-09T09:46:22Z

trigger: test-robottelo
pytest: tests/foreman/api/test_contentview.py tests/foreman/cli/test_contentview.py tests/foreman/ui/test_activationkey.py tests/foreman/ui/test_errata.py tests/foreman/ui/test_role.py -k 'test_negative_readonly_user_actions or test_negative_non_readonly_user_actions or test_negative_user_with_read_only_cv_permission or test_positive_access_non_admin_user or test_positive_list_permission or test_positive_delete_cloned_builtin or test_positive_create_filter or default_role_permission_with_filter_setup'

Satellite-QE · 2025-10-09T10:22:54Z

PRT Result

Build Number: 13087
Build Status: SUCCESS
PRT Comment: pytest tests/foreman/api/test_contentview.py  tests/foreman/cli/test_contentview.py tests/foreman/ui/test_activationkey.py tests/foreman/ui/test_errata.py tests/foreman/ui/test_role.py -k test_negative_readonly_user_actions or test_negative_non_readonly_user_actions or test_negative_user_with_read_only_cv_permission or  test_positive_access_non_admin_user or test_positive_list_permission or test_positive_delete_cloned_builtin or test_positive_create_filter or default_role_permission_with_filter_setup --external-logging
Test Result : ========= 9 passed, 318 deselected, 180 warnings in 1625.77s (0:27:05) =========

sourcery-ai

Hey there - I've reviewed your changes and they look great!

Prompt for AI Agents

Please address the comments from this code review:

## Individual Comments

### Comment 1
<location> `tests/foreman/cli/test_filter.py:53` </location>
<code_context>
     assert set(filter_['permissions'].split(", ")) == set(module_perms)


-def test_positive_create_with_org(module_perms, function_role, target_sat):
-    """Create a filter and assign it some permissions.
-
</code_context>

<issue_to_address>
**suggestion (testing):** Test for creating a filter with organization assignment has been removed.

Consider adding a negative test to verify that the CLI correctly rejects organization assignments to filters, ensuring the new behavior is covered.

Suggested implementation:

```python
import pytest


```

```python
def test_negative_create_with_org(cli_runner, module_perms, function_role, target_sat):
    """
    Negative test: Attempt to create a filter with organization assignment.
    The CLI should reject this and return an error.
    """
    org_id = "test-org"
    filter_name = "test-filter-with-org"
    permissions = ",".join(module_perms)
    # Attempt to create filter with organization assignment
    result = cli_runner.invoke(
        [
            "filter", "create",
            "--name", filter_name,
            "--permissions", permissions,
            "--organization", org_id
        ]
    )
    # Assert that the CLI returns a non-zero exit code or error message
    assert result.exit_code != 0
    assert "organization assignment is not allowed" in result.output.lower()

```
</issue_to_address>

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.}

adamruzicka · 2025-10-09T13:17:12Z

trigger: test-robottelo
pytest: tests/foreman/api/test_contentview.py tests/foreman/cli/test_contentview.py tests/foreman/ui/test_activationkey.py tests/foreman/ui/test_errata.py tests/foreman/ui/test_role.py -k 'test_negative_readonly_user_actions or test_negative_non_readonly_user_actions or test_negative_user_with_read_only_cv_permission or test_positive_access_non_admin_user or test_positive_list_permission or test_positive_delete_cloned_builtin or test_positive_create_filter or default_role_permission_with_filter_setup'
theforeman:
foreman: 10370

Satellite-QE · 2025-10-09T13:59:20Z

PRT Result

Build Number: 13093
Build Status: UNSTABLE
PRT Comment: pytest tests/foreman/api/test_contentview.py tests/foreman/cli/test_contentview.py tests/foreman/ui/test_activationkey.py tests/foreman/ui/test_errata.py tests/foreman/ui/test_role.py -k test_negative_readonly_user_actions or test_negative_non_readonly_user_actions or test_negative_user_with_read_only_cv_permission or test_positive_access_non_admin_user or test_positive_list_permission or test_positive_delete_cloned_builtin or test_positive_create_filter or default_role_permission_with_filter_setup --external-logging
Test Result : ==== 7 failed, 2 passed, 318 deselected, 158 warnings in 1648.55s (0:27:28) ====

adamruzicka · 2025-10-10T09:34:28Z

trigger: test-robottelo
pytest: tests/foreman/api/test_contentview.py tests/foreman/cli/test_contentview.py tests/foreman/ui/test_activationkey.py tests/foreman/ui/test_errata.py tests/foreman/ui/test_role.py -k 'test_negative_readonly_user_actions or test_negative_non_readonly_user_actions or test_negative_user_with_read_only_cv_permission or test_positive_access_non_admin_user or test_positive_list_permission or test_positive_delete_cloned_builtin or test_positive_create_filter or default_role_permission_with_filter_setup'
airgun: 2141
nailgun: 1363
theforeman:
foreman: 10370
hammer-cli-foreman: 645

Satellite-QE · 2025-10-10T10:16:46Z

PRT Result

Build Number: 13106
Build Status: UNSTABLE
PRT Comment: pytest tests/foreman/api/test_contentview.py tests/foreman/cli/test_contentview.py tests/foreman/ui/test_activationkey.py tests/foreman/ui/test_errata.py tests/foreman/ui/test_role.py -k test_negative_readonly_user_actions or test_negative_non_readonly_user_actions or test_negative_user_with_read_only_cv_permission or test_positive_access_non_admin_user or test_positive_list_permission or test_positive_delete_cloned_builtin or test_positive_create_filter or default_role_permission_with_filter_setup --external-logging
Test Result : ==== 1 failed, 8 passed, 318 deselected, 178 warnings in 1873.71s (0:31:13) ====

adamruzicka · 2025-10-10T12:05:34Z

The ui test failure is unrelated to the changes introduced here and is being fixed elsewhere

pondrejk · 2025-10-15T08:57:31Z

tests/foreman/api/test_contentview.py

        target_sat.api.HostCollection(server_config=cfg).create()


 def test_negative_non_readonly_user_actions(target_sat, content_view, function_role, module_org):


@adamruzicka if you are removing the fixture use in the test body, there's no need to include it here, this way I think the function_role still gets created for no use. otoh you could consider creating a different fixture function_role_with_org for a bit of a code reuse. Not sure how much the function_role gets used outside the tests you are touching, if it doesn't, you could add the org in that fixture as well I suppose

adamruzicka · 2025-10-15T10:12:20Z

trigger: test-robottelo
pytest: tests/foreman/api/test_contentview.py tests/foreman/cli/test_contentview.py tests/foreman/ui/test_activationkey.py tests/foreman/ui/test_errata.py tests/foreman/ui/test_role.py -k 'test_negative_readonly_user_actions or test_negative_non_readonly_user_actions or test_negative_user_with_read_only_cv_permission or test_positive_access_non_admin_user or test_positive_list_permission or test_positive_delete_cloned_builtin or test_positive_create_filter or default_role_permission_with_filter_setup'
nailgun: 1363
theforeman:
foreman: 10370
hammer-cli-foreman: 645

Satellite-QE · 2025-10-15T10:57:54Z

PRT Result

Build Number: 13181
Build Status: UNSTABLE
PRT Comment: pytest tests/foreman/api/test_contentview.py tests/foreman/cli/test_contentview.py tests/foreman/ui/test_activationkey.py tests/foreman/ui/test_errata.py tests/foreman/ui/test_role.py -k test_negative_readonly_user_actions or test_negative_non_readonly_user_actions or test_negative_user_with_read_only_cv_permission or test_positive_access_non_admin_user or test_positive_list_permission or test_positive_delete_cloned_builtin or test_positive_create_filter or default_role_permission_with_filter_setup --external-logging
Test Result : ==== 2 failed, 7 passed, 318 deselected, 179 warnings in 1975.29s (0:32:55) ====

A recent change in Foreman dropped the option to assign (and override) organizations and locations on filters as well as the unlimited and override fields from api responses.

adamruzicka · 2025-10-15T11:20:02Z

trigger: test-robottelo
pytest: tests/foreman/api/test_contentview.py tests/foreman/cli/test_contentview.py tests/foreman/ui/test_activationkey.py tests/foreman/ui/test_errata.py tests/foreman/ui/test_role.py -k 'test_negative_readonly_user_actions or test_negative_non_readonly_user_actions or test_negative_user_with_read_only_cv_permission or test_positive_access_non_admin_user or test_positive_list_permission or test_positive_delete_cloned_builtin or test_positive_create_filter or default_role_permission_with_filter_setup'
nailgun: 1363
theforeman:
foreman: 10370
hammer-cli-foreman: 645

Satellite-QE · 2025-10-15T12:03:38Z

PRT Result

Build Number: 13186
Build Status: UNSTABLE
PRT Comment: pytest tests/foreman/api/test_contentview.py tests/foreman/cli/test_contentview.py tests/foreman/ui/test_activationkey.py tests/foreman/ui/test_errata.py tests/foreman/ui/test_role.py -k test_negative_readonly_user_actions or test_negative_non_readonly_user_actions or test_negative_user_with_read_only_cv_permission or test_positive_access_non_admin_user or test_positive_list_permission or test_positive_delete_cloned_builtin or test_positive_create_filter or default_role_permission_with_filter_setup --external-logging
Test Result : ==== 1 failed, 8 passed, 318 deselected, 182 warnings in 1976.02s (0:32:56) ====

adamruzicka · 2025-10-15T12:05:51Z

Guess theforeman/foreman#10717 hasn't landed in stream yet

pnovotny

LGTM

pnovotny · 2025-10-16T14:33:35Z

The one failure in PRT in test_positive_create_filter doesn't look connected to this PR. The test actually fails at later stage than in the last test execution without the PR changes. Thus, the failure can handled separately.
Marking as PRT-Passed.

adamruzicka added the No-CherryPick PR doesnt need CherryPick to previous branches label Oct 7, 2025

adamruzicka force-pushed the no-filter-override branch from c289a4c to 163c364 Compare October 8, 2025 14:25

Satellite-QE added the PRT-Failed Indicates that latest PRT run is failed for the PR label Oct 8, 2025

adamruzicka force-pushed the no-filter-override branch from 163c364 to 869c123 Compare October 9, 2025 09:13

Satellite-QE added PRT-Passed Indicates that latest PRT run is passed for the PR and removed PRT-Failed Indicates that latest PRT run is failed for the PR labels Oct 9, 2025

adamruzicka force-pushed the no-filter-override branch from 869c123 to 81071b7 Compare October 9, 2025 12:53

Satellite-QE removed the PRT-Passed Indicates that latest PRT run is passed for the PR label Oct 9, 2025

adamruzicka marked this pull request as ready for review October 9, 2025 13:05

adamruzicka requested review from a team as code owners October 9, 2025 13:05

github-actions bot assigned adamruzicka Oct 9, 2025

sourcery-ai bot reviewed Oct 9, 2025

View reviewed changes

Satellite-QE added the PRT-Failed Indicates that latest PRT run is failed for the PR label Oct 9, 2025

pondrejk reviewed Oct 15, 2025

View reviewed changes

adamruzicka force-pushed the no-filter-override branch from 81071b7 to 875ea1e Compare October 15, 2025 10:01

adamruzicka requested a review from a team as a code owner October 15, 2025 10:01

SAT-28731 - Do not assign taxonomies to filters

a98c1f5

A recent change in Foreman dropped the option to assign (and override) organizations and locations on filters as well as the unlimited and override fields from api responses.

adamruzicka force-pushed the no-filter-override branch from 875ea1e to a98c1f5 Compare October 15, 2025 11:19

pondrejk approved these changes Oct 15, 2025

View reviewed changes

pnovotny approved these changes Oct 16, 2025

View reviewed changes

pnovotny added PRT-Passed Indicates that latest PRT run is passed for the PR and removed PRT-Failed Indicates that latest PRT run is failed for the PR labels Oct 16, 2025

pnovotny mentioned this pull request Oct 16, 2025

Drop field mapping for Filter SatelliteQE/nailgun#1363

Merged

pnovotny merged commit 73e9003 into SatelliteQE:master Oct 16, 2025
11 of 12 checks passed

adamruzicka deleted the no-filter-override branch November 18, 2025 13:34

		target_sat.api.HostCollection(server_config=cfg).create()


		def test_negative_non_readonly_user_actions(target_sat, content_view, function_role, module_org):

SAT-28731 - Do not assign taxonomies to filters #19899

SAT-28731 - Do not assign taxonomies to filters #19899

Uh oh!

Conversation

adamruzicka commented Oct 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem Statement

Solution

Related Issues

Uh oh!

adamruzicka commented Oct 8, 2025

Uh oh!

adamruzicka commented Oct 8, 2025

Uh oh!

Satellite-QE commented Oct 8, 2025

Uh oh!

Satellite-QE commented Oct 8, 2025

Uh oh!

adamruzicka commented Oct 9, 2025

Uh oh!

Satellite-QE commented Oct 9, 2025

Uh oh!

sourcery-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

adamruzicka commented Oct 9, 2025

Uh oh!

Satellite-QE commented Oct 9, 2025

Uh oh!

adamruzicka commented Oct 10, 2025

Uh oh!

Satellite-QE commented Oct 10, 2025

Uh oh!

adamruzicka commented Oct 10, 2025

Uh oh!

pondrejk Oct 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adamruzicka commented Oct 15, 2025

Uh oh!

Satellite-QE commented Oct 15, 2025

Uh oh!

adamruzicka commented Oct 15, 2025

Uh oh!

Satellite-QE commented Oct 15, 2025

Uh oh!

adamruzicka commented Oct 15, 2025

Uh oh!

pnovotny left a comment

Choose a reason for hiding this comment

Uh oh!

pnovotny commented Oct 16, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

adamruzicka commented Oct 7, 2025 •

edited

Loading

pondrejk Oct 15, 2025 •

edited

Loading