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.

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

Expose Agreements and Locale Fields in COM #2897

Merged
merged 18 commits into from
Mar 8, 2023

Conversation

ryfu-msft
Copy link
Contributor

@ryfu-msft ryfu-msft commented Jan 31, 2023

The changes in this PR expose all of the locale fields including agreements. Also provides a way for the COM caller to accept or reject package/source agreements. To avoid breaking existing usage, the default behavior for accepting source/package agreements has been set to true.

Changes:

  • Modifies InstallOptions to expose AcceptPackageAgreements(bool)
  • Modifies PackageCatalogReference to expose AcceptSourceAgreements(bool)
  • Modifies PackageVersionInfo to expose two methods for obtaining a CatalogPackageMetadata object which contains the locale fields (including agreements). GetCatalogPackageMetadata() with no parameters uses the default localization, while GetCatalogPackageMetadata(string locale) applies the specified locale and retrieves those locale fields.
    Tests:

Added tests to verify that all locale fields and agreements are outputted correctly.

Microsoft Reviewers: Open in CodeFlow

@ryfu-msft ryfu-msft requested a review from a team as a code owner January 31, 2023 18:29
Windows::Foundation::Collections::IVector<winrt::Microsoft::Management::Deployment::Documentation> m_documentations{ nullptr };
Windows::Foundation::Collections::IVector<hstring> m_tags{ nullptr };
#endif
};
Copy link
Contributor

Choose a reason for hiding this comment

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

PurchaseURL?
Moniker?

@ryfu-msft ryfu-msft marked this pull request as draft January 31, 2023 19:50
@ryfu-msft ryfu-msft marked this pull request as ready for review February 1, 2023 19:02
@ryfu-msft ryfu-msft requested a review from JohnMcPMS February 1, 2023 19:04
Copy link
Member

@JohnMcPMS JohnMcPMS left a comment

Choose a reason for hiding this comment

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

I haven't looked at the implementation really, was just reviewing the interface changes.

src/Microsoft.Management.Deployment/PackageVersionInfo.cpp Outdated Show resolved Hide resolved
@ryfu-msft ryfu-msft requested a review from JohnMcPMS February 8, 2023 18:11
@JohnMcPMS JohnMcPMS dismissed their stale review February 9, 2023 02:50

API looks good, but I haven't reviewed implementation.

var installResult = await this.packageManager.InstallPackageAsync(searchResult.CatalogPackage, installOptions);

// Assert
Assert.AreEqual(InstallResultStatus.CatalogError, installResult.Status);
Copy link
Contributor

Choose a reason for hiding this comment

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

Assert.AreEqual(InstallResultStatus.CatalogError, installResult.Status);

I don't think CatalogError should be returned if agreements were not accepted

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed to new error - 'PackageAgreementsNotAccepted'

src/AppInstallerCLIE2ETests/Interop/FindPackagesInterop.cs Outdated Show resolved Hide resolved
src/Microsoft.Management.Deployment/CatalogPackage.cpp Outdated Show resolved Hide resolved
src/Microsoft.Management.Deployment/CatalogPackage.h Outdated Show resolved Hide resolved
src/Microsoft.Management.Deployment/InstallOptions.h Outdated Show resolved Hide resolved
src/Microsoft.Management.Deployment/PackageManager.idl Outdated Show resolved Hide resolved
src/Microsoft.Management.Deployment/PackageAgreement.cpp Outdated Show resolved Hide resolved
src/Microsoft.Management.Deployment/Documentation.h Outdated Show resolved Hide resolved
src/Microsoft.Management.Deployment/Documentation.cpp Outdated Show resolved Hide resolved
src/Microsoft.Management.Deployment/PackageManager.idl Outdated Show resolved Hide resolved
src/Microsoft.Management.Deployment/PackageManager.idl Outdated Show resolved Hide resolved
src/Microsoft.Management.Deployment/Converters.h Outdated Show resolved Hide resolved
@ryfu-msft ryfu-msft merged commit 6966cf1 into microsoft:master Mar 8, 2023
@ryfu-msft ryfu-msft deleted the comAgreements branch February 29, 2024 18:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants