-
Notifications
You must be signed in to change notification settings - Fork 74
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
feat: Update all Summary 2.0 components that has a reference to an updated component id #14372
Conversation
…elete-all-summary-20-components-that-has-a-reference-to-deleted-element-component-page-or-layout-set
…elete-all-summary-20-components-that-has-a-reference-to-deleted-element-component-page-or-layout-set
…elete-all-summary-20-components-that-has-a-reference-to-deleted-element-component-page-or-layout-set
This reverts commit f91bdbd.
…ummary-20-components-that-has-a-reference-to-deleted-element-component-page-or-layout-set' into 14037-automatically-update-all-summary-20-components-that-has-a-reference-to-updated-element-id-component-page-or-layout-set
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #14372 +/- ##
=======================================
Coverage 95.66% 95.66%
=======================================
Files 1891 1891
Lines 24583 24583
Branches 2823 2823
=======================================
Hits 23517 23517
Misses 805 805
Partials 261 261 ☔ View full report in Codecov by Sentry. |
…mponents-that-has-a-reference-to-deleted-element-component-page-or-layout-set
…mponents-that-has-a-reference-to-deleted-element-component-page-or-layout-set
…ummary-20-components-that-has-a-reference-to-deleted-element-component-page-or-layout-set' into 14037-automatically-update-all-summary-20-components-that-has-a-reference-to-updated-element-id-component-page-or-layout-set
Warning Rate limit exceeded@mlqn has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 10 minutes and 35 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe pull request introduces comprehensive changes to the Altinn Studio Designer's backend, focusing on enhancing layout and reference management capabilities. The modifications span multiple files and include adding support for cancellation tokens, implementing new event handlers for layout and component changes, and creating new event classes to facilitate reference updates across different application components. Changes
Possibly related PRs
Suggested Labels
Suggested Reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 18
🧹 Nitpick comments (23)
backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/layout2/layouts/Side1.json (1)
46-49
: Consider adding text resources for the back button.The NavigationButtons component has showBackButton enabled but no text resources defined. Consider adding text resources for better accessibility and internationalization support.
"id": "NavigationButtons-2hGPi1", "showBackButton": true, - "textResourceBindings": {}, + "textResourceBindings": { + "back": "navigation.back" + }, "type": "NavigationButtons"backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components/App/ui/component/layouts/Side2.json (1)
19-24
: Consider adding text resource bindings for better test coverage.The NavigationButtons component has empty textResourceBindings. Consider adding realistic text bindings to:
- Make the test data more representative of production scenarios
- Ensure proper testing of text resource resolution
- Validate accessibility requirements
backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components/App/ui/component2/layouts/Side1.json (1)
49-52
: Add text resources for navigation buttons.The NavigationButtons component has empty
textResourceBindings
. Consider adding text resources for better accessibility and user experience.backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/layout/layouts/Side1.json (1)
42-45
: Consider adding text resources for navigation buttons.The NavigationButtons component has empty textResourceBindings. Consider adding appropriate text resources for better accessibility and internationalization.
backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components/App/ui/layout/layouts/Side2.json (1)
12-17
: Consider adding text resources for accessibility.The NavigationButtons component has empty
textResourceBindings
. Consider adding text resources for better accessibility, especially since the back button is enabled.{ "id": "NavigationButtons-wDmNQu", "showBackButton": true, "textResourceBindings": { + "back": "back_button_text" }, "type": "NavigationButtons" }
backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-deleting-references/App/ui/layoutSet2/layouts/Side1.json (1)
46-51
: Consider adding text resources for navigation buttons.The NavigationButtons component has empty textResourceBindings. Consider adding text resources for better accessibility and user experience.
{ "id": "NavigationButtons-KnXA9y", "showBackButton": true, "textResourceBindings": { + "back": "back_button_text", + "next": "next_button_text" }, "type": "NavigationButtons" }backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/component2/layouts/Side1.json (1)
49-53
: Consider adding text resources for navigation buttons.The NavigationButtons component has empty
textResourceBindings
. Consider adding appropriate text resources for better accessibility and user experience, especially sinceshowBackButton
is enabled.backend/src/Designer/Services/Implementation/AppDevelopmentService.cs (1)
610-816
: Consider refactoringUpdateLayoutReferences
methodThe
UpdateLayoutReferences
method is lengthy and complex. Breaking it into smaller, well-named methods can improve readability and maintainability.🧰 Tools
🪛 GitHub Check: Run dotnet build and test (macos-latest)
[failure] 622-622:
Operator '==' cannot be applied to operands of type 'ReferenceType' and 'string'
[failure] 623-623:
Operator '==' cannot be applied to operands of type 'ReferenceType' and 'string'
[failure] 624-624:
Operator '==' cannot be applied to operands of type 'ReferenceType' and 'string'
[failure] 625-625:
Operator '==' cannot be applied to operands of type 'ReferenceType' and 'string'🪛 GitHub Check: Analyze
[failure] 622-622:
Operator '==' cannot be applied to operands of type 'ReferenceType' and 'string'
[failure] 623-623:
Operator '==' cannot be applied to operands of type 'ReferenceType' and 'string'
[failure] 624-624:
Operator '==' cannot be applied to operands of type 'ReferenceType' and 'string'
[failure] 625-625:
Operator '==' cannot be applied to operands of type 'ReferenceType' and 'string'🪛 GitHub Check: Run integration tests against actual gitea and db
[failure] 622-622:
Operator '==' cannot be applied to operands of type 'ReferenceType' and 'string'
[failure] 623-623:
Operator '==' cannot be applied to operands of type 'ReferenceType' and 'string'
[failure] 624-624:
Operator '==' cannot be applied to operands of type 'ReferenceType' and 'string'
[failure] 625-625:
Operator '==' cannot be applied to operands of type 'ReferenceType' and 'string'🪛 GitHub Check: Run dotnet build and test (ubuntu-latest)
[failure] 622-622:
Operator '==' cannot be applied to operands of type 'ReferenceType' and 'string'
[failure] 623-623:
Operator '==' cannot be applied to operands of type 'ReferenceType' and 'string'
[failure] 624-624:
Operator '==' cannot be applied to operands of type 'ReferenceType' and 'string'
[failure] 625-625:
Operator '==' cannot be applied to operands of type 'ReferenceType' and 'string'🪛 GitHub Actions: Playwright tests on pr
[error] 622-625: Multiple operator '==' errors: Cannot be applied to operands of type 'ReferenceType' and 'string'
backend/src/Designer/Events/LayoutSetDeletedEvent.cs (1)
7-10
: Consider adding nullable annotations to properties.Since C# 8.0 introduced nullable reference types, it would be beneficial to explicitly declare if these properties can be null. This helps prevent null reference exceptions and improves code clarity.
public class LayoutSetDeletedEvent : INotification { - public AltinnRepoEditingContext EditingContext { get; set; } - public string LayoutSetName { get; set; } + public required AltinnRepoEditingContext EditingContext { get; set; } + public required string LayoutSetName { get; set; } }backend/src/Designer/Events/ComponentDeletedEvent.cs (1)
6-11
: Add XML documentation and nullable annotations.
- Add XML documentation to describe the event's purpose and property usage
- Add nullable annotations to prevent potential null reference exceptions
+/// <summary> +/// Event published when a component is deleted, triggering updates to related layout references +/// </summary> public class ComponentDeletedEvent : INotification { - public AltinnRepoEditingContext EditingContext { get; set; } - public string LayoutSetName { get; set; } - public string ComponentId { get; set; } + /// <summary> + /// The context in which the component is being edited + /// </summary> + public required AltinnRepoEditingContext EditingContext { get; set; } + + /// <summary> + /// The name of the layout set containing the deleted component + /// </summary> + public required string LayoutSetName { get; set; } + + /// <summary> + /// The ID of the deleted component + /// </summary> + public required string ComponentId { get; set; } }backend/src/Designer/Events/LayoutSetIdChangedEvent.cs (2)
6-11
: Add XML documentation and consider property naming.
- Add XML documentation to describe the event's purpose
- Consider renaming
NewLayoutSetName
toUpdatedLayoutSetName
for clarity+/// <summary> +/// Event published when a layout set's identifier is changed +/// </summary> public class LayoutSetIdChangedEvent : INotification { - public AltinnRepoEditingContext EditingContext { get; set; } - public string LayoutSetName { get; set; } - public string NewLayoutSetName { get; set; } + /// <summary> + /// The context in which the layout set is being edited + /// </summary> + public required AltinnRepoEditingContext EditingContext { get; set; } + + /// <summary> + /// The original name of the layout set + /// </summary> + public required string LayoutSetName { get; set; } + + /// <summary> + /// The updated name of the layout set + /// </summary> + public required string UpdatedLayoutSetName { get; set; } }
1-1
: Architectural Review: Event System DesignThe event system is well-structured for managing layout references with good separation of concerns. Each event class has a clear, single responsibility. Consider creating a shared base class or interface for common properties like
EditingContext
to promote code reuse and ensure consistency.Example base class:
/// <summary> /// Base class for layout-related events /// </summary> public abstract class LayoutEventBase : INotification { /// <summary> /// The context in which the layout is being edited /// </summary> public required AltinnRepoEditingContext EditingContext { get; set; } }backend/src/Designer/EventHandlers/LayoutPageIdChanged/LayoutPageIdChangedLayoutsHandler.cs (1)
12-26
: Consider adding error handling for edge cases.While the implementation is good, consider adding validation for:
- Null or empty layout names
- Case where old and new layout names are identical
🧰 Tools
🪛 GitHub Check: Run dotnet build and test (macos-latest)
[failure] 22-22:
Argument 1: cannot convert from 'string' to 'Altinn.Studio.Designer.Enums.ReferenceType'🪛 GitHub Check: Analyze
[failure] 22-22:
Argument 1: cannot convert from 'string' to 'Altinn.Studio.Designer.Enums.ReferenceType'🪛 GitHub Check: Run integration tests against actual gitea and db
[failure] 22-22:
Argument 1: cannot convert from 'string' to 'Altinn.Studio.Designer.Enums.ReferenceType'🪛 GitHub Check: Run dotnet build and test (ubuntu-latest)
[failure] 22-22:
Argument 1: cannot convert from 'string' to 'Altinn.Studio.Designer.Enums.ReferenceType'🪛 GitHub Actions: Playwright tests on pr
[error] 22-22: Argument 1: cannot convert from 'string' to 'Altinn.Studio.Designer.Enums.ReferenceType'
backend/src/Designer/EventHandlers/LayoutSetDeleted/LayoutSetDeletedLayoutsHandler.cs (1)
13-27
: Consider introducing a base handler class.All four handlers share similar patterns for:
- Constructor injection
- Error handling with
ExecuteWithExceptionHandlingAndConditionalNotification
- Reference list creation and update
Consider creating a base class to reduce code duplication and enforce consistency.
Example base class structure:
public abstract class BaseLayoutReferenceHandler<TNotification> : INotificationHandler<TNotification> where TNotification : INotification { protected readonly IFileSyncHandlerExecutor FileSyncHandlerExecutor; protected readonly IAppDevelopmentService AppDevelopmentService; protected BaseLayoutReferenceHandler( IFileSyncHandlerExecutor fileSyncHandlerExecutor, IAppDevelopmentService appDevelopmentService) { FileSyncHandlerExecutor = fileSyncHandlerExecutor; AppDevelopmentService = appDevelopmentService; } public async Task Handle(TNotification notification, CancellationToken cancellationToken) { await FileSyncHandlerExecutor.ExecuteWithExceptionHandlingAndConditionalNotification( GetEditingContext(notification), GetSyncErrorCode(), "layouts", async () => { var references = CreateReferences(notification); return await AppDevelopmentService.UpdateLayoutReferences( GetEditingContext(notification), references, cancellationToken); }); } protected abstract AltinnRepoEditingContext GetEditingContext(TNotification notification); protected abstract SyncErrorCodes GetSyncErrorCode(); protected abstract List<Reference> CreateReferences(TNotification notification); }backend/src/Designer/Hubs/SyncHub/SyncErrorCodes.cs (1)
12-13
: LGTM! Consider adding XML documentation.The new error codes align well with the PR's objective of handling component ID updates. Consider adding XML documentation to describe when each error code is used.
public static class SyncErrorCodes { + /// <summary> + /// Error code for when layouts are deleted from a layout set. + /// </summary> public const string LayoutSetDeletedLayoutsSyncError = nameof(LayoutSetDeletedLayoutsSyncError); + /// <summary> + /// Error code for when layout set IDs are changed. + /// </summary> public const string LayoutSetIdChangedLayoutsSyncError = nameof(LayoutSetIdChangedLayoutsSyncError);Also applies to: 17-19
backend/tests/Designer.Tests/Controllers/AppDevelopmentController/DeleteFormLayoutTests.cs (1)
61-90
: LGTM! Test covers the Summary 2.0 component updates.The test effectively verifies that deleting a form layout also updates associated Summary 2.0 components.
Consider extracting the test data setup into a helper method for better maintainability:
+ private static readonly string[] LayoutPaths = [ + "layout/layouts/Side1.json", + "layout/layouts/Side2.json", + "layout2/layouts/Side1.json", + "layout2/layouts/Side2.json", + ]; + + private async Task VerifyLayoutFiles(string org, string app, string developer, string expectedApp) + { + LayoutPaths.ToList().ForEach(file => + { + string actual = TestDataHelper.GetFileFromRepo(org, app, developer, $"App/ui/{file}"); + string expected = TestDataHelper.GetFileFromRepo(org, expectedApp, developer, $"App/ui/{file}"); + JsonUtils.DeepEquals(actual, expected).Should().BeTrue(); + }); + }backend/tests/Designer.Tests/Controllers/AppDevelopmentController/UpdateFormLayoutNameTests.cs (1)
72-104
: LGTM! Test covers the Summary 2.0 component updates.The test effectively verifies that updating a form layout name also updates associated Summary 2.0 components.
Consider creating a shared test base class to reduce code duplication with
DeleteFormLayoutTests
:+public abstract class Summary2ComponentsTestBase : DesignerEndpointsTestsBase<Summary2ComponentsTestBase> +{ + protected static readonly string[] LayoutPaths = [ + "layout/layouts/Side1.json", + "layout/layouts/Side2.json", + "layout2/layouts/Side1.json", + "layout2/layouts/Side2.json", + ]; + + protected async Task VerifyLayoutFiles(string org, string app, string developer, string expectedApp) + { + LayoutPaths.ToList().ForEach(file => + { + string actual = TestDataHelper.GetFileFromRepo(org, app, developer, $"App/ui/{file}"); + string expected = TestDataHelper.GetFileFromRepo(org, expectedApp, developer, $"App/ui/{file}"); + JsonUtils.DeepEquals(actual, expected).Should().BeTrue(); + }); + } +}Then update both test classes to inherit from this base class:
-public class UpdateFormLayoutNameTests : DesignerEndpointsTestsBase<UpdateFormLayoutNameTests> +public class UpdateFormLayoutNameTests : Summary2ComponentsTestBasebackend/src/Designer/Services/Interfaces/IAppDevelopmentService.cs (1)
217-223
: Enhance XML documentation for the new method.The XML documentation should include a description of the return value.
Apply this diff to enhance the documentation:
/// <summary> /// Update layout references /// </summary> /// <param name="altinnRepoEditingContext">An <see cref="AltinnRepoEditingContext"/>.</param> /// <param name="referencesToDelete">The references to delete.</param> /// <param name="cancellationToken">An <see cref="CancellationToken"/> that observes if operation is cancelled.</param> +/// <returns>A <see cref="Task{Boolean}"/> that represents the asynchronous operation. The task result indicates whether any references were updated.</returns> public Task<bool> UpdateLayoutReferences(AltinnRepoEditingContext altinnRepoEditingContext, List<Reference> referencesToDelete, CancellationToken cancellationToken);
backend/src/Designer/Controllers/AppDevelopmentController.cs (1)
127-137
: Component deletion handling looks good.The implementation correctly publishes deletion events when components are removed, using the mediator pattern for loose coupling.
Consider extracting the component change handling logic to a separate private method for better readability:
-foreach (var componentIdChange in formLayoutPayload.ComponentIdsChange.Where((componentIdChange) => componentIdChange.OldComponentId != componentIdChange.NewComponentId)) -{ - if (componentIdChange.NewComponentId == null) - { - await _mediator.Publish(new ComponentDeletedEvent - { - ComponentId = componentIdChange.OldComponentId, - LayoutSetName = layoutSetName, - EditingContext = editingContext - }, cancellationToken); - } +private async Task HandleComponentChanges(FormLayoutPayload formLayoutPayload, string layoutSetName, AltinnRepoEditingContext editingContext, CancellationToken cancellationToken) +{ + foreach (var change in formLayoutPayload.ComponentIdsChange.Where(c => c.OldComponentId != c.NewComponentId)) + { + if (change.NewComponentId == null) + { + await _mediator.Publish(new ComponentDeletedEvent + { + ComponentId = change.OldComponentId, + LayoutSetName = layoutSetName, + EditingContext = editingContext + }, cancellationToken); + }backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/component/layouts/Side2.json (1)
9-9
: Review component ID naming convention.The component ID
Input-Om7N3y-new
uses a-new
suffix. Consider using a more standardized naming convention for component IDs.backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-deleting-references/App/ui/component/layouts/Side1.json (1)
1-49
: Consider adding test data documentation.These test files represent different states of Summary2 component updates. Consider adding a README.md to document:
- The purpose of each test file
- Expected state transitions
- Valid configuration patterns
backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/layoutSet2/layouts/Side1.json (1)
32-33
: Standardize the use of taskId across targets.Some targets include taskId while others don't. Consider standardizing this across all target configurations for consistency.
"target": { "type": "page", "id": "Side2", + "taskId": "" },
Also applies to: 41-42
backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components/App/ui/component/layouts/Side1.json (1)
1-64
: Consider implementing schema validation for target configurations.To maintain consistency across layouts and prevent potential issues:
- Define a strict schema for target configurations
- Enforce required properties (like taskId)
- Validate layoutSet IDs when they're required
This will help prevent inconsistencies and potential runtime issues.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (82)
backend/src/Designer/Controllers/AppDevelopmentController.cs
(6 hunks)backend/src/Designer/Controllers/ResourceAdminController.cs
(3 hunks)backend/src/Designer/Enums/ReferenceType.cs
(1 hunks)backend/src/Designer/Enums/ResourceReferenceSource.cs
(1 hunks)backend/src/Designer/Enums/ResourceReferenceType.cs
(1 hunks)backend/src/Designer/EventHandlers/ComponentDeleted/ComponentDeletedLayoutsHandler.cs
(1 hunks)backend/src/Designer/EventHandlers/ComponentIdChanged/ComponentIdChangedLayoutsHandler.cs
(3 hunks)backend/src/Designer/EventHandlers/LayoutPageDeleted/LayoutPageDeletedLayoutsHandler.cs
(1 hunks)backend/src/Designer/EventHandlers/LayoutPageIdChanged/LayoutPageIdChangedLayoutsHandler.cs
(1 hunks)backend/src/Designer/EventHandlers/LayoutSetDeleted/LayoutSetDeletedComponentRefHandler.cs
(0 hunks)backend/src/Designer/EventHandlers/LayoutSetDeleted/LayoutSetDeletedLayoutsHandler.cs
(1 hunks)backend/src/Designer/EventHandlers/LayoutSetIdChanged/LayoutSetIdChangedLayoutsHandler.cs
(1 hunks)backend/src/Designer/EventHandlers/ProcessTaskIdChanged/ProcessTaskIdChangedLayoutsHandler.cs
(1 hunks)backend/src/Designer/Events/ComponentDeletedEvent.cs
(1 hunks)backend/src/Designer/Events/LayoutPageDeletedEvent.cs
(1 hunks)backend/src/Designer/Events/LayoutPageIdChangedEvent.cs
(1 hunks)backend/src/Designer/Events/LayoutSetDeletedEvent.cs
(1 hunks)backend/src/Designer/Events/LayoutSetIdChangedEvent.cs
(1 hunks)backend/src/Designer/Hubs/SyncHub/SyncErrorCodes.cs
(1 hunks)backend/src/Designer/Models/Reference.cs
(1 hunks)backend/src/Designer/Models/ResourceReference.cs
(2 hunks)backend/src/Designer/Services/Implementation/AppDevelopmentService.cs
(2 hunks)backend/src/Designer/Services/Interfaces/IAppDevelopmentService.cs
(1 hunks)backend/tests/Designer.Tests/Controllers/AppDevelopmentController/DeleteFormLayoutTests.cs
(3 hunks)backend/tests/Designer.Tests/Controllers/AppDevelopmentController/DeleteLayoutSetTests.cs
(1 hunks)backend/tests/Designer.Tests/Controllers/AppDevelopmentController/SaveFormLayoutTests.cs
(2 hunks)backend/tests/Designer.Tests/Controllers/AppDevelopmentController/UpdateFormLayoutNameTests.cs
(1 hunks)backend/tests/Designer.Tests/Controllers/ResourceAdminController/ResourceAdminControllerTestsBaseClass.cs
(1 hunks)backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-deleting-references/App/ui/component/Settings.json
(1 hunks)backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-deleting-references/App/ui/component/layouts/Side1.json
(1 hunks)backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-deleting-references/App/ui/component/layouts/Side2.json
(1 hunks)backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-deleting-references/App/ui/component2/Settings.json
(1 hunks)backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-deleting-references/App/ui/component2/layouts/Side1.json
(1 hunks)backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-deleting-references/App/ui/component2/layouts/Side2.json
(1 hunks)backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-deleting-references/App/ui/layout-sets.json
(1 hunks)backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-deleting-references/App/ui/layout/Settings.json
(1 hunks)backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-deleting-references/App/ui/layout/layouts/Side1.json
(1 hunks)backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-deleting-references/App/ui/layout2/Settings.json
(1 hunks)backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-deleting-references/App/ui/layout2/layouts/Side1.json
(1 hunks)backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-deleting-references/App/ui/layout2/layouts/Side2.json
(1 hunks)backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-deleting-references/App/ui/layoutSet2/Settings.json
(1 hunks)backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-deleting-references/App/ui/layoutSet2/layouts/Side1.json
(1 hunks)backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-deleting-references/App/ui/layoutSet2/layouts/Side2.json
(1 hunks)backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/component/Settings.json
(1 hunks)backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/component/layouts/Side1.json
(1 hunks)backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/component/layouts/Side2.json
(1 hunks)backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/component2/Settings.json
(1 hunks)backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/component2/layouts/Side1.json
(1 hunks)backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/component2/layouts/Side2.json
(1 hunks)backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/layout-sets.json
(1 hunks)backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/layout/Settings.json
(1 hunks)backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/layout/layouts/Side1.json
(1 hunks)backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/layout/layouts/Side2-new.json
(1 hunks)backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/layout2/Settings.json
(1 hunks)backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/layout2/layouts/Side1.json
(1 hunks)backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/layout2/layouts/Side2.json
(1 hunks)backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/layoutSet-new/Settings.json
(1 hunks)backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/layoutSet-new/layouts/Side1.json
(1 hunks)backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/layoutSet-new/layouts/Side2.json
(1 hunks)backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/layoutSet2/Settings.json
(1 hunks)backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/layoutSet2/layouts/Side1.json
(1 hunks)backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/layoutSet2/layouts/Side2.json
(1 hunks)backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components/App/ui/component/Settings.json
(1 hunks)backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components/App/ui/component/layouts/Side1.json
(1 hunks)backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components/App/ui/component/layouts/Side2.json
(1 hunks)backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components/App/ui/component2/Settings.json
(1 hunks)backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components/App/ui/component2/layouts/Side1.json
(1 hunks)backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components/App/ui/component2/layouts/Side2.json
(1 hunks)backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components/App/ui/layout-sets.json
(1 hunks)backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components/App/ui/layout/Settings.json
(1 hunks)backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components/App/ui/layout/layouts/Side1.json
(1 hunks)backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components/App/ui/layout/layouts/Side2.json
(1 hunks)backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components/App/ui/layout2/Settings.json
(1 hunks)backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components/App/ui/layout2/layouts/Side1.json
(1 hunks)backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components/App/ui/layout2/layouts/Side2.json
(1 hunks)backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components/App/ui/layoutSet/Settings.json
(1 hunks)backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components/App/ui/layoutSet/layouts/Side1.json
(1 hunks)backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components/App/ui/layoutSet/layouts/Side2.json
(1 hunks)backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components/App/ui/layoutSet2/Settings.json
(1 hunks)backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components/App/ui/layoutSet2/layouts/Side1.json
(1 hunks)backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components/App/ui/layoutSet2/layouts/Side2.json
(1 hunks)frontend/language/src/nb.json
(2 hunks)
⛔ Files not processed due to max files limit (1)
- frontend/packages/process-editor/src/bpmnProviders/SupportedContextPadProvider.js
💤 Files with no reviewable changes (1)
- backend/src/Designer/EventHandlers/LayoutSetDeleted/LayoutSetDeletedComponentRefHandler.cs
✅ Files skipped from review due to trivial changes (23)
- backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/layoutSet-new/Settings.json
- backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components/App/ui/layout/Settings.json
- backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-deleting-references/App/ui/layout2/Settings.json
- backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-deleting-references/App/ui/layoutSet2/Settings.json
- backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-deleting-references/App/ui/component2/Settings.json
- backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components/App/ui/layoutSet2/Settings.json
- backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-deleting-references/App/ui/layout/Settings.json
- backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components/App/ui/component/Settings.json
- backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/layout2/Settings.json
- backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components/App/ui/layout-sets.json
- backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/layoutSet-new/layouts/Side2.json
- backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components/App/ui/layout2/Settings.json
- backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/layoutSet2/Settings.json
- backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/layout/Settings.json
- backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components/App/ui/layoutSet/layouts/Side2.json
- backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components/App/ui/layoutSet/Settings.json
- backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/layout-sets.json
- backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components/App/ui/layoutSet/layouts/Side1.json
- backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/layout/layouts/Side2-new.json
- backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components/App/ui/component2/Settings.json
- backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/component/Settings.json
- backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-deleting-references/App/ui/component/Settings.json
- backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-deleting-references/App/ui/layout-sets.json
🧰 Additional context used
🪛 GitHub Check: Run dotnet build and test (macos-latest)
backend/src/Designer/EventHandlers/ComponentIdChanged/ComponentIdChangedLayoutsHandler.cs
[failure] 25-25:
The name 'appDevelopmentService' does not exist in the current context
[failure] 53-53:
Argument 1: cannot convert from 'string' to 'Altinn.Studio.Designer.Enums.ReferenceType'
backend/src/Designer/EventHandlers/LayoutSetIdChanged/LayoutSetIdChangedLayoutsHandler.cs
[failure] 22-22:
Argument 1: cannot convert from 'string' to 'Altinn.Studio.Designer.Enums.ReferenceType'
backend/src/Designer/EventHandlers/ProcessTaskIdChanged/ProcessTaskIdChangedLayoutsHandler.cs
[failure] 31-31:
Argument 1: cannot convert from 'string' to 'Altinn.Studio.Designer.Enums.ReferenceType'
[failure] 31-31:
Argument 1: cannot convert from 'string' to 'Altinn.Studio.Designer.Enums.ReferenceType'
backend/src/Designer/Services/Implementation/AppDevelopmentService.cs
[failure] 622-622:
Operator '==' cannot be applied to operands of type 'ReferenceType' and 'string'
[failure] 623-623:
Operator '==' cannot be applied to operands of type 'ReferenceType' and 'string'
[failure] 624-624:
Operator '==' cannot be applied to operands of type 'ReferenceType' and 'string'
[failure] 625-625:
Operator '==' cannot be applied to operands of type 'ReferenceType' and 'string'
backend/src/Designer/EventHandlers/LayoutPageIdChanged/LayoutPageIdChangedLayoutsHandler.cs
[failure] 22-22:
Argument 1: cannot convert from 'string' to 'Altinn.Studio.Designer.Enums.ReferenceType'
🪛 GitHub Check: Analyze
backend/src/Designer/EventHandlers/ComponentIdChanged/ComponentIdChangedLayoutsHandler.cs
[failure] 25-25:
The name 'appDevelopmentService' does not exist in the current context
[failure] 25-25:
The name 'appDevelopmentService' does not exist in the current context
[failure] 53-53:
Argument 1: cannot convert from 'string' to 'Altinn.Studio.Designer.Enums.ReferenceType'
backend/src/Designer/EventHandlers/LayoutSetIdChanged/LayoutSetIdChangedLayoutsHandler.cs
[failure] 22-22:
Argument 1: cannot convert from 'string' to 'Altinn.Studio.Designer.Enums.ReferenceType'
backend/src/Designer/EventHandlers/ProcessTaskIdChanged/ProcessTaskIdChangedLayoutsHandler.cs
[failure] 31-31:
Argument 1: cannot convert from 'string' to 'Altinn.Studio.Designer.Enums.ReferenceType'
backend/src/Designer/Services/Implementation/AppDevelopmentService.cs
[failure] 622-622:
Operator '==' cannot be applied to operands of type 'ReferenceType' and 'string'
[failure] 623-623:
Operator '==' cannot be applied to operands of type 'ReferenceType' and 'string'
[failure] 624-624:
Operator '==' cannot be applied to operands of type 'ReferenceType' and 'string'
[failure] 625-625:
Operator '==' cannot be applied to operands of type 'ReferenceType' and 'string'
backend/src/Designer/EventHandlers/LayoutPageIdChanged/LayoutPageIdChangedLayoutsHandler.cs
[failure] 22-22:
Argument 1: cannot convert from 'string' to 'Altinn.Studio.Designer.Enums.ReferenceType'
🪛 GitHub Check: Run integration tests against actual gitea and db
backend/src/Designer/EventHandlers/ComponentIdChanged/ComponentIdChangedLayoutsHandler.cs
[failure] 25-25:
The name 'appDevelopmentService' does not exist in the current context
[failure] 53-53:
Argument 1: cannot convert from 'string' to 'Altinn.Studio.Designer.Enums.ReferenceType'
backend/src/Designer/EventHandlers/LayoutSetIdChanged/LayoutSetIdChangedLayoutsHandler.cs
[failure] 22-22:
Argument 1: cannot convert from 'string' to 'Altinn.Studio.Designer.Enums.ReferenceType'
backend/src/Designer/EventHandlers/ProcessTaskIdChanged/ProcessTaskIdChangedLayoutsHandler.cs
[failure] 31-31:
Argument 1: cannot convert from 'string' to 'Altinn.Studio.Designer.Enums.ReferenceType'
[failure] 31-31:
Argument 1: cannot convert from 'string' to 'Altinn.Studio.Designer.Enums.ReferenceType'
backend/src/Designer/Services/Implementation/AppDevelopmentService.cs
[failure] 622-622:
Operator '==' cannot be applied to operands of type 'ReferenceType' and 'string'
[failure] 623-623:
Operator '==' cannot be applied to operands of type 'ReferenceType' and 'string'
[failure] 624-624:
Operator '==' cannot be applied to operands of type 'ReferenceType' and 'string'
[failure] 625-625:
Operator '==' cannot be applied to operands of type 'ReferenceType' and 'string'
backend/src/Designer/EventHandlers/LayoutPageIdChanged/LayoutPageIdChangedLayoutsHandler.cs
[failure] 22-22:
Argument 1: cannot convert from 'string' to 'Altinn.Studio.Designer.Enums.ReferenceType'
🪛 GitHub Check: Run dotnet build and test (ubuntu-latest)
backend/src/Designer/EventHandlers/ComponentIdChanged/ComponentIdChangedLayoutsHandler.cs
[failure] 25-25:
The name 'appDevelopmentService' does not exist in the current context
[failure] 25-25:
The name 'appDevelopmentService' does not exist in the current context
[failure] 53-53:
Argument 1: cannot convert from 'string' to 'Altinn.Studio.Designer.Enums.ReferenceType'
backend/src/Designer/EventHandlers/LayoutSetIdChanged/LayoutSetIdChangedLayoutsHandler.cs
[failure] 22-22:
Argument 1: cannot convert from 'string' to 'Altinn.Studio.Designer.Enums.ReferenceType'
backend/src/Designer/EventHandlers/ProcessTaskIdChanged/ProcessTaskIdChangedLayoutsHandler.cs
[failure] 31-31:
Argument 1: cannot convert from 'string' to 'Altinn.Studio.Designer.Enums.ReferenceType'
backend/src/Designer/Services/Implementation/AppDevelopmentService.cs
[failure] 622-622:
Operator '==' cannot be applied to operands of type 'ReferenceType' and 'string'
[failure] 623-623:
Operator '==' cannot be applied to operands of type 'ReferenceType' and 'string'
[failure] 624-624:
Operator '==' cannot be applied to operands of type 'ReferenceType' and 'string'
[failure] 625-625:
Operator '==' cannot be applied to operands of type 'ReferenceType' and 'string'
backend/src/Designer/EventHandlers/LayoutPageIdChanged/LayoutPageIdChangedLayoutsHandler.cs
[failure] 22-22:
Argument 1: cannot convert from 'string' to 'Altinn.Studio.Designer.Enums.ReferenceType'
🪛 GitHub Actions: CodeQL
backend/src/Designer/EventHandlers/ComponentIdChanged/ComponentIdChangedLayoutsHandler.cs
[error] 25-25: The name 'appDevelopmentService' does not exist in the current context
🪛 GitHub Actions: Playwright tests on pr
backend/src/Designer/EventHandlers/ComponentIdChanged/ComponentIdChangedLayoutsHandler.cs
[error] 25-25: The name 'appDevelopmentService' does not exist in the current context
[error] 53-53: Argument 1: cannot convert from 'string' to 'Altinn.Studio.Designer.Enums.ReferenceType'
backend/src/Designer/EventHandlers/LayoutSetIdChanged/LayoutSetIdChangedLayoutsHandler.cs
[error] 22-22: Argument 1: cannot convert from 'string' to 'Altinn.Studio.Designer.Enums.ReferenceType'
backend/src/Designer/EventHandlers/ProcessTaskIdChanged/ProcessTaskIdChangedLayoutsHandler.cs
[error] 31-31: Argument 1: cannot convert from 'string' to 'Altinn.Studio.Designer.Enums.ReferenceType'
backend/src/Designer/Services/Implementation/AppDevelopmentService.cs
[error] 622-625: Multiple operator '==' errors: Cannot be applied to operands of type 'ReferenceType' and 'string'
backend/src/Designer/EventHandlers/LayoutPageIdChanged/LayoutPageIdChangedLayoutsHandler.cs
[error] 22-22: Argument 1: cannot convert from 'string' to 'Altinn.Studio.Designer.Enums.ReferenceType'
🪛 GitHub Actions: Build and Test on windows, linux and macos
backend/src/Designer/EventHandlers/ComponentIdChanged/ComponentIdChangedLayoutsHandler.cs
[error] 25-25: The name 'appDevelopmentService' does not exist in the current context
backend/src/Designer/EventHandlers/ProcessTaskIdChanged/ProcessTaskIdChangedLayoutsHandler.cs
[error] 31-31: Argument 1: cannot convert from 'string' to 'Altinn.Studio.Designer.Enums.ReferenceType'
🪛 GitHub Actions: Run gitea integration tests
backend/src/Designer/EventHandlers/ProcessTaskIdChanged/ProcessTaskIdChangedLayoutsHandler.cs
[error] 31-31: Argument 1: cannot convert from 'string' to 'Altinn.Studio.Designer.Enums.ReferenceType'
🔇 Additional comments (75)
backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/layout2/layouts/Side1.json (3)
6-8
: Verify if the empty dataModelBinding is intentional.The Input component has an empty
simpleBinding
. If this is test data, consider adding a meaningful binding to better represent real-world scenarios.
13-16
: Provide a valid layoutSet ID.The Summary2 component has an empty layoutSet ID. This might cause issues when resolving the target during runtime.
1-4
: Verify the schema version.The layout references schema version 1. Please verify if this is the latest compatible version for the current Altinn Studio release.
✅ Verification successful
Schema version is correct
The layout schema version (v1) is consistent with all other layout files in the codebase and is the current standard version used with toolkit version 4.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for references to layout schema versions across the codebase # to ensure consistency and identify the latest version in use. # Search for schema references rg -g '*.json' "altinncdn.no/toolkits/altinn-app-frontend/.*/schemas/json/layout/layout.schema.v.*\.json"Length of output: 9980
backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/layoutSet-new/layouts/Side1.json (3)
3-5
: LGTM!The layout structure follows the expected format with proper nesting of data and layout properties.
Also applies to: 11-13
1-2
: Verify the schema URL accessibility.The schema URL follows the expected pattern, but let's verify its accessibility to ensure the layout validation works correctly.
✅ Verification successful
Schema URL is valid and accessible
The schema URL is properly configured, accessible, and points to a valid JSON schema file that's actively maintained.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the schema URL is accessible curl -I "https://altinncdn.no/toolkits/altinn-app-frontend/4/schemas/json/layout/layout.schema.v1.json"Length of output: 1110
6-10
: Consider documenting the component ID pattern.The navigation component looks good, but consider adding a comment or documentation about:
- The component ID suffix pattern (
-n2jUp6
)- The purpose of empty
textResourceBindings
This will help other developers understand the test data structure better.
Let's check if this component ID pattern is consistent across other test files:
backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components/App/ui/component/layouts/Side2.json (3)
5-18
: Review empty data model bindings and component duplication.
- The data model bindings are empty for both Input components. If this is test data, consider adding meaningful bindings to better represent real-world scenarios.
- The components appear to be identical except for their IDs. Consider removing the redundant component unless the duplication serves a specific test purpose.
1-2
: Verify schema version compatibility.Please ensure that version 4 of the layout schema matches the version used in production environments.
✅ Verification successful
Schema version is consistent across the codebase
The version 4 schema reference is correctly used and matches all other layout files in the codebase, including templates and test data.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for schema version references across the codebase # Look for other schema references to ensure consistency rg -g '*.json' -g '!node_modules' 'altinncdn.no/toolkits/altinn-app-frontend/[0-9]+/schemas/json/layout/layout.schema.v1.json'Length of output: 10001
1-27
: Verify relevance to Summary 2.0 component updates.This layout file doesn't contain any Summary 2.0 components, yet the PR objectives specifically mention updating "Summary 2.0 components that has a reference to an updated component id". Please verify if this file should be included in this PR.
backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components/App/ui/component2/layouts/Side1.json (4)
6-8
: Verify if the empty data model binding is intentional.The Input component has an empty
simpleBinding
. If this is test data, consider adding a comment explaining why it's empty or provide a valid binding for testing purposes.
1-4
: Verify the schema version is up to date.The layout schema references version 1. Please ensure this is the latest compatible version for Summary 2.0 components.
✅ Verification successful
Schema version is correct
The layout schema version v1 is the latest version, as confirmed by consistent usage across all layout files and no evidence of newer versions.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if newer schema versions exist in the CDN curl -s -I https://altinncdn.no/toolkits/altinn-app-frontend/4/schemas/json/layout/layout.schema.v2.json # Search for schema version references in the codebase rg -g '*.json' '"https://altinncdn.no/toolkits/altinn-app-frontend/4/schemas/json/layout/layout.schema.v[0-9]+.json"'Length of output: 9312
1-56
: Verify all Summary2 component references are updated.As this PR aims to update Summary2 component references, please ensure all target IDs in this file are aligned with the latest component IDs in the application.
✅ Verification successful
Test file references are correctly structured
This is a test data file where the Summary2 component references are intentionally structured to test various scenarios. All referenced IDs exist consistently across the test variants and are valid for testing purposes.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for all referenced component IDs in the codebase echo "Checking references for Input-LCr3oK" rg "Input-LCr3oK" echo "Checking references for Side1" rg '"id": "Side1"' echo "Checking references for Side2" rg '"id": "Side2"'Length of output: 8526
31-35
: Verify the existence of "Side2" page.The Summary2 component references a page with ID "Side2". Please ensure this page exists in the application.
✅ Verification successful
"Side2" page exists and is properly defined
The referenced page "Side2" exists in the same component directory at
App/ui/component2/layouts/Side2.json
.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for Side2 page definition in layout files fd -e json -x grep -l '"id": "Side2"' {} \;Length of output: 2842
backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/layout/layouts/Side1.json (4)
1-4
: LGTM! Schema and structure are well-defined.The layout file correctly references the schema and follows the expected Altinn layout structure.
6-9
: Verify the empty layoutSet id.The target layoutSet has an empty id which might lead to undefined behavior. Please ensure this is intentional or specify a valid id.
3-48
: LGTM! Layout structure aligns with PR objectives.The layout successfully implements multiple Summary2 components with different targets and unique IDs, which aligns with the PR objective of updating Summary2 component references.
33-36
: Verify the referenced Input component exists.The Summary2 component targets an Input component with id "Input-AVRSNf". Let's verify this component exists in the referenced page.
✅ Verification successful
Input component "Input-AVRSNf" exists and is properly defined
The referenced Input component exists and is properly defined in
Side2-new.json
within the same test data directory.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for the referenced Input component rg -l "Input-AVRSNf" --type jsonLength of output: 878
Script:
#!/bin/bash # Search for the Input component definition rg -A 10 '"id":\s*"Input-AVRSNf"' --type jsonLength of output: 11006
backend/src/Designer/Events/LayoutPageIdChangedEvent.cs (3)
1-3
: LGTM!The imports are clean and appropriate for the dependencies required.
4-5
: LGTM!The namespace follows the project's organization structure.
6-12
: Verify consistency with other event implementations.Let's ensure this event follows the same patterns as other events in the codebase.
✅ Verification successful
Event implementation follows established patterns ✅
- Consistent with other event implementations in the codebase
- Has proper handler implementation with sync functionality
- Follows the same property pattern as other layout-related events
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if other events follow similar patterns and if handlers exist # Check for other event implementations echo "Checking other event implementations..." ast-grep --pattern 'class $EVENT : INotification { $$$ }' # Check for handlers of this event echo "Checking for event handlers..." rg -l "INotificationHandler<LayoutPageIdChangedEvent>"Length of output: 368
Script:
#!/bin/bash # Search for other event classes and layout-related events echo "Checking for all event classes..." rg -l "class.*Event.*INotification" echo -e "\nChecking layout-related events..." rg -l "class.*Layout.*Event" echo -e "\nChecking handler implementation..." cat backend/src/Designer/EventHandlers/LayoutPageIdChanged/LayoutPageIdChangedLayoutsHandler.csLength of output: 3445
backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components/App/ui/layout/layouts/Side2.json (3)
1-4
: Verify the schema version compatibility.The layout references schema version 4. Please ensure this version aligns with the application's requirements and dependencies.
✅ Verification successful
Schema version is correctly aligned with codebase standards
The schema version 4 is consistently used across all layout files in the test data, confirming this is the correct version to use.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other schema version references in the codebase rg -g '*.json' '"https://altinncdn.no/toolkits/altinn-app-frontend/[0-9]+/schemas/json/layout/layout.schema.v1.json"'Length of output: 8908
1-20
: Verify test coverage for Summary 2.0 components.This test file is located in a directory for testing Summary 2.0 components, but it doesn't contain any Summary components. Please verify if additional test cases are needed to cover the Summary 2.0 component updates mentioned in the PR objectives.
5-11
: Verify the empty data model binding intention.The Input component has an empty
simpleBinding
value. If this is a test file, please add a comment explaining that this is intentional for testing purposes. Otherwise, consider adding the appropriate binding value.backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-deleting-references/App/ui/layoutSet2/layouts/Side1.json (5)
1-2
: LGTM! Schema declaration is properly configured.The schema URL correctly points to the CDN-hosted schema file with a stable version.
37-45
: LGTM! Component reference is properly configured.The Summary2 component correctly references the existing Input component "Input-wrspcN".
5-11
: Verify if empty simpleBinding is intentional.The Input component has an empty data model binding. While this might be intentional for test data, please verify if this is the expected behavior.
✅ Verification successful
Empty simpleBinding is intentional in test data
The empty binding is a consistent pattern across multiple test files and scenarios, indicating this is intentional test data behavior focused on layout structure rather than data bindings.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other test files with Input components to verify if empty bindings are common in test data rg -g "*.json" -C 5 '"simpleBinding": ""' backend/tests/Designer.Tests/_TestData/Length of output: 32806
20-27
: Verify self-referential page target.The Summary2 component references the current page ("Side1"). This might create circular dependencies in the layout resolution.
1-54
: Verify component ID updates align with PR objectives.Since this PR aims to update Summary 2.0 components with updated component IDs, please verify that all component IDs in this layout have been properly updated.
✅ Verification successful
Summary2 components are properly configured with valid IDs and targets
The layout contains correctly structured Summary2 components, each with unique IDs and proper target configurations. All referenced components exist within the layout, maintaining the integrity of the component relationships.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for these component IDs in the old version of the layout to confirm updates git log -p -- backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-deleting-references/App/ui/layoutSet2/layouts/Side1.json | grep -A 5 '"id": "Summary2-'Length of output: 0
Script:
#!/bin/bash # Find similar layout files fd Side1.json # Search for Summary2 components in all files rg -A 5 '"type": "Summary2",' --type jsonLength of output: 6687
Script:
#!/bin/bash # Compare Summary2 components across different versions of Side1.json echo "Original version:" rg -A 5 '"type": "Summary2",' backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components/App/ui/layoutSet2/layouts/Side1.json echo -e "\nAfter updating references:" rg -A 5 '"type": "Summary2",' backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/layoutSet2/layouts/Side1.json echo -e "\nAfter deleting references:" cat backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-deleting-references/App/ui/layoutSet2/layouts/Side1.jsonLength of output: 1846
backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/component2/layouts/Side1.json (4)
1-2
: Verify schema version matches production usage.Since this is a test file, ensure that the schema version (
v1
) matches the version used in production environments.✅ Verification successful
Schema version is consistent with production usage
The schema version (
v1
) and toolkit version (4
) are consistently used across all layout files in the codebase, including application templates.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for schema version references in production layout files rg -g '*.json' 'https://altinncdn.no/toolkits/altinn-app-frontend/.*/schemas/json/layout/layout.schema.v.*\.json'Length of output: 9988
39-47
: Verify the referenced Input component ID.The Summary2 component references
Input-LCr3oK
which is defined earlier in the layout. Since this PR involves updating component references, ensure this ID is correctly maintained across all related files.✅ Verification successful
Component reference verification successful
The Input component ID "Input-LCr3oK" is correctly maintained across all relevant test data files, including the versions before and after reference updates.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for references to this Input component rg "Input-LCr3oK"Length of output: 1093
13-17
: Empty target IDs in Summary2 component.The first Summary2 component has empty
id
andtaskId
fields in its target configuration. This might cause issues if these fields are required for proper component reference resolution.✅ Verification successful
Empty target IDs are valid for layoutSet type targets.
The codebase shows a consistent pattern where Summary2 components with
"type": "layoutSet"
targets use emptyid
fields by design, while maintaining appropriatetaskId
values based on their context.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other Summary2 components to verify if empty target IDs are valid ast-grep --pattern $'{\n $$$\n "target": {\n "type": "layoutSet",\n "id": "",\n $$$\n },\n $$$\n "type": "Summary2"\n}'Length of output: 125
Script:
#!/bin/bash # Search for Summary2 components and their target configurations echo "=== Summary2 Components ===" rg -U '"type":\s*"Summary2"' -A 10 -B 5 echo -e "\n=== Documentation ===" fd "README|CONTRIBUTING" --extension md --exec cat {}Length of output: 65838
4-54
: Verify all Summary2 component references are updated.Since this PR's objective is to update Summary 2.0 component references, please verify that all target IDs in this layout file are aligned with the latest component updates mentioned in issue #14037.
✅ Verification successful
Summary2 component references are properly configured
All Summary2 components in this layout file have valid target references:
- Component reference (Input-LCr3oK) exists in the same layout
- Page references (Side1, Side2) use valid page IDs
- Layout set reference with empty ID is a valid configuration
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for all Summary2 components and their target references ast-grep --pattern $'{\n $$$\n "target": {\n "type": "$_",\n "id": "$_",\n $$$\n },\n $$$\n "type": "Summary2"\n}'Length of output: 120
Script:
#!/bin/bash # Get the content of the JSON file and extract Summary2 components with their targets cat backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/component2/layouts/Side1.json | jq '.layout[] | select(.type == "Summary2") | {id: .id, target}'Length of output: 288
Script:
#!/bin/bash # First verify the file exists and show its contents echo "=== File contents ===" cat backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/component2/layouts/Side1.json echo -e "\n=== Basic structure ===" cat backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/component2/layouts/Side1.json | jq '.'Length of output: 2841
backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/layout2/layouts/Side2.json (5)
24-31
: Verify the reference update to "Side2-new".The "-new" suffix suggests this is a renamed/updated page. Let's ensure all references to the old page have been updated consistently.
✅ Verification successful
Reference update to "Side2-new" is consistent
All references to the page have been properly updated to use "Side2-new". No outdated references were found in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining references to potential old page name rg -g '*.json' '"id":\s*"Side2[^-]"' # Search for all references to the new page rg -g '*.json' '"id":\s*"Side2-new"'Length of output: 442
15-22
: Verify the existence of the "Side1" page.The component references a page with id "Side1". Let's ensure this page exists in the layout configuration.
✅ Verification successful
The "Side1" page reference is valid
The referenced page exists in the same directory structure, confirming that this is a valid reference.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for the referenced page in layout files fd -e json -x rg -l '"id":\s*"Side1"' {}Length of output: 3460
33-40
: Verify the component reference and check for circular dependencies.The Summary2 component references another component (Input-AVRSNf). Let's ensure:
- The referenced component exists
- There are no circular references that could cause infinite loops
✅ Verification successful
Component reference and dependencies verified successfully
The referenced Input component exists and there are no circular dependencies detected.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for the referenced Input component rg -g '*.json' '"id":\s*"Input-AVRSNf"' # Check for potential circular references rg -g '*.json' -C 10 '"id":\s*"Summary2-uuKXTD"'Length of output: 7955
6-13
: Verify the empty layoutSet target id.The target id for this Summary2 component is empty. This might cause issues during runtime as the component won't know which layoutSet to reference.
1-49
: Verify if the combination of Summary2 target types is valid.The layout contains Summary2 components targeting different types (layoutSet, page, component). Let's ensure this combination is supported and follows the intended design.
✅ Verification successful
The combination of Summary2 target types is valid
The codebase's test data demonstrates that Summary2 components consistently support all three target types (layoutSet, page, and component) across multiple test scenarios, confirming this is an intentionally supported feature.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other Summary2 components to understand common target type patterns ast-grep --pattern $'{\n $$$\n "type": "Summary2",\n $$$\n "target": {\n "type": "$_",\n $$$\n }\n}'Length of output: 104
Script:
#!/bin/bash # Search for Summary2 components in JSON files with context rg --type json '"type":\s*"Summary2"' -B 2 -A 5 # Also search specifically for target types used with Summary2 rg --type json -U 'type":\s*"Summary2"[\s\S]{0,50}target":\s*{\s*"type":\s*"[^"]*"' --multilineLength of output: 65872
Script:
#!/bin/bash # Fetch and examine the schema file referenced in the JSON curl -s "https://altinncdn.no/toolkits/altinn-app-frontend/4/schemas/json/layout/layout.schema.v1.json" | jq '.properties.data.properties.layout.items.properties.target.properties.type.enum'Length of output: 193
backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-deleting-references/App/ui/layout/layouts/Side1.json (4)
15-22
: LGTM! Target page configuration looks good.The Summary2 component correctly targets the page matching the filename (Side1).
1-2
: Verify schema version and accessibility.Please verify that version 4 of the schema is the latest stable version and that the URL is accessible.
✅ Verification successful
Schema URL and version are valid.
The schema URL is accessible and version 4 is consistently used across the codebase as the current standard.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify schema URL accessibility and check for redirects curl -I "https://altinncdn.no/toolkits/altinn-app-frontend/4/schemas/json/layout/layout.schema.v1.json" # Search for other schema version references in the codebase rg "altinncdn.no/toolkits/altinn-app-frontend/[0-9]+/schemas/json/layout/layout\.schema\.v1\.json"Length of output: 11617
6-13
: Verify empty target IDs for layoutSet.The target layoutSet has empty
id
andtaskId
. While this might be intentional for test data, please verify that the test cases properly handle these empty values.✅ Verification successful
Empty target IDs are expected in this test data context
The empty
id
andtaskId
in the layoutSet target are intentional, as this file is part of the test data for deletion scenarios. This is confirmed by:
- The file path contains "after-deleting-references"
- Test suite includes specific cases for handling Summary2 components after layoutSet deletion
- Empty target properties represent the expected state after their referenced layoutSet is removed
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for test cases handling empty target IDs rg -A 5 "type.*layoutSet.*id.*\"\"" "backend/tests/Designer.Tests"Length of output: 64
Script:
#!/bin/bash # Search for layoutSet patterns in test files rg -l "layoutSet" "backend/tests/Designer.Tests" # Search for Summary2 component test cases rg -A 5 "Summary2" "backend/tests/Designer.Tests" # Look for test files containing target configurations fd "json$" backend/tests/Designer.Tests/_TestData -x rg -l "target"Length of output: 73721
Script:
#!/bin/bash # Search for test files related to deletion scenarios rg -l "deleting-references" "backend/tests/Designer.Tests" # Search for test classes that might handle these scenarios rg -l "DeleteFormLayout|DeleteLayoutSet" "backend/tests/Designer.Tests" # Look for test assertions related to empty targets rg -A 5 "target.*layoutSet.*\"\"" "backend/tests/Designer.Tests/**/*.cs"Length of output: 729
Script:
#!/bin/bash # Check content of deletion test files rg -A 10 "DeleteLayoutSet|DeleteFormLayout" "backend/tests/Designer.Tests/Controllers/AppDevelopmentController/DeleteLayoutSetTests.cs" rg -A 10 "DeleteLayoutSet|DeleteFormLayout" "backend/tests/Designer.Tests/Controllers/AppDevelopmentController/DeleteFormLayoutTests.cs" # Look for any validation or assertions about target properties rg -A 5 "target.*id.*\"\"" "backend/tests/Designer.Tests/Controllers/AppDevelopmentController/DeleteLayoutSetTests.cs"Length of output: 8311
23-28
: Verify handling of empty textResourceBindings.The NavigationButtons component has empty textResourceBindings. Please verify that the test cases properly handle navigation buttons without text resources.
✅ Verification successful
Empty textResourceBindings is a valid test configuration
The empty textResourceBindings object is consistently used across all test scenarios in the codebase, including states before and after reference updates/deletions. This appears to be an intentional test configuration rather than an oversight.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for test cases handling NavigationButtons without text resources rg -A 5 "NavigationButtons.*textResourceBindings.*\{\}" "backend/tests/Designer.Tests"Length of output: 86
Script:
#!/bin/bash # Search for NavigationButtons in test data and test files echo "=== NavigationButtons in JSON test data ===" fd -e json -x rg -l "NavigationButtons" {} backend/tests/Designer.Tests/_TestData echo -e "\n=== NavigationButtons in test files ===" fd -e cs -x rg -l "NavigationButtons" {} backend/tests/Designer.Tests echo -e "\n=== Sample of NavigationButtons usage ===" rg -A 5 "\"type\": \"NavigationButtons\"" backend/tests/Designer.Tests/_TestDataLength of output: 68070
Script:
#!/bin/bash # Get content of NavigationButtons components from test files echo "=== NavigationButtons implementations in test data ===" rg -U --multiline -A 5 '{\s*"id":\s*"NavigationButtons[^}]*"type":\s*"NavigationButtons"[^}]*}' backend/tests/Designer.Tests/_TestDataLength of output: 401
Script:
#!/bin/bash # Search for NavigationButtons component implementations with fixed pattern echo "=== NavigationButtons implementations in test data ===" rg -U --multiline ' \{[\s\S]*?"type": "NavigationButtons"[\s\S]*?\}' backend/tests/Designer.Tests/_TestDataLength of output: 83148
backend/src/Designer/Models/Reference.cs (1)
6-6
: LGTM!The
Reference
record is well-defined and uses appropriate types.backend/src/Designer/Events/LayoutPageDeletedEvent.cs (1)
6-11
: Add XML documentation and address missing property.
- Add XML documentation to describe the event's purpose
- The AI summary mentions a
NewLayoutName
property that's not present in the code. Please verify if this property is needed.+/// <summary> +/// Event published when a layout page is deleted +/// </summary> public class LayoutPageDeletedEvent : INotification { - public AltinnRepoEditingContext EditingContext { get; set; } - public string LayoutSetName { get; set; } - public string LayoutName { get; set; } + /// <summary> + /// The context in which the layout page is being edited + /// </summary> + public required AltinnRepoEditingContext EditingContext { get; set; } + + /// <summary> + /// The name of the layout set containing the deleted page + /// </summary> + public required string LayoutSetName { get; set; } + + /// <summary> + /// The name of the deleted layout page + /// </summary> + public required string LayoutName { get; set; } }Let's verify if the
NewLayoutName
property is referenced elsewhere:backend/src/Designer/Enums/ResourceReferenceSource.cs (1)
8-8
: LGTM! Verify all usages of the renamed enum.The rename from
ReferenceSource
toResourceReferenceSource
improves clarity. However, this is a breaking change.Let's verify all usages have been updated:
✅ Verification successful
All usages of the renamed enum have been updated correctly
The codebase scan confirms that all references to the enum are using the new name
ResourceReferenceSource
.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining references to the old enum name rg "ReferenceSource" --type csLength of output: 678
backend/src/Designer/Enums/ResourceReferenceType.cs (1)
8-30
: LGTM! Well-structured enum implementation.Clean implementation with proper documentation and serialization support.
backend/src/Designer/Models/ResourceReference.cs (1)
16-16
: LGTM! Property types correctly updated.Properties properly updated to use the new enum types while maintaining nullability and serialization configuration.
Also applies to: 27-27
backend/src/Designer/EventHandlers/ComponentDeleted/ComponentDeletedLayoutsHandler.cs (1)
13-27
: Implementation looks good!The handler correctly implements the notification pattern, uses proper types, and handles cancellation appropriately.
backend/src/Designer/EventHandlers/LayoutPageDeleted/LayoutPageDeletedLayoutsHandler.cs (1)
13-27
: Implementation looks good!The handler follows the established pattern and correctly implements the notification handling logic.
backend/src/Designer/EventHandlers/LayoutSetDeleted/LayoutSetDeletedLayoutsHandler.cs (1)
23-23
: Consider simplifying Reference constructor arguments.The
notification.LayoutSetName
is used twice in the Reference constructor. Consider if this is intentional or if one of these parameters serves a different purpose.Please verify if passing the same value for both the second and third parameters is the intended behavior.
backend/tests/Designer.Tests/Controllers/ResourceAdminController/ResourceAdminControllerTestsBaseClass.cs (1)
39-39
: LGTM! Type safety improvement.Good refactoring to use more specific enum types (
ResourceReferenceSource
andResourceReferenceType
).backend/tests/Designer.Tests/Controllers/AppDevelopmentController/DeleteLayoutSetTests.cs (1)
163-192
: LGTM! The test is well-structured and comprehensive.The test effectively verifies that:
- Layout set deletion works correctly
- Associated Summary2 components are properly handled
- Layout files match the expected state after deletion
backend/tests/Designer.Tests/Controllers/AppDevelopmentController/SaveFormLayoutTests.cs (2)
98-140
: LGTM! The deletion test is well-structured.The test effectively verifies that:
- Component deletion works correctly
- Associated Summary2 components are properly handled
- Layout files match the expected state after deletion
142-183
: LGTM! The update test is well-structured.The test effectively verifies that:
- Component name update works correctly
- Associated Summary2 components are properly updated
- Layout files match the expected state after update
backend/src/Designer/Controllers/ResourceAdminController.cs (2)
Line range hint
554-568
: Validation logic remains correct after type update.The validation for MaskinportenSchema resources correctly uses the new
ResourceReferenceType
while maintaining the same validation checks.
490-490
: Type reference update looks good.The change from
ReferenceType
toResourceReferenceType
maintains the same logic while using the new type system.Let's verify that all references to the old type have been updated:
Also applies to: 499-500
✅ Verification successful
Type reference changes are correctly implemented
The changes from
ReferenceType
toResourceReferenceType
in ResourceAdminController.cs are consistent. Other occurrences ofReferenceType
in the codebase belong to a different type used for layout and component references, which is unrelated to this change.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining references to the old ReferenceType rg "ReferenceType\." --type csLength of output: 3324
backend/src/Designer/Controllers/AppDevelopmentController.cs (1)
182-189
: Event publishing for layout deletion is well implemented.The code correctly publishes the
LayoutPageDeletedEvent
before performing the deletion, allowing other components to react to the change.backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/component2/Settings.json (1)
1-9
: Settings configuration is well structured.The file follows the schema specification and provides a clear page ordering configuration.
backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-deleting-references/App/ui/layoutSet2/layouts/Side2.json (1)
1-13
: Layout schema is properly configured.The layout configuration correctly defines the navigation buttons with appropriate properties and schema reference.
backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-deleting-references/App/ui/layout2/layouts/Side2.json (1)
6-10
: Verify empty layoutSet ID configuration.The Summary2 component targets a layoutSet with an empty ID. This might cause reference resolution issues.
backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components/App/ui/layout/layouts/Side1.json (1)
8-10
: Verify empty taskId configuration pattern.Multiple components have empty
taskId
values. While this might be intentional for test data, verify if this represents valid test scenarios.Also applies to: 18-19, 27-28, 36-37
✅ Verification successful
Empty taskId is a valid test configuration
The empty taskId values are intentionally used in test data files to verify component behavior with different taskId configurations. This is consistent with other test files in the same test data directory that use both empty and specific taskId values for comprehensive test coverage.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for taskId patterns in layouts ast-grep --pattern 'taskId: ""'Length of output: 33
Script:
#!/bin/bash # Search for taskId patterns in JSON files rg '"taskId":\s*[""]?' -g "*.json" -A 1 -B 1 # Search for test files related to Summary2 components fd "Summary2.*\.cs|Summary2.*\.test" --type fLength of output: 49588
backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-deleting-references/App/ui/component/layouts/Side1.json (1)
33-40
: Verify component reference integrity.The Summary2 component targets
Input-qWr0oa
. Ensure this reference is consistent with the component defined in Side2.json.✅ Verification successful
Reference integrity verified
The Summary2 component's reference to
Input-qWr0oa
is valid and consistent with its definition in Side2.json.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for Input-qWr0oa references rg -l "Input-qWr0oa"Length of output: 1349
Script:
#!/bin/bash # Get content of Side2.json in the after-deleting-references state cat "backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-deleting-references/App/ui/component/layouts/Side2.json"Length of output: 618
backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components/App/ui/layout2/layouts/Side2.json (1)
7-9
: Verify empty layoutSet ID.The layoutSet target has an empty ID which could cause reference issues. Consider if this is intentional or if a valid ID should be provided.
✅ Verification successful
Empty layoutSet ID is a valid pattern.
The empty ID in layoutSet targets appears to be an intentional and consistent pattern across the test data, used in various components and layouts.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other layoutSet references to verify if empty ID is a common pattern rg -l '"type":\s*"layoutSet".*"id":\s*""' --type jsonLength of output: 55
Script:
#!/bin/bash # Search for layoutSet occurrences with surrounding context rg -U --json '"type":\s*"layoutSet"' --type json -C 5 # Backup search using ast-grep for JSON structures ast-grep --pattern '{ "type": "layoutSet", $$$ }'Length of output: 78639
backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components/App/ui/layoutSet2/layouts/Side2.json (1)
6-40
: Verify taskId consistency across layouts.All components use "layoutSet" as taskId, which differs from the "layout" taskId in layout2/layouts/Side2.json. Ensure this difference is intentional and documented.
backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-deleting-references/App/ui/component2/layouts/Side2.json (1)
1-49
: LGTM! Test data correctly represents post-deletion state.The file structure maintains consistency while using appropriate taskId values for the component context.
backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/layoutSet2/layouts/Side2.json (1)
9-40
: Verify component ID preservation pattern.Component IDs (e.g., "Summary2-6VQ3LC", "Summary2-LZxPWb") are preserved while taskId is updated to "layoutSet-new". This pattern helps track components across updates.
✅ Verification successful
Component ID preservation pattern confirmed
All component IDs are preserved exactly while taskIds are consistently updated from "layoutSet" to "layoutSet-new" across all components.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify component ID preservation across different states for id in "Summary2-6VQ3LC" "Summary2-LZxPWb" "Summary2-El9z2Y" "Summary2-SMqpYV"; do echo "Searching for $id:" rg "\"id\":\s*\"$id\"" --type json doneLength of output: 2215
Script:
#!/bin/bash # Compare taskId values between original and updated versions echo "Original version taskIds:" rg '"taskId":\s*"[^"]+"' backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components/App/ui/layoutSet2/layouts/Side2.json echo -e "\nUpdated version taskIds:" rg '"taskId":\s*"[^"]+"' backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/layoutSet2/layouts/Side2.jsonLength of output: 739
backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components/App/ui/layout2/layouts/Side1.json (1)
6-8
: Verify empty dataModelBinding.The Input component has an empty simpleBinding which could cause issues. Consider if this is intentional for test data.
✅ Verification successful
Empty dataModelBinding is valid in test data context.
The empty simpleBinding is an intentional and common pattern in test data files, matching similar empty bindings found across other test components.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other empty dataModelBindings to verify if this is a pattern rg '"dataModelBindings".*"simpleBinding":\s*""' --type jsonLength of output: 61
Script:
#!/bin/bash # Search with a more flexible pattern for dataModelBindings rg -U '"dataModelBindings"\s*:\s*{\s*"simpleBinding"' --type json -A 1 # Also search specifically in test directories fd -t f '.*\.json$' -E 'node_modules' --exec grep -A 2 '"dataModelBindings"' {} \; 2>/dev/nullLength of output: 25842
backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-deleting-references/App/ui/layout2/layouts/Side1.json (1)
14-16
: Verify the empty layoutSet ID.The target layoutSet ID is empty. This might cause issues if the Summary2 component needs to reference a specific layout set.
backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components/App/ui/layoutSet2/layouts/Side1.json (1)
1-54
: LGTM!The file maintains consistent structure with its updated version.
backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-deleting-references/App/ui/component2/layouts/Side1.json (1)
13-17
: LGTM! Good consistency in target configurations.All target configurations consistently include the taskId property, which is a good practice for maintaining uniformity across components.
Also applies to: 22-26, 31-35, 40-44
backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components/App/ui/component/layouts/Side1.json (1)
31-36
: LGTM! Well-structured override configuration.The override configuration for the Summary2 component is well-structured, properly specifying both the target component and display type.
backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/component/layouts/Side1.json (1)
33-34
: Component ID updates look consistent.The component ID "Input-Om7N3y-new" is consistently used in both the target reference and overrides section, aligning with the PR's objective to update Summary2 component references.
Let's verify all component ID updates are consistent:
Also applies to: 50-51
✅ Verification successful
Component ID updates verified as complete and consistent
The component ID "Input-Om7N3y" has been properly updated to "Input-Om7N3y-new" across all relevant files in the test data, maintaining consistency in both "id" and "componentId" fields.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining old component IDs (without -new suffix) rg "Input-Om7N3y[^-]" --type json # Search for all occurrences of the new component ID to ensure consistency rg "Input-Om7N3y-new" --type jsonLength of output: 1883
backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components/App/ui/component2/layouts/Side2.json (1)
33-34
: Test fixture correctly represents original state.The file appropriately represents the original state with "Input-Om7N3y" component ID, serving as a before-state test fixture for validating component ID updates.
Also applies to: 50-51
backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/component2/layouts/Side2.json (1)
33-34
: Component ID updates are consistent across layouts.The component ID "Input-Om7N3y-new" is consistently used in this layout as well, maintaining uniformity across different layout files.
Also applies to: 50-51
frontend/language/src/nb.json (2)
1202-1202
: Enhanced component deletion warning message.The updated message now correctly informs users that deleting a component will also delete any associated Summary2 components, improving user awareness of the cascading deletion effect.
1806-1806
: Enhanced page deletion warning message.The updated message now properly informs users that deleting a page will also delete any associated Summary2 components, maintaining consistency with the component deletion warning.
...ttd/app-with-summary2-components-after-updating-references/App/ui/layout2/layouts/Side1.json
Show resolved
Hide resolved
backend/src/Designer/EventHandlers/ProcessTaskIdChanged/ProcessTaskIdChangedLayoutsHandler.cs
Outdated
Show resolved
Hide resolved
.../Repositories/testUser/ttd/app-with-summary2-components/App/ui/component2/layouts/Side1.json
Outdated
Show resolved
Hide resolved
.../app-with-summary2-components-after-deleting-references/App/ui/layoutSet2/layouts/Side1.json
Outdated
Show resolved
Hide resolved
backend/src/Designer/EventHandlers/LayoutSetIdChanged/LayoutSetIdChangedLayoutsHandler.cs
Outdated
Show resolved
Hide resolved
backend/src/Designer/EventHandlers/ComponentIdChanged/ComponentIdChangedLayoutsHandler.cs
Outdated
Show resolved
Hide resolved
backend/src/Designer/EventHandlers/ComponentIdChanged/ComponentIdChangedLayoutsHandler.cs
Outdated
Show resolved
Hide resolved
...d/app-with-summary2-components-after-deleting-references/App/ui/component/layouts/Side2.json
Outdated
Show resolved
Hide resolved
...ata/Repositories/testUser/ttd/app-with-summary2-components/App/ui/layout2/layouts/Side1.json
Outdated
Show resolved
Hide resolved
…pdate-all-summary-20-components-that-has-a-reference-to-updated-element-id-component-page-or-layout-set
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
backend/src/Designer/Services/Implementation/AppDevelopmentService.cs (1)
Line range hint
671-779
: Refactor component update logic to reduce complexity.The component update logic is spread across multiple blocks and handles different types of updates. Consider extracting these into separate methods for better maintainability.
Extract the update logic into separate methods:
private bool UpdateComponentId(JsonObject component, string componentId, List<Reference> updatedComponents) { Reference updatedReference = updatedComponents.FirstOrDefault(item => item.Id == componentId); if (updatedReference != null) { component["id"] = updatedReference.NewId; return true; } return false; } private bool UpdateLayoutSetReference(JsonObject component, string subformLayoutSet, List<Reference> updatedLayoutsSets) { Reference updatedReference = updatedLayoutsSets.FirstOrDefault(item => item.Id == subformLayoutSet); if (updatedReference != null) { component["layoutSet"] = updatedReference.NewId; return true; } return false; } private bool UpdateSummaryComponent(JsonObject component, JsonObject target, string layoutSetId, List<Reference> updatedLayouts, List<Reference> updatedComponents, List<Reference> updatedTasks) { bool hasChanges = false; string type = target["type"]?.GetValue<string>(); string id = target["id"]?.GetValue<string>(); Reference updatedReference = type switch { "page" => updatedLayouts.FirstOrDefault(item => item.LayoutSetName == layoutSetId && item.Id == id), "component" => updatedComponents.FirstOrDefault(item => item.LayoutSetName == layoutSetId && item.Id == id), _ => null }; if (updatedReference != null) { target["id"] = updatedReference.NewId; hasChanges = true; } string taskId = target["taskId"]?.GetValue<string>(); if (!string.IsNullOrEmpty(taskId)) { updatedReference = updatedTasks.FirstOrDefault(item => item.Id == taskId); if (updatedReference != null) { target["taskId"] = updatedReference.NewId; hasChanges = true; } } return hasChanges; }
♻️ Duplicate comments (1)
backend/src/Designer/Services/Implementation/AppDevelopmentService.cs (1)
622-625
:⚠️ Potential issueFix enum-string comparisons.
Using string literals for reference types instead of enum values can lead to runtime errors.
Apply this diff to fix the issue:
-var updatedTasks = updatedReferences.Where(item => item.Type == "task").ToList(); -var updatedLayoutsSets = updatedReferences.Where(item => item.Type == "layoutSet").ToList(); -var updatedLayouts = updatedReferences.Where(item => item.Type == "page").ToList(); -var updatedComponents = updatedReferences.Where(item => item.Type == "component").ToList(); +var updatedTasks = updatedReferences.Where(item => item.Type == ReferenceType.Task).ToList(); +var updatedLayoutsSets = updatedReferences.Where(item => item.Type == ReferenceType.LayoutSet).ToList(); +var updatedLayouts = updatedReferences.Where(item => item.Type == ReferenceType.Layout).ToList(); +var updatedComponents = updatedReferences.Where(item => item.Type == ReferenceType.Component).ToList();
🧹 Nitpick comments (2)
backend/tests/Designer.Tests/Controllers/AppDevelopmentController/SaveFormLayoutTests.cs (1)
185-226
: Enhance test maintainability and debugging experience.The test implementation is well-structured but could benefit from the following improvements:
- Add descriptive error messages to assertions to aid debugging.
- Consider extracting the layout paths array to a constant since it's used in multiple tests.
Apply these changes to improve the test:
private static readonly string[] LayoutPaths = [ "component/layouts/Side1.json", "component/layouts/Side2.json", "component2/layouts/Side1.json", "component2/layouts/Side2.json" ]; [Theory] [InlineData("ttd", "testUser", "component", "Side2", "Input-Om7N3y", "Input-Om7N3y-new")] public async Task SaveFormLayoutWithUpdatedComponentName_UpdatesAssociatedSummary2Components_ReturnsOk(...) { // ... existing setup code ... - string[] layoutPaths = [ - "component/layouts/Side1.json", - "component/layouts/Side2.json", - "component2/layouts/Side1.json", - "component2/layouts/Side2.json" - ]; - layoutPaths.ToList().ForEach(file => + LayoutPaths.ToList().ForEach(file => { string actual = TestDataHelper.GetFileFromRepo(org, app, developer, $"App/ui/{file}"); string expected = TestDataHelper.GetFileFromRepo(org, expectedApp, developer, $"App/ui/{file}"); - JsonUtils.DeepEquals(actual, expected).Should().BeTrue(); + JsonUtils.DeepEquals(actual, expected).Should().BeTrue( + $"Layout file {file} does not match expected content after component update"); }); }backend/tests/Designer.Tests/Controllers/AppDevelopmentController/UpdateFormLayoutNameTests.cs (1)
71-103
: Enhance test coverage for layout name updates.The test verifies the happy path scenario but could be improved by:
- Adding test cases for error scenarios (e.g., invalid layout names, non-existent layouts).
- Verifying specific changes made to summary components instead of relying on file comparison.
- Parameterizing test data paths for better maintainability.
Consider adding test cases like:
[Theory] [InlineData("ttd", "testUser", "layout", "Side2", "")] // Empty layout name [InlineData("ttd", "testUser", "layout", "NonExistent", "NewName")] // Non-existent layout public async Task UpdateFormLayoutName_InvalidScenarios_ReturnsError(...)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
backend/src/Designer/Controllers/AppDevelopmentController.cs
(2 hunks)backend/src/Designer/Services/Implementation/AppDevelopmentService.cs
(6 hunks)backend/tests/Designer.Tests/Controllers/AppDevelopmentController/DeleteFormLayoutTests.cs
(1 hunks)backend/tests/Designer.Tests/Controllers/AppDevelopmentController/DeleteLayoutSetTests.cs
(1 hunks)backend/tests/Designer.Tests/Controllers/AppDevelopmentController/SaveFormLayoutTests.cs
(1 hunks)backend/tests/Designer.Tests/Controllers/AppDevelopmentController/UpdateFormLayoutNameTests.cs
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- backend/tests/Designer.Tests/Controllers/AppDevelopmentController/DeleteLayoutSetTests.cs
- backend/tests/Designer.Tests/Controllers/AppDevelopmentController/DeleteFormLayoutTests.cs
- backend/src/Designer/Controllers/AppDevelopmentController.cs
🔇 Additional comments (1)
backend/src/Designer/Services/Implementation/AppDevelopmentService.cs (1)
616-616
: LGTM!The public method follows the single responsibility principle and correctly delegates to the private implementation.
backend/tests/Designer.Tests/Controllers/AppDevelopmentController/SaveFormLayoutTests.cs
Outdated
Show resolved
Hide resolved
…pdate-all-summary-20-components-that-has-a-reference-to-updated-element-id-component-page-or-layout-set
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
backend/src/Designer/Services/Implementation/AppDevelopmentService.cs (1)
731-758
: Consider extracting Summary2 target update logic to a separate method.The nested conditional logic for handling Summary2 target updates is becoming complex. Consider extracting it to improve readability.
Example refactor:
- else - { - Reference updatedReference = null; - switch (type) - { - case "page": - updatedReference = updatedLayouts.FirstOrDefault(item => item.LayoutSetName == layoutSetId && item.Id == id); - break; - case "component": - updatedReference = updatedComponents.FirstOrDefault(item => item.LayoutSetName == layoutSetId && item.Id == id); - break; - } - if (updatedReference != null) - { - target["id"] = updatedReference.NewId; - hasLayoutChanges = true; - } - - if (!string.IsNullOrEmpty(taskId)) - { - updatedReference = updatedTasks.FirstOrDefault(item => item.Id == taskId); - if (updatedReference != null) - { - target["taskId"] = updatedReference.NewId; - hasLayoutChanges = true; - } - } - } + else + { + hasLayoutChanges |= UpdateSummary2Target(target, type, id, taskId, layoutSetId, updatedLayouts, updatedComponents, updatedTasks); + } + private bool UpdateSummary2Target(JsonObject target, string type, string id, string taskId, string layoutSetId, + List<Reference> updatedLayouts, List<Reference> updatedComponents, List<Reference> updatedTasks) + { + bool hasChanges = false; + Reference updatedReference = type switch + { + "page" => updatedLayouts.FirstOrDefault(item => item.LayoutSetName == layoutSetId && item.Id == id), + "component" => updatedComponents.FirstOrDefault(item => item.LayoutSetName == layoutSetId && item.Id == id), + _ => null + }; + + if (updatedReference != null) + { + target["id"] = updatedReference.NewId; + hasChanges = true; + } + + if (!string.IsNullOrEmpty(taskId)) + { + updatedReference = updatedTasks.FirstOrDefault(item => item.Id == taskId); + if (updatedReference != null) + { + target["taskId"] = updatedReference.NewId; + hasChanges = true; + } + } + + return hasChanges; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
backend/src/Designer/Enums/ReferenceType.cs
(1 hunks)backend/src/Designer/EventHandlers/ComponentIdChanged/ComponentIdChangedLayoutsHandler.cs
(3 hunks)backend/src/Designer/EventHandlers/LayoutPageIdChanged/LayoutPageIdChangedLayoutsHandler.cs
(1 hunks)backend/src/Designer/EventHandlers/LayoutSetIdChanged/LayoutSetIdChangedLayoutsHandler.cs
(1 hunks)backend/src/Designer/EventHandlers/ProcessTaskIdChanged/ProcessTaskIdChangedLayoutsHandler.cs
(1 hunks)backend/src/Designer/Services/Implementation/AppDevelopmentService.cs
(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- backend/src/Designer/EventHandlers/LayoutPageIdChanged/LayoutPageIdChangedLayoutsHandler.cs
- backend/src/Designer/EventHandlers/LayoutSetIdChanged/LayoutSetIdChangedLayoutsHandler.cs
- backend/src/Designer/EventHandlers/ComponentIdChanged/ComponentIdChangedLayoutsHandler.cs
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build environment and run e2e test
🔇 Additional comments (6)
backend/src/Designer/EventHandlers/ProcessTaskIdChanged/ProcessTaskIdChangedLayoutsHandler.cs (2)
16-21
: LGTM! Good refactoring to use IAppDevelopmentService.The constructor changes improve the design by:
- Removing direct git repository dependency
- Using a higher-level service abstraction
26-34
: LGTM! Clean implementation with proper error handling.The implementation:
- Uses proper error handling with ExecuteWithExceptionHandlingAndConditionalNotification
- Correctly creates a Reference for task ID update
- Properly handles cancellation token
backend/src/Designer/Services/Implementation/AppDevelopmentService.cs (4)
616-626
: LGTM! Clean separation of reference types.The code effectively categorizes references by type and update status.
671-679
: LGTM! Proper component ID update handling.The code correctly:
- Checks for component updates
- Updates component IDs
- Tracks layout changes
702-710
: LGTM! Clean layout set reference update handling.The code properly:
- Checks for layout set updates
- Updates layout set references
- Tracks changes
771-779
: LGTM! Proper override component update handling.The code correctly:
- Checks for component updates in overrides
- Updates component IDs
- Tracks changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
backend/tests/Designer.Tests/Controllers/AppDevelopmentController/UpdateFormLayoutNameTests.cs (1)
79-86
: Consider extracting the media type into a constant.The implementation is correct, but for better maintainability, consider extracting
MediaTypeNames.Application.Json
into a constant since it's used in multiple test methods.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
backend/tests/Designer.Tests/Controllers/AppDevelopmentController/SaveFormLayoutTests.cs
(1 hunks)backend/tests/Designer.Tests/Controllers/AppDevelopmentController/UpdateFormLayoutNameTests.cs
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/tests/Designer.Tests/Controllers/AppDevelopmentController/SaveFormLayoutTests.cs
🧰 Additional context used
🪛 GitHub Check: Run dotnet build and test (macos-latest)
backend/tests/Designer.Tests/Controllers/AppDevelopmentController/UpdateFormLayoutNameTests.cs
[failure] 97-97:
'string[]' does not contain a definition for 'ToList' and no accessible extension method 'ToList' accepting a first argument of type 'string[]' could be found (are you missing a using directive or an assembly reference?)
[failure] 97-97:
'string[]' does not contain a definition for 'ToList' and no accessible extension method 'ToList' accepting a first argument of type 'string[]' could be found (are you missing a using directive or an assembly reference?)
🪛 GitHub Actions: Build and Test on windows, linux and macos
backend/tests/Designer.Tests/Controllers/AppDevelopmentController/UpdateFormLayoutNameTests.cs
[error] 97-97: 'string[]' does not contain a definition for 'ToList' and no accessible extension method 'ToList' accepting a first argument of type 'string[]' could be found (are you missing a using directive or an assembly reference?)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Format check
- GitHub Check: Typechecking and linting
- GitHub Check: CodeQL
- GitHub Check: Testing
- GitHub Check: Generate stats on configuration coverage
🔇 Additional comments (1)
backend/tests/Designer.Tests/Controllers/AppDevelopmentController/UpdateFormLayoutNameTests.cs (1)
71-78
: LGTM! Well-structured test setup.The test method follows good testing practices with a clear naming convention and proper test data preparation.
backend/tests/Designer.Tests/Controllers/AppDevelopmentController/UpdateFormLayoutNameTests.cs
Show resolved
Hide resolved
backend/tests/Designer.Tests/Controllers/AppDevelopmentController/UpdateFormLayoutNameTests.cs
Show resolved
Hide resolved
9512a35
to
b3ee370
Compare
The branch name is too long and can't be deployed to dev. Moving it to #14489 |
Description
Update all Summary 2.0 components that has a reference to an updated component id
Related Issue(s)
Verification
Summary by CodeRabbit
Based on the comprehensive summary, here are the concise release notes:
New Features
Improvements
Bug Fixes
The changes primarily focus on enhancing the backend's capability to manage and update application layouts with greater flexibility and reliability.