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

Fix the diff functionality when using with_present #615

Merged
merged 1 commit into from
Jun 9, 2023

Conversation

Rickmarges
Copy link
Contributor

What does this PR do?

The PR extends on the previous PR #613

While testing with with_present: true we noticed no items were created when the api does not provide any results.
This is a small rework of the diff calculation to also add the state: present items whenever the api is empty.

How should this be tested?

There are a few cases to test the behavior of with_present: true

  1. Empty API and empty configuration
  2. Filled API and empty configuration
  3. Empty API and filled configuration
  4. Filled API and filled configuration

In all of these cases the diff functionality works as expected.

Is there a relevant Issue open for this?

Other Relevant info, PRs, etc

@@ -105,6 +105,14 @@ def handle_error(self, **kwargs):
def warn_callback(self, warning):
self.display.warning(warning)

def create_present_list(self, api_list, compare_list):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why are we even passing the api list if we do nothing with it. we should just cut that entire part out.

Copy link
Collaborator

@sean-m-sullivan sean-m-sullivan left a comment

Choose a reason for hiding this comment

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

LGTM, just trying to cut down on operations, keeping it simple :)

@sean-m-sullivan sean-m-sullivan enabled auto-merge (squash) June 9, 2023 14:39
@sean-m-sullivan sean-m-sullivan merged commit c39d834 into redhat-cop:devel Jun 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants