Fix role schema #27
Fix role schema #27
Conversation
jbristow
requested review from
angelosantos4,
ccloes,
rreddy2 and
zprobst
as code owners
| if permission: | ||
| output["permission"] = permission | ||
| return output |
There was a problem hiding this comment.
this is unnecessary and potentially confusing, removed
Coverage Report
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if affiliation: | ||
| output["affiliation"] = affiliation | ||
| return output |
There was a problem hiding this comment.
this is unnecessary and potentially confusing, removed
| security_and_analysis_advanced_security: !jmespath 'security_and_analysis.advanced_security.status' | ||
| security_and_analysis_secret_scanning: !jmespath 'security_and_analysis.secret_scanning.status' | ||
| security_and_analysis_secret_scanning_push_protection: !jmespath 'security_and_analysis.secret_scanning_push_protection.status' | ||
| - type: source_node |
There was a problem hiding this comment.
curse ye, autoformat
| if self.include_repositories: | ||
| full_org["repositories"] = [ | ||
| simplify_repo( | ||
| repo, permission=full_org.get("default_repository_permission") |
There was a problem hiding this comment.
Duplicating this onto the relationship was a mistake.
| affiliation=CollaboratorAffiliation.DIRECT, | ||
| ): | ||
| collaborators.append(simplify_user(user, affiliation="direct")) | ||
| collaborators.append(simplify_user(user) | {"affiliation": "direct"}) |
There was a problem hiding this comment.
see how much nicer this is?
| "node_id": "MDEwOlJlcG9zaXRvcnkxMjk2MjY5", | ||
| "permission": "admin", | ||
| "url": "https://HOSTNAME/repos/octocat/Hello-World", | ||
| "role_name": "read", |
There was a problem hiding this comment.
note that I am making this "role_name" now in order to be consistent with the GitHub REST api. It is up to the end user to change it if they want.
Previously, the team.permission value was copied up into team.repos[].permission and this turns out to be overly confusing.
| outbound: false | ||
| key_normalization: | ||
| do_lowercase_strings: false | ||
|
|
There was a problem hiding this comment.
if we remove the permission in the IN_ORGANIZATION relationship, where would we store it in the next iteration? Will that be done through the simplify_repo/simplify_user changes you made in /relationship/repository.py and /relationship/user.py?
There was a problem hiding this comment.
It's in org.default_repository_permissions which is where I've been writing queries from. I don't think we're even writing this permission to our data since our yaml is a custom version of this.
Summary of Change
Roles were being added willy nilly and reflected the author's poor understanding of GitHub permissions and how they interact with the UI and legacy implementations.
This fixes them and adds back a little info about permissions where we can.
Before Review
Before Merge