Cli-microsoft365: Update spo field remove to remove all field in a given group

[russgove]

Usage

spo field remove --webUrl https://contoso.sharepoint.com/sites/contoso-sales --group MyGroup

Description

Above command would remove all filed that are in the group MyGroup

[waldekmastykarz]

Nice suggestion! 👏

[plamber]

@waldekmastykarz assign it to me

[waldekmastykarz]

Appreciate your help! All yours!

[waldekmastykarz]

Hey @plamber, are you still working on this?

[plamber]

Have not started yet unfortunately. Planned to do it asap. Same holds for the other assigned tasks

[plamber]

@waldekmastykarz

In this specific case I have two solutions how to retrieve the site URL coming from the GroupId. I can use the Graph API using https://graph.microsoft.com/v1.0/groups/$groupId/sites/root/weburl or I could use the hidden list in the SharePoint Online admin catalog to retrieve the URL by filtering it using the GroupId.

I prefer using Graph in this case. This requires me to write some code in Graph not directly related to a command. The authentication is therefore failing.
What is the best approach to merge two operations that have different authentication requirements? Do we have already something else running?

Thank you,
Patrick

[waldekmastykarz]

I think the idea behind this command was to remove all fields belonging to the specified field group, not an Office 365 group. 😊

[plamber]

@waldekmastykarz

Requirements are clear now :/

I am almost done with the implementation. I just wanted to ask you a favor. I am struggling to find the reason why my branch test coverage is still reporting that I am missing some branches. I do not find the line in question.

I made a commit of the solution here: https://github.com/plamber/office365-cli/commits/spo-field-remove-group

Do you mind having a look on it? This is the last activity and we can push these changes for final review.

Thank you

[waldekmastykarz]

c8, our code coverage solution, doesn't properly support async methods which is why the reported coverage isn't accurate. To keep things simple and consistent, I'd suggest you used promises instead of async. Sorry for the trouble

[plamber]

@waldekmastykarz tried it out with a promise.all and have still the same issue.

Do you have a suggestion how to handle multiple commands and wait for all of them to complete until calling cb? I updated the code as reference

[waldekmastykarz]

Just to double check that I'm looking at the right question: your code is working and you'd like me to look at the coverage or your code isn't working as expected and you'd like me to have a look at that? 😊

[plamber]

Yes, the code coverage is still not ok. I can't find the issue

[waldekmastykarz]

Cool, I'll take a look

[waldekmastykarz]

There are two methods decorated with async:

https://github.com/plamber/office365-cli/blob/1555189c1220a85f67799f6362173b376b0ce0ce/src/o365/spo/commands/field/field-remove.ts#L57

and

https://github.com/plamber/office365-cli/blob/1555189c1220a85f67799f6362173b376b0ce0ce/src/o365/spo/commands/field/field-remove.ts#L112

This causes TypeScript to generate a helper that isn't fully covered.

Right now, we can't use async and need to use promises to maintain coverage and be consistent with the rest of the codebase.

[plamber]

@waldekmastykarz ahh... now I got it.

Would it be possible to subscribe to waldekAsADebuggingService? :)

Just pushed the changes for review

[waldekmastykarz]

Happy to help! 😄