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

Add Data Access Control #26492

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

Add Data Access Control #26492

wants to merge 6 commits into from

Conversation

urseberry
Copy link
Contributor

What does this PR do? What is the motivation?

Merge instructions

Merge queue is enabled in this repo. To have it automatically merged after it receives the required reviews, create the PR (from a branch that follows the <yourname>/description naming convention) and then add the following PR comment:

/merge

Additional notes

@urseberry urseberry requested a review from a team as a code owner November 26, 2024 01:32
@urseberry urseberry added the editorial review Waiting on a more in-depth review label Nov 26, 2024
@github-actions github-actions bot added Architecture Everything related to the Doc backend Images Images are added/removed with this PR labels Nov 26, 2024
Copy link
Contributor

Preview links (active after the build_preview check completes)

New or renamed files

@urseberry urseberry force-pushed the uchen/data-access branch 3 times, most recently from b76858f to 6917125 Compare November 26, 2024 08:27
Copy link
Contributor

@aliciascott aliciascott left a comment

Choose a reason for hiding this comment

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

Hi @urseberry this looks good! I have some suggestions mostly for wording and flow and trying to be consistent with capitalizing or not capitalizing Restricted Dataset. Also I think the Supported and not supported examples could benefit from a little bit of re-formatting, but feel free to keep it as is!

content/en/account_management/rbac/data_access.md Outdated Show resolved Hide resolved
content/en/account_management/rbac/data_access.md Outdated Show resolved Hide resolved
content/en/account_management/rbac/data_access.md Outdated Show resolved Hide resolved
content/en/account_management/rbac/data_access.md Outdated Show resolved Hide resolved
content/en/account_management/rbac/data_access.md Outdated Show resolved Hide resolved
content/en/account_management/rbac/data_access.md Outdated Show resolved Hide resolved
content/en/account_management/rbac/data_access.md Outdated Show resolved Hide resolved
content/en/account_management/rbac/data_access.md Outdated Show resolved Hide resolved
content/en/account_management/rbac/data_access.md Outdated Show resolved Hide resolved
content/en/account_management/rbac/data_access.md Outdated Show resolved Hide resolved
@urseberry
Copy link
Contributor Author

I'm doing the changes in batches to make them easier to reason about. Will come back later to address the rest of the feedback.

@urseberry
Copy link
Contributor Author

urseberry commented Nov 27, 2024

Alicia, this is ready for your re-review. Some things to note:

  • I accepted all your wording suggestions.
  • I decided to make Restricted Dataset capitalized throughout. Thanks for pointing out the inconsistency!
  • Data Access Control will be added to the preview signup site soon.
  • The in-app link to Data Access Control requires the org_management permission, which I don't have in any org, so I can't test it. I assume the PM gave me the right link. Do you have org_management anywhere?
  • I adjusted your formatting for the examples a bit. I also removed the bolding from the field names, because I thought it made them hard to read. See attached screenshot. Thanks for the suggestion, though. I think the new version is clearer than what I originally had.
Screenshot 2024-11-27 at 12 45 25 AM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Architecture Everything related to the Doc backend editorial review Waiting on a more in-depth review Images Images are added/removed with this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants