-
Notifications
You must be signed in to change notification settings - Fork 4
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: Policies always deny #28
base: main
Are you sure you want to change the base?
Conversation
…nated. This fix also resolves loft-sh#9.
This PR also solves the same issue, but slightly different solution: #10 (open for 2 years...) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great!
To maintainers: Can we please get this merged? This error bit us when trying to use the SDK.
@LukasGentele or @FabianKramm, could you please take a look? This fixes a pretty old bug in the SDK. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm. @FabianKramm this works fine and fixes the issue
When there are no errors, the
combinedErrors
array will consist of empty arrays:[ [], [], ..., [] ]
. Consequently, the conditioncombinedErrors.length > 0
will always evaluate to true, as the length ofcombinedErrors
equals the number of inner arrays, thereby triggering the denial of the request.In this pull request, I'm utilizing the
errors.flat()
method, which returns a new array with its sub-array elements concatenated into it. This ensures that when no error is present,combinedErrors
will be equal to an empty array[]
, while when errors are present, it will contain strings like "error 1","error 2", ..., "error n"
.This fix also resolves #9.