v1.1.0 - Create PDP XSD for IAP's, and add New-IapSubmissionPackage #11
Conversation
|
Hi @HowardWolosky-MSFT, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
TTYL, MSBOT; |
| ### Getting Your IapId | ||
|
|
||
| The next steps require you to know the IapId for the In-App Product ("add on") that you are trying | ||
| to use StoreBroker with. |
|
|
||
| You can read [PDP.md](PDP.md) for greater detail on PDP files. Right now, we just need to get you | ||
| started by generating your IAP's PDP files based on your current published (or pending) submission. | ||
|
|
There was a problem hiding this comment.
good catch. resolved.
|
|
||
| 2. Once you have the config, review all of the properties at the bottom half of the file. | ||
| These are the values for these properties as they are configured for your IAP in the store today. | ||
| Some users have realized that the values in the store (and thus in this file) are not what they |
There was a problem hiding this comment.
In step 1 you have "the Store" and in step 2 you have "the store". Should be consistent
There was a problem hiding this comment.
agreed. Fixed all instances to capitalized Store.
| expected, so it's worth checking them here and fixing them if need be. (If you do change any of | ||
| these properties, you'll need to use the appropriate _switch_ to the `Update-InAppProductSubmission` | ||
| later on to make sure that your changes are applied). | ||
|
|
There was a problem hiding this comment.
We could link to the section on using the Update- cmdlets
| As part of its input, `New-InAppProductSubmissionPackage` expects a configuration file, which | ||
| you should have [already created](SETUP.md#getting-your-iap-config). | ||
|
|
||
| ### IAP Commands |
There was a problem hiding this comment.
Line 588, "there exits an" --> "there exists an"
There was a problem hiding this comment.
good catch. fixed.
| * **`<productType>`** is either `Consumable` for a "Developer managed consumable", or | ||
| `Durable` for a "durable managed consumable". Please note that at this time, although the | ||
| Developer Web Portal supports the creation of "Store Managed Consumables", the Store Submission | ||
| API _does not_. | ||
| * **`<productId>`** is a unique name that you provide to refer to this IAP in this API and in your |
There was a problem hiding this comment.
Could you order these bullets according to the order they appear in the example command, or alphabetically? Right now it's neither. Are Consumable/Durable product types well known or explained earlier? If not, it would be helpful to have a link here.
There was a problem hiding this comment.
good feedback. Re-ordered to match the example, and then added a documentation link to further explain those two product types.
| .PARAMETER IapId | ||
| The ID of the IAP that the PDP's will be getting created for. | ||
| The most recent submission for this IAO will be used unless a SubmissionId is | ||
| explicitly specified. |
|
|
||
| # Import Write-Log | ||
| $rootDir = Split-Path -Path $PSScriptRoot -Parent | ||
| . $rootDir\StoreBroker\Helpers.ps1 |
There was a problem hiding this comment.
This assumes the script isn't moved out of the folder it came in. Do you care to add an existence check for the file?
There was a problem hiding this comment.
Good suggestion. Will do, here and in the other conversion script.
| $comment = $Xml.CreateComment(" $iconElementXml ") | ||
| $Xml.DocumentElement.RemoveChild($elementNode) | Out-Null | ||
| $Xml.DocumentElement.AppendChild($comment ) | Out-Null | ||
| } |
There was a problem hiding this comment.
Would be nice to explain this in the notes
There was a problem hiding this comment.
Added .NOTES to explain what's going on here.
| [string] $FileName | ||
| ) | ||
|
|
||
| $dropFolder = Join-Path $PdpRootPath $Lang |
There was a problem hiding this comment.
Full parameter names for Join-Path?
|
|
||
| # If there are no images being used at all, then we can also early return | ||
| $imageCount = 0; | ||
| foreach ($lang in $LangImageNames.GetEnumerator()) |
There was a problem hiding this comment.
nit: foreach ($lang in $LangImageNames.Values)
unless there is a specific reason you want it this way
There was a problem hiding this comment.
Won't fix. I think this might be personal opinion here, but I feel like it's more clear what's happening when using the KeyValuePair within the foreach as opposed to just the value.
|
|
||
| # Quick analysis to help teams out if they need to do anything special with their PDP's | ||
|
|
||
| $seenImages = $LangImageNames[$langs[0].Name] |
There was a problem hiding this comment.
Where did $langs come from? I don't see it in this function
There was a problem hiding this comment.
Nice catch. I missed this when I wrote ConvertFrom-ExistingSubmission. It's created in Main and is being treated as a global. I'll update the code to get the info directly from $LangImageNames instead.
|
|
||
| foreach ($image in $LangImageNames[$langs[$i].Name]) | ||
| { | ||
| if (-not $seenImages.Contains($image)) |
There was a problem hiding this comment.
nit: -contains operator
There was a problem hiding this comment.
I don't feel strongly either way here. Changed.
| else | ||
| { | ||
| $output = @() | ||
| $output += "Every language that has a PDP that references the following images:" |
|
|
||
|
|
||
| # function Main invocation | ||
| Main |
There was a problem hiding this comment.
No newline at end of file
| </xs:attribute> | ||
| </xs:attributeGroup> | ||
|
|
||
| <xs:attributeGroup name="ScreenshotImageGroup"> |
There was a problem hiding this comment.
right-o. Meant to remove that. Thanks.
| // The delimiter and any remaining text on that line will be removed. | ||
|
|
||
| { | ||
| // New-InAppProductSubmissionPackage parameters |
There was a problem hiding this comment.
4 space tabs, not 2
There was a problem hiding this comment.
VS had undone that. Fixed.
| // to the wrong product when working at the commandline. | ||
| // | ||
| // Ex: "iapId": "0ABCDEF12345" | ||
| "iapId": "", |
There was a problem hiding this comment.
not here, but noticed that the example for "appId" uses "AppId" instead of lowercase.
|
|
||
| "notesForCertification": "" | ||
| } | ||
| } |
There was a problem hiding this comment.
no newline at end of file
|
|
||
| $sub = Get-InAppProductSubmission -IapId $IapId -SubmissionId $submissionId | ||
|
|
||
| $updated = $updated -replace '"iapId": "",', "`"iapId`": `"$IapId`"," |
There was a problem hiding this comment.
Seems like there could be a simple function to do this work. Maybe you won't think it's worth the extra change.
There was a problem hiding this comment.
Won't fix. I don't think it's worth the refactoring work in this instance.
| $sourcePath = Join-Path -Path $PSScriptRoot -ChildPath $script:defaultIapConfigFileName | ||
|
|
||
| # Get-Content returns an array of lines.... using Out-String gives us back the linefeeds. | ||
| $template = (Get-Content $sourcePath) | Out-String |
There was a problem hiding this comment.
Get-Content needs Parameter Names. Also, add -Encoding utf8
There was a problem hiding this comment.
Fixed all instances.
| } | ||
| catch | ||
| { | ||
| throw |
There was a problem hiding this comment.
Why catch at all instead of letting it bubble?
There was a problem hiding this comment.
Fixed. For some reason, I recall that the behavior with the nested exceptions is that it reported the exception and then continued executing the next line. I just tested it now though, and that didn't happen. Not sure what I was seeing before at this point.
| } | ||
|
|
||
| Write-Log "Copying (Item: $sourcePath) to (Target: $Path)." -Level Verbose | ||
| Set-Content -Path $Path -Value $template -Force |
There was a problem hiding this comment.
Fixed all instances.
| @@ -204,7 +382,7 @@ function New-StoreBrokerConfigFile | |||
| New-Item -Force -ItemType Directory -Path $dir | Out-Null | |||
| } | |||
|
|
|||
| $sourcePath = Join-Path $PSScriptRoot $script:defaultConfigFileName | |||
| $sourcePath = Join-Path -Path $PSScriptRoot -ChildPath $script:defaultConfigFileName | |||
|
|
|||
| # Get-Content returns an array of lines.... using Out-String gives us back the linefeeds. | |||
| $template = (Get-Content $sourcePath) | Out-String | |||
There was a problem hiding this comment.
-Encoding and -Path here too
There was a problem hiding this comment.
Fixed all instances.
|
|
||
| .PARAMETER XmlFilePath | ||
| A full path to the localized .xml file to be parsed. | ||
|
|
| if (Test-Path -Path $imageContainerPath -PathType Container) | ||
| { | ||
| $image = Get-ChildItem -Recurse -File -Path $imageContainerPath -Include $imageFileName | Select-Object -First 1 | ||
|
|
There was a problem hiding this comment.
nit: I would remove this space since it directly relates to the previous line
| if ($null -eq $image) | ||
| { | ||
| Write-Log "Could not find image '$($imageFileName)' in any subdirectory of '$imageContainerPath'." -Level Error | ||
| throw "Halt Execution" |
There was a problem hiding this comment.
I saw elsewhere in this change you are just using "throw". Why use "Halt Execution" here and not there?
There was a problem hiding this comment.
Won't fix: throw by itself should be reserved for within a catch statement -- it means that it will re-throw the exception that was just caught, without altering it.
| containing localized data for the app (Title, Description, Icon, etc.) | ||
|
|
||
| .EXAMPLE | ||
| Convert-ListingsMetadata -PDPRootPath 'C:\PDP\' -PDPInclude 'InAppProductDescription.xml' -ImagesRootPath 'C:\IapIcons' |
| C:\PDP\language2\...\InAppProductDescription.xml | ||
| #> | ||
| [CmdletBinding()] | ||
| [OutputType([Hashtable])] |
There was a problem hiding this comment.
Seems like we should be consistent about using OutputType.
There was a problem hiding this comment.
I originally fixed all the instances that PSScriptAnalyzer was complaining about. I've opened #14 to track this suggestion.
| $submissionRequestBody.listings = Convert-ListingsMetadata @listingsResources | ||
| } | ||
|
|
||
| $submissionRequestBody = Remove-DeprecatedProperties $submissionRequestBody |
There was a problem hiding this comment.
need Parameter Name
| @@ -1340,6 +1874,9 @@ function Resolve-PackageParameters | |||
| Hashtable mapping the parameters of New-SubmissionPackage (except for ConfigPath) | |||
| to their provided values. | |||
|
|
|||
| .PARAMETER SkipValidation | |||
| An array of parameters that should this method should not attempt to be validate. | |||
| # If either 'PDPRootPath' or 'ImagesRootPath' is present, both must be present | ||
| if ((-not [String]::IsNullOrWhiteSpace($ParamMap[$script:s_PDPRootPath])) -xor | ||
| (-not [String]::IsNullOrWhiteSpace($ParamMap[$script:s_ImagesRootPath]))) | ||
| if (($SkipValidation -inotcontains $script:s_PDPRootPath) -and ($SkipValidation -inotcontains $script:s_ImagesRootPath)) |
There was a problem hiding this comment.
-notcontains is case insensitive by default. I'd rather use that much like how we use -like instead of -ilike
There was a problem hiding this comment.
Won't fix. While I do know that the comparisons are case-insensitive by default, I started getting into the habit of being explicit to improve code readability for those that don't realize that.
* Added new PDP schema and example file for IAP's and updated the corresonding PDP.md documentation. * Adds ConvertFrom-ExistingIapSubmission.ps1, mirroring ConvertFrom-ExistingSubmission.ps1, to auto-generate the IAP PDP set from an existing publishing or pending submission. * Added the ability to generate a new IAP StoreBroker configuration file based on a default config file, as well as based on a current submission for that IAP, similar to what we do for applications. * Added New-InAppProductSubmissionPackage to generate a StoreBroker payload from a config file and IAP PDP's (and optional icons). * Updated PackageTool's Resolve-PackageParameters helper method to optionally skip a list of parameters so that the same method could be used for both packaging commands since they're so similar, even though IAP's don't use appx packages. * Updated Update-InAppProductSubmission to do a similar iapId check of the commandline vs the payload's json value, similar to what we do for Update-ApplicationSubmission. * Updated SETUP.md and USAGE.md documentation to reflect these changes, and removed the note in README.md that this work hasn't been done yet. Additionally: * Fixed telemetry property typos for Iap's (referencing an Iap property instead of IapId), along with other AppId->IapId references for the IAP methods. * Fixed documentation example typo in ConvertFrom-ExistingSubmission.ps1 * Minor related Comment-Based-Help (CBH) changes to PackageTool.
corresonding PDP.md documentation.
ConvertFrom-ExistingSubmission.ps1, to auto-generate the IAP
PDP set from an existing publishing or pending submission.
file based on a default config file, as well as based on a
current submission for that IAP, similar to what we do for
applications.
payload from a config file and IAP PDP's (and optional icons).
optionally skip a list of parameters so that the same method could
be used for both packaging commands since they're so similar, even
though IAP's don't use appx packages.
of the commandline vs the payload's json value, similar to what we
do for Update-ApplicationSubmission.
and removed the note in README.md that this work hasn't been done yet.
Additionally:
instead of IapId), along with other AppId->IapId references for the
IAP methods.
Resolves #3 : Create PDP XSD for IAP's, and add New-IapSubmissionPackage