Skip to content

UpdateServicesServer, UpdateServicesApprovalRule: Fixing Products processing, verbose output#64

Open
MartinVokurek wants to merge 4 commits into
dsccommunity:mainfrom
MartinVokurek:Fix-Products
Open

UpdateServicesServer, UpdateServicesApprovalRule: Fixing Products processing, verbose output#64
MartinVokurek wants to merge 4 commits into
dsccommunity:mainfrom
MartinVokurek:Fix-Products

Conversation

@MartinVokurek

@MartinVokurek MartinVokurek commented Feb 12, 2021

Copy link
Copy Markdown

Pull Request (PR) description

  • Fixed UpdateServicesServer and UpdateServicesApprovalRule
    • Process multiple product categories with the same name correctly (e.g. "Windows Admin Center")
    • Verbose output of Products only displayed one product
  • Fixed verbose output of Languages in UpdateServiceServer
  • Fixed verbose output of WSUS server in UpdateServicesApprovalRule

This Pull Request (PR) fixes the following issues

Task list

  • Added an entry under the Unreleased section of the change log in the CHANGELOG.md.
    Entry should say what was changed, and how that affects users (if applicable).
  • Resource documentation added/updated in README.md.
  • Resource parameter descriptions added/updated in README.md, schema.mof
    and comment-based help.
  • Comment-based help added/updated.
  • Localization strings added/updated in all localization files as appropriate.
  • Examples appropriately added/updated.
  • Unit tests added/updated. See DSC Resource Testing Guidelines.
  • Integration tests added/updated (where possible). See DSC Resource Testing Guidelines.
  • New/changed code adheres to DSC Resource Style Guidelines and Best Practices.

This change is Reviewable

@MartinVokurek

Copy link
Copy Markdown
Author

I think I'll need a bit of advice on how to deal with all the failed checks, as they seem to be mostly unrelated to the changes that I made. Should I ignore them? Or do they need to be fixed first, before my changes can be accepted?

@daemenseth daemenseth left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Tested looks good

daemenseth
daemenseth previously approved these changes Apr 9, 2021

@daemenseth daemenseth left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Looks good. tested->ok

@daemenseth

Copy link
Copy Markdown

Project likes dead. How can we made progression in it that the pull request are approved?

@luzkenin

Copy link
Copy Markdown

Is this project abandoned?

@gaelcolas

Copy link
Copy Markdown
Member

No, but I need to be reminded (on slack) or see the notifications.
We also need more reviewers that can eventually help maintain the repo.
I'll try to have a look this weekend.

@TheBlackMini

Copy link
Copy Markdown

Curious if this PR will be merged?

@gaelcolas

Copy link
Copy Markdown
Member

@NicolasBn has this been included part of your merged changes?

@gaelcolas

Copy link
Copy Markdown
Member

@TheBlackMini or @luzkenin are you able to rebase this branch and re-submit a PR.

@NicolasBn

Copy link
Copy Markdown
Member

@gaelcolas Indeed. To cover issue #61, I need to update UpdateServicesApprovalRule resource.

@TheBlackMini

Copy link
Copy Markdown

So, @NicolasBn do I need to do anything?

@NicolasBn

Copy link
Copy Markdown
Member

@TheBlackMini I don't have any time before next week. If you don't want to wait, you can submit your PR :)

@NicolasBn

NicolasBn commented Feb 17, 2023

Copy link
Copy Markdown
Member

I worked on a solution here : Get-UpdateServicesDscProduct

I created a function to find all products based on value passed on Products property.

I 'just' need to use it in UpdateServicesApprovalRule, and adapt pester tests.
It was a few month ago. So I need times to reintegrate my code :)

@rlmass

rlmass commented Jun 15, 2026

Copy link
Copy Markdown

@gaelcolas @NicolasBn Any updates on getting this merged?

@Borgquite Borgquite left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hey,

There are some useful fixes here, but some are already implemented? Can you resubmit and rebase?

else
{
$Languages = $WsusConfiguration.GetEnabledUpdateLanguages()
$Languages = ($WsusConfiguration.GetEnabledUpdateLanguages()) -join ','

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This needs to be

$Languages = [String[]]$WsusConfiguration.GetEnabledUpdateLanguages()

As otherwise it's not returning a string array, but a single string joined by a comma

Write-Verbose -Message ($script:localizedData.WsusClassifications -f $Classifications)
Write-Verbose -Message $script:localizedData.GettingWsusProducts
if ($Products = @($WsusSubscription.GetUpdateCategories().Title))
if ($Products = (@($WsusSubscription.GetUpdateCategories().Title) | Sort-Object -Unique))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This change is already in the main branch - you may need to rebase?

if ($Products = (@($WsusSubscription.GetUpdateCategories().Title) | Sort-Object -Unique))
{
if ($null -eq (Compare-Object -ReferenceObject ($Products | Sort-Object -Unique) -DifferenceObject `
if ($null -eq (Compare-Object -ReferenceObject $Products -DifferenceObject `

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ditto - need to rebase

if ($null -ne $WsusServer)
{
Write-Verbose -Message ('Identified WSUS server information: {0}' -f $WsusServer)
Write-Verbose -Message ('Identified WSUS server information: {0}' -f $WsusServer.Name)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is already in the Main branch - need to rebase?

Comment thread CHANGELOG.md

## [Unreleased]

### Fixed

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Need to update CHANGELOG as some changes are already implemented

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

8 participants