pnp / cli-microsoft365

Manage Microsoft 365 and SharePoint Framework projects on any platform
https://aka.ms/cli-m365
MIT License
920 stars 325 forks source link

Implement more purview commands #4351

Open nicodecleyre opened 1 year ago

nicodecleyre commented 1 year ago

Now that we are working on retention labels it also seems interesting to look at information protection and implement these commands:

More info

These apis for sensitivity labels & sensitivity label policy are currently only available in beta, so these should contain a remark in the documentation

waldekmastykarz commented 1 year ago

If I recall it correctly, @martinlingstuyl suggested that we split sensitivitylabel into two words sensitivity and label, similarly to retention label, right @martinlingstuyl?

martinlingstuyl commented 1 year ago

Well, that's an open discussion @waldekmastykarz. I'm not quite sure.

this is the other thread where we're talking about this

https://github.com/pnp/cli-microsoft365/issues/4145#issuecomment-1381475858

martinlingstuyl commented 1 year ago

Thinking about it: While sensitivity is not really a thing without the word label, the other words tend to get too long. And I think it is clear what is meant.

Im for expanding in the direction of

purview sensitivity label
purview sensitivity policy 
purview retention label
purview retention eventtype
purview retention event
purview threatassessment 

The last one can hardly be shortened, so I'd say keep it like this.

nicodecleyre commented 1 year ago

The retention label commands that we use are not splitted IMG_20230115_023638.jpg, the same goes for other commands like for example m365 pp aibuildermodel get. Also every split feels like it should be in a new folder in our structure.

I understand the suggestion to split the commands, but for consistency and ease of maintenance, I would suggest to keep the command structure as it is.

martinlingstuyl commented 1 year ago

They are currently not split indeed, but the point is: when looking ahead, will there come a moment when you think: *^^%, we should have split them.

In this case because purview has a lot of 'sections' with completely different functionality, like information protection and record management. And because the other retention commands might need a very long name, like retentioneventtype, if you leave them below the purview root.

waldekmastykarz commented 1 year ago

We only split command names into separate words, when the word describes a group of commands. For m365 pp aibuildermodel get, will we have multiple m365 pp ai commands, or m365 pp aibuilder commands? If not, then we keep aibuildermodel as a single group to make it easier to discover and navigate commands' hierarchy.

waldekmastykarz commented 1 year ago

Im for expanding in the direction of

purview sensitivity label
purview sensitivity policy 
purview retention label
purview retention eventtype
purview retention event
purview threatassessment 

The last one can hardly be shortened, so I'd say keep it like this.

In favor. Shall we add an issue to track this and get this done before we release the current version where we introduced these commands?

martinlingstuyl commented 1 year ago

We already released a few, so this will be refactor and add aliasses. I'll create the issue. 👍

martinlingstuyl commented 1 year ago

In favor. Shall we add an issue to track this and get this done

Slight side note @waldekmastykarz, @nicodecleyre : this would only change on the purview end. The spo commands for applying retention labels would still be called the same, agreed?

spo listitem retentionlabel ensure

Etc

In the future we might also add:

spo web sensitivitylabel ensure

So these command nouns will diverge from the purview management nouns as there's no reason to split the noun here.

nicodecleyre commented 1 year ago

Sounds good to me!

waldekmastykarz commented 1 year ago

@martinlingstuyl it seems odd to me to name the same things differently in different places. I suggest that we choose one convention and use it consistently throughout.

martinlingstuyl commented 1 year ago

Hi @waldekmastykarz, I see what you mean. That would mean we're back to using:

purview retentionlabel <verb>
purview retentioneventtype <verb>
purview sensitivitylabel <verb>
purview sensitivitylabel policy <verb>

Ideas?

waldekmastykarz commented 1 year ago

If breaking up sensitivitylabel for spo commands doesn't make sense, then indeed let's use the naming you've just suggested.

martinlingstuyl commented 1 year ago

Hi @nicodecleyre, this epic is OK. If you like you can spec some issues out. For inspiration you can checkout the issues that where created for the purview retentionlabel commands in epic #4145.

nicodecleyre commented 1 year ago

Hi @nicodecleyre, this epic is OK. If you like you can spec some issues out. For inspiration you can checkout the issues that where created for the purview retentionlabel commands in epic #4145.

Thanks for the info @martinlingstuyl, will spec out some issues shortly!