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.

Sign up for GitHub

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

Jump to bottom

enhance spo file roleassignment command #6431

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

enhance spo file roleassignment command #6431

wants to merge 2 commits into from

Conversation

ktskumar
Copy link
Contributor

Closes #6197
Added two options entratGroupId and entraGroupName to below commands
spo file roleassignement add
spo file roleassignement remove

@milanholemans
Copy link
Contributor

Thanks, we'll review it ASAP!

@Adam-it Adam-it added the hacktoberfest-accepted Accept for hacktoberfest, will merge later label Oct 29, 2024
@Adam-it
Copy link
Member

Adam-it commented Oct 29, 2024

@ktskumar I added the hacktoberfest-accepted label to this PR which means that this PR will count as done for the Hacktoberfest event. So if you participate in this event it will get you unblocked and it allows us to merge this PR later when we catch up 👍
Thanks for your support and awesome contribution 👏 You Rock 🤩

@Adam-it Adam-it self-assigned this Nov 20, 2024
Adam-it
Adam-it requested changes Nov 20, 2024
View reviewed changes
Copy link
Member

@Adam-it Adam-it left a comment

Choose a reason for hiding this comment

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

@ktskumar awesome work so far 👏👏👏👏
Let's do a bit of a clean up before we merge 🚀. You Rock 🤩

docs/docs/cmd/spo/file/file-roleassignment-add.mdx Outdated

`--groupName [groupName]`
: The group name of the SharePoint group Specify either `upn`, `groupName`, or `principalId` but not multiple.
: The group name of the SharePoint group Specify either `upn`, `groupName`, entraGroupId, entraGroupName, or `principalId` but not multiple. This option is only used for SharePoint Online groups.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
: The group name of the SharePoint group Specify either `upn`, `groupName`, entraGroupId, entraGroupName, or `principalId` but not multiple. This option is only used for SharePoint Online groups.
: The group name of the SharePoint group. Specify either `upn`, `groupName`, entraGroupId, entraGroupName, or `principalId` but not multiple. This option is only used for SharePoint Online groups.

docs/docs/cmd/spo/file/file-roleassignment-add.mdx Outdated
@@ -26,10 +26,16 @@ m365 spo file roleassignment add [options]
: The SharePoint Id of the principal. It may be either a user id or group id to add a role assignment for. Specify either `upn`, `groupName`, or `principalId` but not multiple.

`--upn [upn]`
: The upn/email of user to assign role to. Specify either `upn`, `groupName`, or `principalId` but not multiple.
: The upn/email of user to assign role to. Specify either `upn`, `groupName`, entraGroupId, entraGroupName, or `principalId` but not multiple.
Copy link
Member

Choose a reason for hiding this comment

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

we should wrap the option names in `` in the description.

Suggested change
: The upn/email of user to assign role to. Specify either `upn`, `groupName`, entraGroupId, entraGroupName, or `principalId` but not multiple.
: The upn/email of user to assign role to. Specify either `upn`, `groupName`, `entraGroupId`, `entraGroupName`, or `principalId` but not multiple.

This also applies to all other places below

docs/docs/cmd/spo/file/file-roleassignment-add.mdx Outdated

`--groupName [groupName]`
: The group name of the SharePoint group Specify either `upn`, `groupName`, or `principalId` but not multiple.
: The group name of the SharePoint group Specify either `upn`, `groupName`, entraGroupId, entraGroupName, or `principalId` but not multiple. This option is only used for SharePoint Online groups.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
: The group name of the SharePoint group Specify either `upn`, `groupName`, entraGroupId, entraGroupName, or `principalId` but not multiple. This option is only used for SharePoint Online groups.
: The group name of the SharePoint group Specify either `upn`, `groupName`, `entraGroupId`, `entraGroupName`, or `principalId` but not multiple. This option is only used for SharePoint Online groups.

docs/docs/cmd/spo/file/file-roleassignment-add.mdx Outdated
: The group name of the SharePoint group Specify either `upn`, `groupName`, entraGroupId, entraGroupName, or `principalId` but not multiple. This option is only used for SharePoint Online groups.

`--entraGroupId [entraGroupId]`
: ID of the Microsoft Entra group to add. Specify either `upn`, `groupName`, entraGroupId, entraGroupName, or `principalId` but not multiple.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
: ID of the Microsoft Entra group to add. Specify either `upn`, `groupName`, entraGroupId, entraGroupName, or `principalId` but not multiple.
: ID of the Microsoft Entra group to add. Specify either `upn`, `groupName`, `entraGroupId`, `entraGroupName`, or `principalId` but not multiple.

