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 organization ruleset conditions schema validation, update docs #1911

Merged
merged 1 commit into from
Sep 21, 2023

Conversation

o-sama
Copy link
Contributor

@o-sama o-sama commented Sep 21, 2023

Resolves #1899 & #1909


Before the change?

  • Validation on organization_ruleset.conditions.0 was partially broken.
    • It correctly prevented setting both repository_name and repository_id.
    • It incorrectly allowed setting neither repository_name nor repository_id.
  • Documentation for bypass_actors didn't specify anything related to actor_id when setting type to OrganizationAdmin, the API docs also don't mention anything but the expected value is 1 as tested by @YElyousfi.

After the change?

  • At least one of repository_name and repository_id is required for organization rulesets conditions, previously this would fail on apply since the API requires at least one to be set alongside ref_name. This shouldn't require a major version change because requests sent using the previous iteration weren't actually succeeding, so there shouldn't be a state change or breakage of existing use.
  • Updated docs for both of the issues mentioned in the before section.

Pull request checklist

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)

Does this introduce a breaking change?

Please see our docs on breaking changes to help!

  • Yes
  • No

@lewismiddleton
Copy link

I left a comment on #1899 with some more bypass actor_id quirkiness. Could expand the scope of this PR to address that too.

I've also opened #1915 which may be of interest to you.

Copy link
Contributor

@nickfloyd nickfloyd left a comment

Choose a reason for hiding this comment

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

Thanks for the commits here! ❤️

@nickfloyd nickfloyd added the Type: Documentation Improvements or additions to documentation label Sep 21, 2023
@nickfloyd nickfloyd merged commit 8540ea4 into integrations:main Sep 21, 2023
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Organization Ruleset missing support for OrganizationAdmin bypass
3 participants