Fix check for pending package migration to use the package not the plan name (#19509)

* Fix check for pending package migration to use the package not plan name.

* Cover all package name/identifier permutations and fix the API output for multiple plans

* Adjusted log message to not refer to unattended migrations as migrations may be being run attended.

---------

Co-authored-by: Kenn Jacobsen <kja@umbraco.dk>

src/Umbraco.Cms.Api.Management/Controllers/Package/AllMigrationStatusPackageController.cs

@@ -1,8 +1,11 @@
 using Asp.Versioning;
 using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Mvc;
+using Microsoft.Extensions.DependencyInjection;
 using Umbraco.Cms.Api.Common.ViewModels.Pagination;
+using Umbraco.Cms.Api.Management.Factories;
 using Umbraco.Cms.Api.Management.ViewModels.Package;
+using Umbraco.Cms.Core.DependencyInjection;
 using Umbraco.Cms.Core.Mapping;
 using Umbraco.Cms.Core.Packaging;
 using Umbraco.Cms.Core.Services;
@@ -14,12 +17,25 @@ namespace Umbraco.Cms.Api.Management.Controllers.Package;
 public class AllMigrationStatusPackageController : PackageControllerBase
 {
     private readonly IPackagingService _packagingService;
-    private readonly IUmbracoMapper _umbracoMapper;
+    private readonly IPackagePresentationFactory _packagePresentationFactory;
 
+    [Obsolete("Please use the non-obsolete constructor. Scheduled for removal in V18.")]
     public AllMigrationStatusPackageController(IPackagingService packagingService, IUmbracoMapper umbracoMapper)
+        : this(packagingService, StaticServiceProvider.Instance.GetRequiredService<IPackagePresentationFactory>())
+    {
+    }
+
+    [Obsolete("Please use the non-obsolete constructor. Scheduled for removal in V18.")]
+    public AllMigrationStatusPackageController(IPackagingService packagingService, IUmbracoMapper umbracoMapper, IPackagePresentationFactory packagePresentationFactory)
+        : this(packagingService, packagePresentationFactory)
+    {
+    }
+
+    [ActivatorUtilitiesConstructor]
+    public AllMigrationStatusPackageController(IPackagingService packagingService, IPackagePresentationFactory packagePresentationFactory)
     {
         _packagingService = packagingService;
-        _umbracoMapper = umbracoMapper;
+        _packagePresentationFactory = packagePresentationFactory;
     }
 
     /// <summary>
@@ -38,11 +54,7 @@ public class AllMigrationStatusPackageController : PackageControllerBase
     {
         PagedModel<InstalledPackage> migrationPlans = await _packagingService.GetInstalledPackagesFromMigrationPlansAsync(skip, take);
 
-        var viewModel = new PagedViewModel<PackageMigrationStatusResponseModel>
-        {
-            Total = migrationPlans.Total,
-            Items = _umbracoMapper.MapEnumerable<InstalledPackage, PackageMigrationStatusResponseModel>(migrationPlans.Items)
-        };
+        PagedViewModel<PackageMigrationStatusResponseModel> viewModel = _packagePresentationFactory.CreatePackageMigrationStatusResponseModel(migrationPlans);
 
         return Ok(viewModel);
     }

src/Umbraco.Cms.Api.Management/Factories/IPackagePresentationFactory.cs

@@ -1,4 +1,6 @@
+using Umbraco.Cms.Api.Common.ViewModels.Pagination;
 using Umbraco.Cms.Api.Management.ViewModels.Package;
+using Umbraco.Cms.Core.Models;
 using Umbraco.Cms.Core.Packaging;
 
 namespace Umbraco.Cms.Api.Management.Factories;
@@ -8,4 +10,6 @@ public interface IPackagePresentationFactory
     PackageDefinition CreatePackageDefinition(CreatePackageRequestModel createPackageRequestModel);
 
     PackageConfigurationResponseModel CreateConfigurationResponseModel();
+
+    PagedViewModel<PackageMigrationStatusResponseModel> CreatePackageMigrationStatusResponseModel(PagedModel<InstalledPackage> installedPackages) => new();
 }

src/Umbraco.Cms.Api.Management/Factories/PackagePresentationFactory.cs

@@ -1,10 +1,12 @@
 using System.Collections.Specialized;
 using System.Web;
 using Microsoft.Extensions.Options;
+using Umbraco.Cms.Api.Common.ViewModels.Pagination;
 using Umbraco.Cms.Api.Management.ViewModels.Package;
 using Umbraco.Cms.Core;
 using Umbraco.Cms.Core.Configuration.Models;
 using Umbraco.Cms.Core.Mapping;
+using Umbraco.Cms.Core.Models;
 using Umbraco.Cms.Core.Packaging;
 using Umbraco.Cms.Core.Services;
 using Umbraco.Extensions;
@@ -42,6 +44,25 @@ internal class PackagePresentationFactory : IPackagePresentationFactory
             MarketplaceUrl = GetMarketplaceUrl(),
         };
 
+    public PagedViewModel<PackageMigrationStatusResponseModel> CreatePackageMigrationStatusResponseModel(PagedModel<InstalledPackage> installedPackages)
+    {
+        InstalledPackage[] installedPackagesAsArray = installedPackages.Items as InstalledPackage[] ?? installedPackages.Items.ToArray();
+
+        return new PagedViewModel<PackageMigrationStatusResponseModel>
+        {
+            Total = installedPackages.Total,
+            Items = installedPackagesAsArray
+                .GroupBy(package => package.PackageName)
+                .Select(packages => packages.OrderByDescending(package => package.HasPendingMigrations).First())
+                .Select(package => new PackageMigrationStatusResponseModel
+                {
+                    PackageName = package.PackageName ?? string.Empty,
+                    HasPendingMigrations = package.HasPendingMigrations,
+                })
+                .ToArray(),
+        };
+    }
+
     private string GetMarketplaceUrl()
     {
         var uriBuilder = new UriBuilder(Constants.Marketplace.Url);

src/Umbraco.Cms.Api.Management/Mapping/Package/PackageViewModelMapDefinition.cs

@@ -58,6 +58,7 @@ public class PackageViewModelMapDefinition : IMapDefinition
     }
 
     // Umbraco.Code.MapAll
+    [Obsolete("Please use the IPackagePresentationFactory instead. Scheduled for removal in V18.")]
     private static void Map(InstalledPackage source, PackageMigrationStatusResponseModel target, MapperContext context)
     {
         if (source.PackageName is not null)

src/Umbraco.Infrastructure/Install/PackageMigrationRunner.cs

@@ -77,8 +77,9 @@ public class PackageMigrationRunner
     /// </summary>
     public async Task<Attempt<bool, PackageMigrationOperationStatus>> RunPendingPackageMigrations(string packageName)
     {
-        // Check if there are any migrations
-        if (_packageMigrationPlans.ContainsKey(packageName) == false)
+        // Check if there are any migrations (note that the key for _packageMigrationPlans is the migration plan name, not the package name).
+        if (_packageMigrationPlans.Values
+            .Any(x => x.PackageName.InvariantEquals(packageName)) is false)
         {
             return Attempt.FailWithStatus(PackageMigrationOperationStatus.NotFound, false);
         }
@@ -121,8 +122,8 @@ public class PackageMigrationRunner
             }
 
             using (_profilingLogger.TraceDuration<PackageMigrationRunner>(
-                       "Starting unattended package migration for " + migrationName,
-                       "Unattended upgrade completed for " + migrationName))
+                       "Starting package migration for " + migrationName,
+                       "Package migration completed for " + migrationName))
             {
                 Upgrader upgrader = new(plan);
 

src/Umbraco.Infrastructure/Services/Implement/PackagingService.cs

@@ -314,8 +314,9 @@ public class PackagingService : IPackagingService
     /// <inheritdoc/>
     public Task<PagedModel<InstalledPackage>> GetInstalledPackagesFromMigrationPlansAsync(int skip, int take)
     {
-        IReadOnlyDictionary<string, string?>? keyValues =
-            _keyValueService.FindByKeyPrefix(Constants.Conventions.Migrations.KeyValuePrefix);
+        IReadOnlyDictionary<string, string?> keyValues =
+            _keyValueService.FindByKeyPrefix(Constants.Conventions.Migrations.KeyValuePrefix)
+            ?? new Dictionary<string, string?>();
 
         InstalledPackage[] installedPackages = _packageMigrationPlans
             .GroupBy(plan => (plan.PackageName, plan.PackageId))
@@ -326,15 +327,21 @@ public class PackagingService : IPackagingService
                     PackageName = group.Key.PackageName,
                 };
 
-                var packageKey = Constants.Conventions.Migrations.KeyValuePrefix + (group.Key.PackageId ?? group.Key.PackageName);
-                var currentState = keyValues?
-                    .GetValueOrDefault(packageKey);
-
                 package.PackageMigrationPlans = group
-                    .Select(plan => new InstalledPackageMigrationPlans
+                    .Select(plan =>
                     {
-                        CurrentMigrationId = currentState,
-                        FinalMigrationId = plan.FinalState,
+                        // look for migration states in this order:
+                        // - plan name
+                        // - package identifier
+                        // - package name
+                        var currentState =
+                            keyValues.GetValueOrDefault($"{Constants.Conventions.Migrations.KeyValuePrefix}{plan.Name}")
+                            ?? keyValues.GetValueOrDefault($"{Constants.Conventions.Migrations.KeyValuePrefix}{plan.PackageId ?? plan.PackageName}");
+
+                        return new InstalledPackageMigrationPlans
+                        {
+                            CurrentMigrationId = currentState, FinalMigrationId = plan.FinalState,
+                        };
                     });
 
                 return package;

tests/Umbraco.Tests.Integration/CompatibilitySuppressions.xml

@@ -0,0 +1,11 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!-- https://learn.microsoft.com/dotnet/fundamentals/package-validation/diagnostic-ids -->
+<Suppressions xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xsd="http://www.w3.org/2001/XMLSchema">
+  <Suppression>
+    <DiagnosticId>CP0002</DiagnosticId>
+    <Target>M:Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services.ContentEditingServiceTests.Updating_Single_Variant_Name_Does_Not_Change_Update_Dates_Of_Other_Vaiants</Target>
+    <Left>lib/net9.0/Umbraco.Tests.Integration.dll</Left>
+    <Right>lib/net9.0/Umbraco.Tests.Integration.dll</Right>
+    <IsBaselineSuppression>true</IsBaselineSuppression>
+  </Suppression>
+</Suppressions>