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

Matthew john 1894 #1910

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Matthew john 1894 #1910

wants to merge 5 commits into from

Conversation

duytiennguyen-okta
Copy link
Contributor

Close #1894

MatthewJohn and others added 5 commits February 8, 2024 08:39
…exactly matches.

In the case where there are two apps: "MyApp" and "MyApp 2".
If the data source for saml apps queried with label "MyApp", if the API returns "MyApp 2" as the first app in the list, the data source will fail.
This change now iterate through all of the apps returned and returning an exact match, if "label" has been specified.
// property. We need to further inspect for an exact label match.
if filters.Label != "" {
// guard on nils, an app is always set
app = appList[0]
Copy link
Contributor

Choose a reason for hiding this comment

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

@duytiennguyen-okta I see you've modified to perform initial assignment, but this will assign it to the first app, even if it doesn't match and there's no logic to fail if a matching app isn't found in the for-loop

Copy link
Contributor

Choose a reason for hiding this comment

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

Is it important that app is always set? Given that if it isn't, we were returning a diag error in 912cfb5#diff-bd713a1119c4af963b9c842b006fdbe0a563da383f7172d61e764375ce3831e6L326

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.

3 participants