mmajcica / DeploySsrs

Build-Release task for VSTS/TFS that manages Microsoft's SQL Server Reporting Services
MIT License
21 stars 21 forks source link

DataSet name is still used instead of SharedDataSetReference when linking #69

Closed muad81 closed 3 years ago

muad81 commented 3 years ago

Hi,

First thanks for that useful extension! It provides all I need for efficient continuous integration with SSRS.

This issue is closely linked to . Correction would improve that shareddatasetreference matching.

I've been investigating why, having all datasets and data sources in the config file, I still have warnings like this :

2021-03-10T15:38:31.4605597Z ##[warning]The reference for dataset ListCountry can not be found.

The strange thing is that the dataset are most of the time correctly linked! Also, I've also noticed that some datasets where wrongly assigned. After investigation, it seems that it is due to the fact that I have some datasets in the config file with the same name as the datasetname (and not the SharedDataSetReference name).

Datasets are assigned twice to the reports in the PS New-SsrsReport function, first time in this code :

@($Datasets | Where-Object { $_.Name -eq $node.ParentNode.Name }) | ForEach-Object { $node.SharedDataSetReference = $_.Path } Second time by updating the references in this code :

                foreach($node in $nodes)
                {
                    $ds = $DataSets | Where-Object { $_.Name -eq $node.ParentNode.Name } | Select-Object -First 1

                    if ($ds)
                    {
                        $Reference = New-Object -TypeName SSRS.ReportingService2010.ItemReference
                        $Reference.Reference = $ds.Path
                        $Reference.Name = $ds.Name

                        $References += $Reference
                    }
                    else
                    {
                        Write-Warning "The reference for dataset $($node.ParentNode.Name) can not be found."
                    }
                }

                if ($References)
                {
                    $Proxy.SetItemReferences($report.Path, $References)
                }

This means that if I have a in the config file named X and in the report, a dataset name Y while the shareddatasetreference name is X, it will be correctly assigned by first lines of code while the second will generate a warning. Which is not correct and very confusing.

In another case, I could have a dataset X, dataset name X in the report and shareddatasetreference name Y, then the dataset X will be assigned, finaly, because first line will not match and second will work. Maybe I have both X and Y defined in my config file... then comes wrong assignment.

All that is pretty confusing, an I quess the best solution would be to use shareddatasetreference name in both methods.

GregoryOtt commented 3 years ago

This is linked with #52 I've done only the first part of the Job. I Will submit a new PR to complete the job in the next few days.

The two statements are required. For SSRS, a shared dataset must be:

  1. referenced by its path in the XML InnerText of node <SharedDataSetReference>
  2. explicitaly linked, by its path and the name in the report XML attribute's value Name of the node <DataSet>, in the ItemCatalog using $Proxy.SetItemReferences
muad81 commented 3 years ago

Can't push my changes no idea why but here is what I modified.

            if ($ReferenceDataSets -and $DataSets)
            {
                [SSRS.ReportingService2010.ItemReference[]]$References = $null
                $nodes = $Definition.SelectNodes('d:Report/d:DataSets/d:DataSet/d:SharedDataSet/d:SharedDataSetReference', $NsMgr)

                foreach($node in $nodes)
                {
                    $ds = $DataSets | Where-Object { $_.Name -eq ($node.InnerText.split("/") | Select-Object -Last 1) } | Select-Object -First 1

                    if ($ds)
                    {
                        $Reference = New-Object -TypeName SSRS.ReportingService2010.ItemReference
                        $Reference.Reference = $ds.Path
                        $Reference.Name = $node.ParentNode.ParentNode.Name

                        $References += $Reference
                    }
                    else
                    {
                        Write-Warning "The reference for dataset $(($node.InnerText.split("/") | Select-Object -Last 1)) can not be found."
                    }
                }

                if ($References)
                {
                    $Proxy.SetItemReferences($report.Path, $References)
                }
            }

I also fixed an issue that prevent the deployment of reports with special char in name:

return GetJsonFolderItems -Folder (Get-Content -Path $FilePath -Encoding utf8 | ConvertFrom-Json)

Could check that and push it if it fits?

muad81 commented 3 years ago

I tested it and it works fine in my devops.

mmajcica commented 3 years ago

I'm tried to publish the new version, seems that my pipeline is not working anymore. A quick fix didn't worked. I need some time tomorrow to figure that out, then a new version will publish all of the new fixes.

GregoryOtt commented 3 years ago

Can't push my changes no idea why but here is what I modified.

You need first to fork the repo then create a PR to be able to push. See: https://docs.github.com/en/github/collaborating-with-issues-and-pull-requests/creating-a-pull-request-from-a-fork

GregoryOtt commented 3 years ago

@muad81 you can check my PR to see if you're ok with it. @mmajcica are you able/ok to deploy a new version with this PR?

muad81 commented 3 years ago

Thank you very much guys! Hadn't got the time to test the new version but I unpublished my test one and will now rely on your latest @mmajcica. Sorry, for that copied version but couldn't find the option to keep it local and test it.

mmajcica commented 3 years ago

For the next time, you can publish it as private and then share it with your organization.

On Wed, 7 Apr 2021, 18:35 muad81, @.***> wrote:

Thank you very much guys! Hadn't got the time to test the new version but I unpublished my test one and will now rely on your latest @mmajcica https://github.com/mmajcica. Sorry, for that copied version but couldn't find the option to keep it local and test it.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mmajcica/DeploySsrs/issues/69#issuecomment-815056874, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAXC456M7N4PN3SPWRMZNYDTHSCWPANCNFSM4ZAIBKAA .