From b5f260c19f46381d47ad92f7954cabfae44ac6cc Mon Sep 17 00:00:00 2001 From: Jonas Dyrlie Date: Mon, 17 Feb 2025 09:53:43 +0100 Subject: [PATCH] refactor(pages-api): change parameter for layout operations to just use layout name Remove file extension formatting on layoutName parameter of service repository layer methods in AltinnAppGitRepository. This makes the method more consistent with the other methods and follows layer responsibilities (service layer does not need to know or specify file extension). commit-id:cf35ff23 --- .../ComponentIdChangedLayoutsHandler.cs | 4 ++- .../GitRepository/AltinnAppGitRepository.cs | 25 ++++++++++--------- .../Implementation/AppDevelopmentService.cs | 12 ++++----- .../Services/Implementation/TextsService.cs | 2 +- .../GetOptionListsReferencesTests.cs | 8 +++--- .../AltinnAppGitRepositoryTests.cs | 6 ++--- .../Services/OptionsServiceTests.cs | 8 +++--- .../Services/TextsServiceTest.cs | 4 +-- 8 files changed, 35 insertions(+), 34 deletions(-) diff --git a/backend/src/Designer/EventHandlers/ComponentIdChanged/ComponentIdChangedLayoutsHandler.cs b/backend/src/Designer/EventHandlers/ComponentIdChanged/ComponentIdChangedLayoutsHandler.cs index 178d940b925..3393bbbb808 100644 --- a/backend/src/Designer/EventHandlers/ComponentIdChanged/ComponentIdChangedLayoutsHandler.cs +++ b/backend/src/Designer/EventHandlers/ComponentIdChanged/ComponentIdChangedLayoutsHandler.cs @@ -42,8 +42,10 @@ await _fileSyncHandlerExecutor.ExecuteWithExceptionHandlingAndConditionalNotific { bool hasChanges = false; string[] layoutNames = repository.GetLayoutNames(notification.LayoutSetName); - foreach (var layoutName in layoutNames) + foreach (var layoutFileName in layoutNames) { + + string layoutName = layoutFileName.Replace(".json", ""); var layout = await repository.GetLayout(notification.LayoutSetName, layoutName, cancellationToken); if (TryChangeComponentId(layout, notification.OldComponentId, notification.NewComponentId)) { diff --git a/backend/src/Designer/Infrastructure/GitRepository/AltinnAppGitRepository.cs b/backend/src/Designer/Infrastructure/GitRepository/AltinnAppGitRepository.cs index 56625ff06c6..c395a1fec0c 100644 --- a/backend/src/Designer/Infrastructure/GitRepository/AltinnAppGitRepository.cs +++ b/backend/src/Designer/Infrastructure/GitRepository/AltinnAppGitRepository.cs @@ -60,11 +60,11 @@ public class AltinnAppGitRepository : AltinnGitRepository private const string SchemaFilePatternJson = "*.schema.json"; private const string SchemaFilePatternXsd = "*.xsd"; - public static readonly string InitialLayoutFileName = "Side1.json"; + public static readonly string InitialLayoutFileName = "Side1"; public readonly JsonNode InitialLayout = new JsonObject { ["$schema"] = LayoutSchemaUrl, ["data"] = new JsonObject { ["layout"] = new JsonArray([]) } }; - public readonly JsonNode InitialLayoutSettings = new JsonObject { ["$schema"] = LayoutSettingsSchemaUrl, ["pages"] = new JsonObject { ["order"] = new JsonArray([InitialLayoutFileName.Replace(".json", "")]) } }; + public readonly JsonNode InitialLayoutSettings = new JsonObject { ["$schema"] = LayoutSettingsSchemaUrl, ["pages"] = new JsonObject { ["order"] = new JsonArray([InitialLayoutFileName]) } }; private static readonly JsonSerializerOptions JsonOptions = new() { @@ -370,10 +370,11 @@ public async Task> GetFormLayouts(string layoutSetN cancellationToken.ThrowIfCancellationRequested(); Dictionary formLayouts = new(); string[] layoutNames = GetLayoutNames(layoutSetName); - foreach (string layoutName in layoutNames) + foreach (string layoutFileName in layoutNames) { + string layoutName = layoutFileName.Replace(".json", ""); JsonNode layout = await GetLayout(layoutSetName, layoutName, cancellationToken); - formLayouts[layoutName.Replace(".json", "")] = layout; + formLayouts[layoutName] = layout; } return formLayouts; @@ -479,7 +480,7 @@ public string[] GetLayoutNames(string layoutSetName) { foreach (string layoutPath in GetFilesByRelativeDirectory(layoutSetPath)) { - layoutNames.Add(Path.GetFileName(layoutPath)); + layoutNames.Add(Path.GetFileNameWithoutExtension(layoutPath)); } } @@ -652,11 +653,11 @@ public async Task SaveLayout(string layoutSetName, string layoutFileName, JsonNo await WriteTextByRelativePathAsync(layoutFilePath, serializedLayout, true, cancellationToken); } - public void UpdateFormLayoutName(string layoutSetName, string layoutFileName, string newFileName) + public void UpdateFormLayoutName(string layoutSetName, string layoutName, string newLayoutName) { - string currentFilePath = GetPathToLayoutFile(layoutSetName, layoutFileName); - string newFilePath = GetPathToLayoutFile(layoutSetName, newFileName); - MoveFileByRelativePath(currentFilePath, newFilePath, newFileName); + string currentFilePath = GetPathToLayoutFile(layoutSetName, layoutName); + string newFilePath = GetPathToLayoutFile(layoutSetName, newLayoutName); + MoveFileByRelativePath(currentFilePath, newFilePath, newLayoutName); } public async Task GetLayoutSetsFile(CancellationToken cancellationToken = default) @@ -1067,11 +1068,11 @@ private static string GetPathToLayoutSet(string layoutSetName, bool excludeLayou } // can be null if app does not use layout set - private static string GetPathToLayoutFile(string layoutSetName, string fileName) + private static string GetPathToLayoutFile(string layoutSetName, string layoutName) { return string.IsNullOrEmpty(layoutSetName) ? - Path.Combine(LayoutsFolderName, LayoutsInSetFolderName, fileName) : - Path.Combine(LayoutsFolderName, layoutSetName, LayoutsInSetFolderName, fileName); + Path.Combine(LayoutsFolderName, LayoutsInSetFolderName, $"{layoutName}.json") : + Path.Combine(LayoutsFolderName, layoutSetName, LayoutsInSetFolderName, $"{layoutName}.json"); } // can be null if app does not use layout set diff --git a/backend/src/Designer/Services/Implementation/AppDevelopmentService.cs b/backend/src/Designer/Services/Implementation/AppDevelopmentService.cs index 065d53c7978..c34096f0882 100644 --- a/backend/src/Designer/Services/Implementation/AppDevelopmentService.cs +++ b/backend/src/Designer/Services/Implementation/AppDevelopmentService.cs @@ -68,7 +68,6 @@ public async Task SaveFormLayout(AltinnRepoEditingContext altinnRepoEditingConte string layoutName, JsonNode formLayout, CancellationToken cancellationToken = default) { cancellationToken.ThrowIfCancellationRequested(); - string layoutFileName = $"{layoutName}.json"; AltinnAppGitRepository altinnAppGitRepository = _altinnGitRepositoryFactory.GetAltinnAppGitRepository(altinnRepoEditingContext.Org, altinnRepoEditingContext.Repo, altinnRepoEditingContext.Developer); @@ -79,14 +78,13 @@ public async Task SaveFormLayout(AltinnRepoEditingContext altinnRepoEditingConte "This app uses layout sets, but no layout set name was provided for this request"); } - await altinnAppGitRepository.SaveLayout(layoutSetName, layoutFileName, formLayout, cancellationToken); + await altinnAppGitRepository.SaveLayout(layoutSetName, layoutName, formLayout, cancellationToken); } /// public void DeleteFormLayout(AltinnRepoEditingContext altinnRepoEditingContext, string layoutSetName, string layoutName) { - string layoutFileName = $"{layoutName}.json"; AltinnAppGitRepository altinnAppGitRepository = _altinnGitRepositoryFactory.GetAltinnAppGitRepository(altinnRepoEditingContext.Org, altinnRepoEditingContext.Repo, altinnRepoEditingContext.Developer); @@ -97,7 +95,7 @@ public void DeleteFormLayout(AltinnRepoEditingContext altinnRepoEditingContext, "This app uses layout sets, but no layout set name was provided for this request"); } - altinnAppGitRepository.DeleteLayout(layoutSetName, layoutFileName); + altinnAppGitRepository.DeleteLayout(layoutSetName, layoutName); } /// @@ -114,7 +112,7 @@ public void UpdateFormLayoutName(AltinnRepoEditingContext altinnRepoEditingConte "This app uses layout sets, but no layout set name was provided for this request"); } - altinnAppGitRepository.UpdateFormLayoutName(layoutSetName, $"{layoutName}.json", $"{newName}.json"); + altinnAppGitRepository.UpdateFormLayoutName(layoutSetName, layoutName, newName); } /// @@ -586,7 +584,7 @@ public async Task AddComponentToLayout( editingContext.Repo, editingContext.Developer ); - JsonNode formLayout = await altinnAppGitRepository.GetLayout(layoutSetName, $"{layoutName}.json", cancellationToken); + JsonNode formLayout = await altinnAppGitRepository.GetLayout(layoutSetName, layoutName, cancellationToken); if (formLayout["data"] is not JsonObject data || data["layout"] is not JsonArray layoutArray) { throw new InvalidOperationException("Invalid form layout structure"); @@ -802,7 +800,7 @@ private async Task UpdateLayoutReferences(AltinnAppGitRepository altinnApp if (hasLayoutChanges) { - await altinnAppGitRepository.SaveLayout(layoutSet.Id, $"{layout.Key}.json", layout.Value, cancellationToken); + await altinnAppGitRepository.SaveLayout(layoutSet.Id, layout.Key, layout.Value, cancellationToken); hasChanges = true; } } diff --git a/backend/src/Designer/Services/Implementation/TextsService.cs b/backend/src/Designer/Services/Implementation/TextsService.cs index 294f418da7a..aa0ba420fa7 100644 --- a/backend/src/Designer/Services/Implementation/TextsService.cs +++ b/backend/src/Designer/Services/Implementation/TextsService.cs @@ -207,7 +207,7 @@ private async Task> UpdateKeysInLayoutsInLayoutSet(string org, stri if (hasMutated) { await altinnAppGitRepository.SaveLayout(layoutSetName, layoutName, layout); - updatedFiles.Add($"App/ui/{layoutSetName}/{layoutName}"); + updatedFiles.Add($"App/ui/{layoutSetName}/{layoutName}.json"); } } return updatedFiles.Count > 0 ? ["App/ui/layouts"] : []; diff --git a/backend/tests/Designer.Tests/Controllers/OptionsController/GetOptionListsReferencesTests.cs b/backend/tests/Designer.Tests/Controllers/OptionsController/GetOptionListsReferencesTests.cs index da8f3556b6b..1c3cf3769d0 100644 --- a/backend/tests/Designer.Tests/Controllers/OptionsController/GetOptionListsReferencesTests.cs +++ b/backend/tests/Designer.Tests/Controllers/OptionsController/GetOptionListsReferencesTests.cs @@ -41,19 +41,19 @@ public async Task GetOptionListsReferences_Returns200OK_WithValidOptionsReferenc new OptionListIdSource { ComponentIds = ["component-using-same-options-id-in-same-set-and-another-layout"], - LayoutName = "layoutWithOneOptionListIdRef.json", + LayoutName = "layoutWithOneOptionListIdRef", LayoutSetId = "layoutSet1" }, new OptionListIdSource { ComponentIds = ["component-using-test-options-id", "component-using-test-options-id-again"], - LayoutName = "layoutWithFourCheckboxComponentsAndThreeOptionListIdRefs.json", + LayoutName = "layoutWithFourCheckboxComponentsAndThreeOptionListIdRefs", LayoutSetId = "layoutSet1" }, new OptionListIdSource { ComponentIds = ["component-using-same-options-id-in-another-set"], - LayoutName = "layoutWithTwoOptionListIdRefs.json", + LayoutName = "layoutWithTwoOptionListIdRefs", LayoutSetId = "layoutSet2" } ] @@ -65,7 +65,7 @@ public async Task GetOptionListsReferences_Returns200OK_WithValidOptionsReferenc new OptionListIdSource { ComponentIds = ["component-using-other-options-id"], - LayoutName = "layoutWithFourCheckboxComponentsAndThreeOptionListIdRefs.json", + LayoutName = "layoutWithFourCheckboxComponentsAndThreeOptionListIdRefs", LayoutSetId = "layoutSet1" } ] diff --git a/backend/tests/Designer.Tests/Infrastructure/GitRepository/AltinnAppGitRepositoryTests.cs b/backend/tests/Designer.Tests/Infrastructure/GitRepository/AltinnAppGitRepositoryTests.cs index 594cffb2549..ff0ddb38fb8 100644 --- a/backend/tests/Designer.Tests/Infrastructure/GitRepository/AltinnAppGitRepositoryTests.cs +++ b/backend/tests/Designer.Tests/Infrastructure/GitRepository/AltinnAppGitRepositoryTests.cs @@ -200,7 +200,7 @@ public async Task GetLayout_WithAppThatUsesLayoutSet_ShouldReturnLayout() string repository = "app-with-layoutsets"; string developer = "testUser"; string layoutSetName = "layoutSet1"; - string layoutName = "layoutFile1InSet1.json"; + string layoutName = "layoutFile1InSet1"; AltinnAppGitRepository altinnAppGitRepository = PrepareRepositoryForTest(org, repository, developer); JsonNode formLayout = await altinnAppGitRepository.GetLayout(layoutSetName, layoutName); @@ -216,7 +216,7 @@ public async Task GetLayout_WithAppThatNotUsesLayoutSet_ShouldReturnLayout() string org = "ttd"; string repository = "app-without-layoutsets"; string developer = "testUser"; - string layoutName = "layoutFile1.json"; + string layoutName = "layoutFile1"; AltinnAppGitRepository altinnAppGitRepository = PrepareRepositoryForTest(org, repository, developer); JsonNode formLayout = await altinnAppGitRepository.GetLayout(null, layoutName); @@ -233,7 +233,7 @@ public async Task SaveLayout_ShouldUpdateLayoutInApp() string repository = "app-with-layoutsets"; string developer = "testUser"; string layoutSetName = "layoutSet1"; - string layoutName = "layoutFile2InSet1.json"; + string layoutName = "layoutFile2InSet1"; string targetRepository = TestDataHelper.GenerateTestRepoName(); try diff --git a/backend/tests/Designer.Tests/Services/OptionsServiceTests.cs b/backend/tests/Designer.Tests/Services/OptionsServiceTests.cs index 6404416c048..4ade5a72df5 100644 --- a/backend/tests/Designer.Tests/Services/OptionsServiceTests.cs +++ b/backend/tests/Designer.Tests/Services/OptionsServiceTests.cs @@ -268,19 +268,19 @@ public async Task GetAllOptionListReferences_ShouldReturnAllReferences_WhenOptio new OptionListIdSource { ComponentIds = ["component-using-same-options-id-in-same-set-and-another-layout"], - LayoutName = "layoutWithOneOptionListIdRef.json", + LayoutName = "layoutWithOneOptionListIdRef", LayoutSetId = "layoutSet1" }, new OptionListIdSource { ComponentIds = ["component-using-test-options-id", "component-using-test-options-id-again"], - LayoutName = "layoutWithFourCheckboxComponentsAndThreeOptionListIdRefs.json", + LayoutName = "layoutWithFourCheckboxComponentsAndThreeOptionListIdRefs", LayoutSetId = "layoutSet1" }, new OptionListIdSource { ComponentIds = ["component-using-same-options-id-in-another-set"], - LayoutName = "layoutWithTwoOptionListIdRefs.json", + LayoutName = "layoutWithTwoOptionListIdRefs", LayoutSetId = "layoutSet2" } ] @@ -292,7 +292,7 @@ public async Task GetAllOptionListReferences_ShouldReturnAllReferences_WhenOptio new OptionListIdSource { ComponentIds = ["component-using-other-options-id"], - LayoutName = "layoutWithFourCheckboxComponentsAndThreeOptionListIdRefs.json", + LayoutName = "layoutWithFourCheckboxComponentsAndThreeOptionListIdRefs", LayoutSetId = "layoutSet1" } ] diff --git a/backend/tests/Designer.Tests/Services/TextsServiceTest.cs b/backend/tests/Designer.Tests/Services/TextsServiceTest.cs index 7b33b52e19f..b2a8f85f1af 100644 --- a/backend/tests/Designer.Tests/Services/TextsServiceTest.cs +++ b/backend/tests/Designer.Tests/Services/TextsServiceTest.cs @@ -29,8 +29,8 @@ public class TextsServiceTest : IDisposable private const string developer = "testUser"; private const string layoutSetName1 = "layoutSet1"; private const string layoutSetName2 = "layoutSet2"; - private const string layoutName1 = "layoutFile1InSet1.json"; - private const string layoutName2 = "layoutFile1InSet2.json"; + private const string layoutName1 = "layoutFile1InSet1"; + private const string layoutName2 = "layoutFile1InSet2"; private async Task<(string targetRepository, AltinnGitRepositoryFactory altinnGitRepositoryFactory, TextsService textsService)> SetupRepository()