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

1640215: Fix incorrect partial compliance status on consumers #2143

Merged
merged 1 commit into from Nov 6, 2018

Conversation

awood
Copy link
Contributor

@awood awood commented Nov 2, 2018

If a consumer does not specify a syspurpose attribute (role, usage,
etc.), do not mark them partially compliant if they attach to a pool
that provides that attribute.

This commit also fixes a Jackson error that I discovered while writing
the test cases. Jackson was attempting to deserialize compliance reason
JSON to a Map<String, String> when the JSON had arrays as the values
instead.

@@ -140,19 +139,21 @@ public SystemPurposeComplianceStatus getStatus(Consumer consumer,
}
unsatisfiedAddons.removeAll(addonsFound);

if (product.hasAttribute(Product.Attributes.SUPPORT_LEVEL)) {
if (StringUtils.isNotEmpty(preferredSla) &&
Copy link
Contributor

Choose a reason for hiding this comment

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

If there's something else to fix in this PR, we could also reorganize this code a bit to only do this check once. Both of the places it was added to could be nested within a single check, since both are contingent on it being non-empty.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure I understand. One spot is StringUtils.isNotEmpty(preferredSla) and the other is StringUtils.isNotEmpty(preferredUsage).

Copy link
Contributor

Choose a reason for hiding this comment

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

Oof... disregard. This is what happens when I review code while dealing with migraine-induced double vision.

If a consumer does not specify a syspurpose attribute (role, usage,
etc.), do not mark them partially compliant if they attach to a pool
that provides that attribute.

This commit also fixes a Jackson error that I discovered while writing
the test cases.  Jackson was attempting to deserialize compliance reason
JSON to a Map<String, String> when the JSON had arrays as the values
instead.
@awood awood force-pushed the awood/1640215-partial-compliance-on-empty-preference branch from f77df19 to 71141e3 Compare November 6, 2018 15:01
Copy link
Contributor

@Ceiu Ceiu left a comment

Choose a reason for hiding this comment

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

ack

@Ceiu Ceiu merged commit f67eb18 into master Nov 6, 2018
@Ceiu Ceiu deleted the awood/1640215-partial-compliance-on-empty-preference branch November 6, 2018 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants