Mapping product names #1091

dagarwal-mitre · 2024-05-07T17:20:02Z

🗣 Description

Added product names to the table with with tenant licensing information
Closes #993

💭 Motivation and context

This helps users understand which licenses they have

🧪 Testing

Tested with the different tenants

✅ Pre-approval checklist

✅ Pre-merge checklist

PR passed smoke test check.
Feature branch has been rebased against changes from parent branch, as needed

Use Rebase branch button below or use this reference to rebase from the command line.
Resolved all merge conflicts on branch
Notified merge coordinator that PR is ready for merge via comment mention

✅ Post-merge checklist

Feature branch deleted after merge to clean up repository.
Verified that all checks pass on parent branch (e.g., main or release-name) after merge.

dagarwal-mitre · 2024-05-07T17:23:07Z

I am unsure where to put the product names csv file, currently it is in the ScubaGear folder, if it needs to be moved to a different location please reply to this comment.

gdasher · 2024-05-07T17:57:31Z

PowerShell/ScubaGear/Modules/CreateReport/CreateReport.psm1

+ $SkuID = $_.SkuId
+ # Find the corresponding product name
+ $matchingRow = $csvData | Where-Object { $_.GUID -eq $SkuID } | Select-Object -First 1
+ $productName = ""


For the case where we fail to find a match (e.g. a new SKU), lets use some language here instead of just blank. "Unknown SKU Name"?

gdasher · 2024-05-07T18:01:08Z

PowerShell/ScubaGear/Modules/CreateReport/CreateReport.psm1

+ $matchingRow = $csvData | Where-Object { $_.GUID -eq $SkuID } | Select-Object -First 1
+ $productName = ""
+ if ($matchingRow) {
+ $productName = $matchingRow.'Product_Display_Name'


Forgive my powershell ignorance, but does this properly select from the matching row object?

Would it be better to use camelCase headers in the CSV or is the CSF just a direct import from Microsoft's website?

This line will select from the matching row based on the SKU ID, but since it is using a direct import from microsoft's website I didn't do camel casing, in order to make it easier for future updates to directly import the csv rather than making changes to it. Currently the CSV file had a few columns removed which were unused, since the file original size was nearly 1MB and now it is closer to 450 kb. The goal was to make sure speed wasn't impacted, so I made the table size smaller.

gdasher · 2024-05-07T18:02:00Z

PowerShell/ScubaGear/Modules/CreateReport/CreateReport.psm1

 [pscustomobject]@{
+ "Product Name" = $productName


Some of the product names are quite long. Can you take a screenshot to show how the table looks now so we can see if it will look ok in practice, at least for commonly used SKUs?

This is currently how the table looks, and I believe of the 3 tenants, this has the longest product name

The longest product name in the current file is 103 characters, the one displayed is 68 characters with padding on either side given the static table width. Doing some quick HTML editing to drop that in still shows it fitting on one line.

gdasher · 2024-05-07T18:02:46Z

PowerShell/ScubaGear/Modules/CreateReport/CreateReport.psm1

@@ -210,8 +210,22 @@ function New-Report {

 # Handle AAD-specific reporting
 if ($BaselineName -eq "aad") {
+
+ # Load the CSV file
+ $csvData = Import-Csv -Path "Product Names for Licensing.csv"


Can we update the unit test for this case (e.g. by mocking out Import-Csv to avoid loading the file)? I think we added a decent test for this table IIRC.

tkol2022 · 2024-05-17T17:51:27Z

I am guessing that Microsoft updates the licensing plan data regularly. What is the plan to keep this information up to date in ScubaGear? Right now the new code is referencing a static CSV file.

gdasher · 2024-05-17T18:55:01Z

I am guessing that Microsoft updates the licensing plan data regularly. What is the plan to keep this information up to date in ScubaGear? Right now the new code is referencing a static CSV file.

Given the direct copy is used, we can likely generalize David Bui's github action to check for updates to this file as well Probably just be comparing hashes for changes and updating if so, for example?

Suggest we file an issue for that after this is committed.

Separately, please ping when the test cases have been updated and I will re-review for approval.

schrolla · 2024-05-20T13:54:35Z

I am guessing that Microsoft updates the licensing plan data regularly. What is the plan to keep this information up to date in ScubaGear? Right now the new code is referencing a static CSV file.

Given the direct copy is used, we can likely generalize David Bui's github action to check for updates to this file as well Probably just be comparing hashes for changes and updating if so, for example?

Suggest we file an issue for that after this is committed.

Separately, please ping when the test cases have been updated and I will re-review for approval.

Github automation is a good idea. My other thought was to add a new cmdlet to the tool that would reach out and grab the new version of the file directly and incorporate it into ScubaGear. That way, users could update the file directly outside the release cycle when needed if they had the necessary connectivity to do so. So I'd say, how about both as TODOs added beyond this issue?

dagarwal-mitre · 2024-05-20T14:08:20Z

I am guessing that Microsoft updates the licensing plan data regularly. What is the plan to keep this information up to date in ScubaGear? Right now the new code is referencing a static CSV file.

Given the direct copy is used, we can likely generalize David Bui's github action to check for updates to this file as well Probably just be comparing hashes for changes and updating if so, for example?
Suggest we file an issue for that after this is committed.
Separately, please ping when the test cases have been updated and I will re-review for approval.

Github automation is a good idea. My other thought was to add a new cmdlet to the tool that would reach out and grab the new version of the file directly and incorporate it into ScubaGear. That way, users could update the file directly outside the release cycle when needed if they had the necessary connectivity to do so. So I'd say, how about both as TODOs added beyond this issue?

2 comments/questions about this.

Currently the CSV File is not a direct copy of the CSV file from Microsoft, because the original file from Microsoft was about 1 MB and in an effort to reduce the size, I had removed the last few columns which we didn't use from the CSV file, but if size is not a concern, we can use a direct copy instead.
If we use a Github automation, couldn't we set it up so that it runs nightly, similar to the Nightly Testing that is run through a Github automation so there is always the newest version of the file and it wouldn't be tied to the release cycle.

schrolla

See notes about recommendations for CSV file suggestions.

schrolla · 2024-05-20T14:01:19Z

Product Names for Licensing.csv

This file should not be present in the top-level folder. Doing so means it would not be distributed in the PowerShell gallery package since it doesn't exist in the actual module. Recommend moving the file into the PowerShell/ScubaGear/Modules/CreateReport folder since that is the scope in which it is used.

schrolla · 2024-05-20T14:04:07Z

Product Names for Licensing.csv

Recommend renaming the file to remove all spaces from the filename and renaming to something more specific such as "MicrosoftLicenseToProductNameMappings.csv". This helps users better understand what the file is and why it exists and removing spaces makes it less likely there will be an error processing the file in code in case someone forgets to enclose the space filled name in quotes.

schrolla · 2024-05-20T14:05:08Z

Product Names for Licensing.csv

The file has headers for 3 fields, but all fields include an unnamed and empty fourth field (shown by the double commas on the end of each row). Is there a reason for the fourth field? If so, it should have a name in the header. If not, the extra comma at the end of every line should be removed.

Extra commas have been removed

schrolla · 2024-05-20T14:24:39Z

PowerShell/ScubaGear/Modules/CreateReport/CreateReport.psm1

 [pscustomobject]@{
+ "Product Name" = $productName


The longest product name in the current file is 103 characters, the one displayed is 68 characters with padding on either side given the static table width. Doing some quick HTML editing to drop that in still shows it fitting on one line.

gdasher · 2024-05-22T00:48:17Z

I am guessing that Microsoft updates the licensing plan data regularly. What is the plan to keep this information up to date in ScubaGear? Right now the new code is referencing a static CSV file.

Given the direct copy is used, we can likely generalize David Bui's github action to check for updates to this file as well Probably just be comparing hashes for changes and updating if so, for example?
Suggest we file an issue for that after this is committed.
Separately, please ping when the test cases have been updated and I will re-review for approval.

Github automation is a good idea. My other thought was to add a new cmdlet to the tool that would reach out and grab the new version of the file directly and incorporate it into ScubaGear. That way, users could update the file directly outside the release cycle when needed if they had the necessary connectivity to do so. So I'd say, how about both as TODOs added beyond this issue?

2 comments/questions about this.

Currently the CSV File is not a direct copy of the CSV file from Microsoft, because the original file from Microsoft was about 1 MB and in an effort to reduce the size, I had removed the last few columns which we didn't use from the CSV file, but if size is not a concern, we can use a direct copy instead.

If we use a Github automation, couldn't we set it up so that it runs nightly, similar to the Nightly Testing that is run through a Github automation so there is always the newest version of the file and it wouldn't be tied to the release cycle.

Ok, makes sense. I think its fine to drop columns we don't need so long as we do it in a standardized way (e.g. ImportFrom-Csv | Select-Object -Blah -Blah | ExportTo-Csv or the like in the automation script) and not a manual process.

Yes, if we do automation we can run it on a cron schedule and just update main periodically. That's how I would think to do it anyways.

dagarwal-mitre added 2 commits May 7, 2024 13:05

Added product name csv file

1bca371

Added column for product names

ce70565

dagarwal-mitre self-assigned this May 7, 2024

dagarwal-mitre requested review from schrolla and tkol2022 May 7, 2024 17:21

dagarwal-mitre requested review from gdasher and removed request for tkol2022 May 7, 2024 17:23

gdasher reviewed May 7, 2024

View reviewed changes

dagarwal-mitre added 3 commits May 7, 2024 14:05

updated case when no product name match

d409c78

updated smoke test

b399946

removed trailing white space

0b91dc9

schrolla added this to the Halibut milestone May 16, 2024

schrolla added the enhancement This issue or pull request will add new or improve existing functionality label May 20, 2024

schrolla requested changes May 20, 2024

View reviewed changes

dagarwal-mitre added 2 commits May 22, 2024 13:20

Changed CSV file location and name

e76d918

Removed extra commas

c00f1fe

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Mapping product names #1091

Mapping product names #1091

dagarwal-mitre commented May 7, 2024 •

edited

dagarwal-mitre commented May 7, 2024

gdasher May 7, 2024

dagarwal-mitre May 7, 2024

gdasher May 7, 2024

dagarwal-mitre May 7, 2024

gdasher May 7, 2024

dagarwal-mitre May 7, 2024

schrolla May 20, 2024

gdasher May 7, 2024

tkol2022 commented May 17, 2024

gdasher commented May 17, 2024

schrolla commented May 20, 2024

dagarwal-mitre commented May 20, 2024

schrolla left a comment

schrolla May 20, 2024

dagarwal-mitre May 22, 2024

schrolla May 20, 2024

dagarwal-mitre May 22, 2024

schrolla May 20, 2024

dagarwal-mitre May 22, 2024

schrolla May 20, 2024

gdasher commented May 22, 2024

Mapping product names #1091

Are you sure you want to change the base?

Mapping product names #1091

Conversation

dagarwal-mitre commented May 7, 2024 • edited

🗣 Description

💭 Motivation and context

🧪 Testing

✅ Pre-approval checklist

✅ Pre-merge checklist

✅ Post-merge checklist

dagarwal-mitre commented May 7, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tkol2022 commented May 17, 2024

gdasher commented May 17, 2024

schrolla commented May 20, 2024

dagarwal-mitre commented May 20, 2024

schrolla left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gdasher commented May 22, 2024

dagarwal-mitre commented May 7, 2024 •

edited