docs/docs/cmd/spo/file/file-roleassignment-add.mdx Outdated
: ID of the Microsoft Entra group to add. Specify either `upn`, `groupName`, entraGroupId, entraGroupName, or `principalId` but not multiple.

`--entraGroupName [entraGroupName]`
: Display name of the Microsoft Entra group to add. Specify either `upn`, `groupName`, entraGroupId, entraGroupName, or `principalId` but not multiple.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
: Display name of the Microsoft Entra group to add. Specify either `upn`, `groupName`, entraGroupId, entraGroupName, or `principalId` but not multiple.
: Display name of the Microsoft Entra group to add. Specify either `upn`, `groupName`, `entraGroupId`, `entraGroupName`, or `principalId` but not multiple.

docs/docs/cmd/spo/file/file-roleassignment-remove.mdx Outdated

`--groupName [groupName]`
: The group name of the SharePoint group Specify either `upn`, `groupName`, or `principalId` but not multiple.
: The group name of the SharePoint group Specify either `upn`, `groupName`, entraGroupId, entraGroupName, or `principalId` but not multiple. This option is only used for SharePoint Online groups.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
: The group name of the SharePoint group Specify either `upn`, `groupName`, entraGroupId, entraGroupName, or `principalId` but not multiple. This option is only used for SharePoint Online groups.
: The group name of the SharePoint group Specify either `upn`, `groupName`, `entraGroupId`, `entraGroupName`, or `principalId` but not multiple. This option is only used for SharePoint Online groups.

docs/docs/cmd/spo/file/file-roleassignment-remove.mdx Outdated
: The group name of the SharePoint group Specify either `upn`, `groupName`, entraGroupId, entraGroupName, or `principalId` but not multiple. This option is only used for SharePoint Online groups.

`--entraGroupId [entraGroupId]`
: ID of the Microsoft Entra group to add. Specify either `upn`, `groupName`, entraGroupId, entraGroupName, or `principalId` but not multiple.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
: ID of the Microsoft Entra group to add. Specify either `upn`, `groupName`, entraGroupId, entraGroupName, or `principalId` but not multiple.
: ID of the Microsoft Entra group to add. Specify either `upn`, `groupName`, `entraGroupId`, `entraGroupName`, or `principalId` but not multiple.

docs/docs/cmd/spo/file/file-roleassignment-remove.mdx Outdated
: ID of the Microsoft Entra group to add. Specify either `upn`, `groupName`, entraGroupId, entraGroupName, or `principalId` but not multiple.

`--entraGroupName [entraGroupName]`
: Display name of the Microsoft Entra group to add. Specify either `upn`, `groupName`, entraGroupId, entraGroupName, or `principalId` but not multiple.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
: Display name of the Microsoft Entra group to add. Specify either `upn`, `groupName`, entraGroupId, entraGroupName, or `principalId` but not multiple.
: Display name of the Microsoft Entra group to add. Specify either `upn`, `groupName`, `entraGroupId`, `entraGroupName`, or `principalId` but not multiple.

src/m365/spo/commands/file/file-roleassignment-remove.spec.ts Outdated
Comment on lines 330 to 333
sinon.stub(spo, 'ensureEntraGroup').withArgs(webUrl, graphGroup).resolves(entraGroupResponse);


sinon.stub(request, 'post').callsFake(async (opts) => {
Copy link
Member

Choose a reason for hiding this comment

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

let's clear out a bit of this space

Suggested change
sinon.stub(spo, 'ensureEntraGroup').withArgs(webUrl, graphGroup).resolves(entraGroupResponse);
sinon.stub(request, 'post').callsFake(async (opts) => {
sinon.stub(spo, 'ensureEntraGroup').withArgs(webUrl, graphGroup).resolves(entraGroupResponse);
sinon.stub(request, 'post').callsFake(async (opts) => {

src/m365/spo/commands/file/file-roleassignment-remove.ts Outdated
Comment on lines 165 to 167
principalId = entraSiteUser.Id;

}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
principalId = entraSiteUser.Id;
}
principalId = entraSiteUser.Id;
}

@Adam-it Adam-it marked this pull request as draft November 20, 2024 23:27
@Adam-it
Copy link
Member

Adam-it commented Nov 26, 2024

@pnp/cli-for-microsoft-365-maintainers due to my short brake I am unassigning myself from this one so that it may be processed by someone else

@Adam-it Adam-it removed their assignment Nov 26, 2024
@ktskumar ktskumar marked this pull request as ready for review December 21, 2024 08:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Adam-it Adam-it Adam-it requested changes

Assignees
No one assigned
Labels
hacktoberfest-accepted Accept for hacktoberfest, will merge later
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Enhance spo file roleassignment commands with Microsoft Entra groups
3 participants
@ktskumar @milanholemans @Adam-it