Skip to content
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 #14489

Merged

Conversation

mlqn
Copy link
Contributor

@mlqn mlqn commented Jan 22, 2025

Description

Update all Summary 2.0 components that has a reference to an updated component id

Related Issue(s)

Verification

  • Your code builds clean without any errors or warnings
  • Manual testing done (required)
  • Relevant automated test added (if you find this hard, leave it and we'll help out)

Summary by CodeRabbit

Based on the comprehensive summary of changes, here are the release notes:

  • New Features

    • Enhanced layout management with improved reference tracking
    • Added support for updating component, layout, and layout set identifiers
    • Introduced new event handling for layout and component changes
  • Improvements

    • Added cancellation token support for asynchronous operations
    • Expanded error handling and synchronization mechanisms
    • Improved flexibility in managing UI component references
  • Technical Updates

    • Added new event classes for tracking layout and component changes
    • Introduced additional error codes for synchronization processes
    • Enhanced AppDevelopmentService to handle complex reference updates
  • Testing

    • Added comprehensive test cases for layout and component updates
    • Expanded test coverage for reference management scenarios

mlqn added 30 commits November 21, 2024 12:52
…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
…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
mlqn added 12 commits January 16, 2025 14:49
…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
…pdate-all-summary-20-components-that-has-a-reference-to-updated-element-id-component-page-or-layout-set
…pdate-all-summary-20-components-that-has-a-reference-to-updated-element-id-component-page-or-layout-set
Copy link
Contributor

coderabbitai bot commented Jan 22, 2025

📝 Walkthrough

Walkthrough

This pull request introduces comprehensive changes to the Altinn Studio Designer's layout management system. The modifications enhance the handling of layout references, component updates, and event-driven architecture. Key changes include adding new event classes for layout and component ID changes, updating services to handle reference updates, and introducing new handler classes to manage these events. The changes support more robust tracking and synchronization of layout-related modifications across different components and layout sets.

Changes

File Change Summary
backend/src/Designer/Controllers/AppDevelopmentController.cs Updated method signatures for UpdateFormLayoutName and UpdateLayoutSetName to include CancellationToken. Added event publishing for layout name changes.
backend/src/Designer/Enums/ReferenceType.cs Added new Task enum value to ReferenceType
backend/src/Designer/EventHandlers/* Added new handler classes for component, layout page, layout set, and process task ID changes
backend/src/Designer/Events/* Introduced new event classes LayoutPageIdChangedEvent and LayoutSetIdChangedEvent
backend/src/Designer/Hubs/SyncHub/SyncErrorCodes.cs Added new error code constants for layout-related synchronization
backend/src/Designer/Services/Implementation/AppDevelopmentService.cs Enhanced UpdateLayoutReferences method to handle updated references for tasks, layout sets, layouts, and components

Possibly related PRs

Suggested labels

area/ui-editor, quality/code, skip-releasenotes, frontend, team/studio-domain1

Suggested reviewers

  • Jondyr
  • standeren

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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions github-actions bot added solution/studio/designer Issues related to the Altinn Studio Designer solution. quality/testing Tests that are missing, needs to be created or could be improved. backend labels Jan 22, 2025
@mlqn mlqn marked this pull request as ready for review January 22, 2025 12:43
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🧹 Nitpick comments (9)
backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/layoutSet2/layouts/Side2.json (1)

41-46: Consider adding text resource bindings for navigation buttons.

The NavigationButtons component has empty textResourceBindings. Consider adding text resources for better accessibility and internationalization support.

Example addition:

   "id": "NavigationButtons-5ukC2N",
   "showBackButton": true,
-  "textResourceBindings": {},
+  "textResourceBindings": {
+    "back": "navigation.back"
+  },
   "type": "NavigationButtons"
backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/layoutSet2/layouts/Side1.json (1)

47-50: Consider adding text resource bindings for navigation buttons.

The NavigationButtons component has empty textResourceBindings. Consider adding custom labels for better user experience.

backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/layout2/layouts/Side2.json (1)

1-49: Consider adding test documentation.

The layout structure is well-formed and serves its purpose as test data for verifying Summary2 component reference updates. Consider adding a README or comments to explain the test scenarios this file covers, particularly:

  • The empty layoutSet id case
  • The "Side2-new" rename scenario
  • The component reference update patterns
backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/component2/layouts/Side2.json (1)

1-3: Consider making the schema URL configurable.

Hardcoding the CDN URL and version number may lead to maintenance challenges when updates are needed. Consider moving this to a configuration file.

backend/src/Designer/Events/LayoutSetIdChangedEvent.cs (1)

8-11: Consider making event properties immutable

For better thread safety and to follow best practices, consider making the properties EditingContext, LayoutSetName, and NewLayoutSetName read-only by removing the setters and initializing them through a constructor.

Apply this diff:

 public class LayoutSetIdChangedEvent : INotification
 {
-    public AltinnRepoEditingContext EditingContext { get; set; }
-    public string LayoutSetName { get; set; }
-    public string NewLayoutSetName { get; set; }
+    public AltinnRepoEditingContext EditingContext { get; }
+    public string LayoutSetName { get; }
+    public string NewLayoutSetName { get; }

+    public LayoutSetIdChangedEvent(AltinnRepoEditingContext editingContext, string layoutSetName, string newLayoutSetName)
+    {
+        EditingContext = editingContext;
+        LayoutSetName = layoutSetName;
+        NewLayoutSetName = newLayoutSetName;
+    }
 }
backend/src/Designer/Events/LayoutPageIdChangedEvent.cs (1)

6-12: Add XML documentation for public members.

Please add XML documentation for the class and its properties to improve code maintainability and IDE support.

+/// <summary>
+/// Event raised when a layout page ID is changed.
+/// </summary>
 public class LayoutPageIdChangedEvent : INotification
 {
+    /// <summary>
+    /// Gets or sets the editing context.
+    /// </summary>
     public AltinnRepoEditingContext EditingContext { get; set; }
+    /// <summary>
+    /// Gets or sets the name of the layout set.
+    /// </summary>
     public string LayoutSetName { get; set; }
+    /// <summary>
+    /// Gets or sets the original layout name.
+    /// </summary>
     public string LayoutName { get; set; }
+    /// <summary>
+    /// Gets or sets the new layout name.
+    /// </summary>
     public string NewLayoutName { get; set; }
 }
backend/src/Designer/EventHandlers/LayoutPageIdChanged/LayoutPageIdChangedLayoutsHandler.cs (1)

13-14: Add XML documentation for the handler class.

Please add XML documentation to describe the purpose of this handler and its dependencies.

+/// <summary>
+/// Handles layout page ID change events by updating layout references.
+/// </summary>
+/// <param name="fileSyncHandlerExecutor">The file sync handler executor.</param>
+/// <param name="appDevelopmentService">The app development service.</param>
 public class LayoutPageIdChangedLayoutsHandler(IFileSyncHandlerExecutor fileSyncHandlerExecutor, IAppDevelopmentService appDevelopmentService) : INotificationHandler<LayoutPageIdChangedEvent>
backend/src/Designer/EventHandlers/LayoutSetIdChanged/LayoutSetIdChangedLayoutsHandler.cs (1)

13-14: Add XML documentation for the handler class.

Please add XML documentation to describe the purpose of this handler and its dependencies.

+/// <summary>
+/// Handles layout set ID change events by updating layout references.
+/// </summary>
+/// <param name="fileSyncHandlerExecutor">The file sync handler executor.</param>
+/// <param name="appDevelopmentService">The app development service.</param>
 public class LayoutSetIdChangedLayoutsHandler(IFileSyncHandlerExecutor fileSyncHandlerExecutor, IAppDevelopmentService appDevelopmentService) : INotificationHandler<LayoutSetIdChangedEvent>
backend/src/Designer/Hubs/SyncHub/SyncErrorCodes.cs (1)

13-13: Add XML documentation for new error codes.

Please add XML documentation for the new error codes to maintain consistency with other members.

+    /// <summary>
+    /// Error code for layout set ID change synchronization errors.
+    /// </summary>
     public const string LayoutSetIdChangedLayoutsSyncError = nameof(LayoutSetIdChangedLayoutsSyncError);
     
     // ... other constants ...

+    /// <summary>
+    /// Error code for layout page ID change synchronization errors.
+    /// </summary>
     public const string LayoutPageIdChangedLayoutsSyncError = nameof(LayoutPageIdChangedLayoutsSyncError);

Also applies to: 19-19

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3d752ba and 1c74423.

📒 Files selected for processing (31)
  • backend/src/Designer/Controllers/AppDevelopmentController.cs (2 hunks)
  • 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/Events/LayoutPageIdChangedEvent.cs (1 hunks)
  • backend/src/Designer/Events/LayoutSetIdChangedEvent.cs (1 hunks)
  • backend/src/Designer/Hubs/SyncHub/SyncErrorCodes.cs (1 hunks)
  • backend/src/Designer/Services/Implementation/AppDevelopmentService.cs (6 hunks)
  • backend/tests/Designer.Tests/Controllers/AppDevelopmentController/SaveFormLayoutTests.cs (1 hunks)
  • backend/tests/Designer.Tests/Controllers/AppDevelopmentController/UpdateFormLayoutNameTests.cs (3 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)
✅ Files skipped from review due to trivial changes (7)
  • 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-updating-references/App/ui/layout2/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-after-updating-references/App/ui/layoutSet-new/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-after-updating-references/App/ui/component2/Settings.json
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: Build environment and run e2e test
🔇 Additional comments (38)
backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/layoutSet2/layouts/Side2.json (5)

1-4: LGTM! Schema and structure are well-defined.

The layout follows the correct schema and structure for Altinn app frontend layouts.

Also applies to: 47-49


5-13: Verify the empty layoutSet target id.

The first Summary2 component has an empty target id for the layoutSet type. This might cause issues in resolving the correct layoutSet reference.

Please ensure that:

  1. The empty target id is intentional for test purposes
  2. If not, specify the correct layoutSet id that this Summary2 component should reference

14-22: LGTM! Cross-page reference is valid.

The Summary2 component correctly references the Side1 page for content aggregation.


23-31: Verify the self-referencing Summary2 component.

The Summary2 component references its own page (Side2). This could potentially create circular dependencies or infinite loops if not handled properly.

Please confirm that:

  1. The self-reference is intentional
  2. There are proper safeguards in place to prevent infinite loops

32-40: Verify the referenced Input component id.

Given that this PR's objective is to update component references, please verify that "Input-hqcYqo" is the correct, updated component id.

Run this script to verify the component reference:

✅ Verification successful

The Input component id "Input-hqcYqo" is correct

The component id remains unchanged between the original and updated versions of the application, confirming its validity.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for the Input component definition to verify its id
rg -g "*.json" '"id":\s*"Input-hqcYqo"' --type json

Length of output: 755

backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/layoutSet2/layouts/Side1.json (3)

6-8: Verify if the empty data model binding is intentional.

The Input component has an empty simpleBinding. This might cause issues if the component is expected to capture user input.


1-4: Verify if a newer schema version is available.

The layout is using schema version 1. Let's verify if this is the latest version.

✅ Verification successful

Schema version is correct

The schema version (v1) is consistent with all layout files in the repository, including templates and test data. This appears to be the current standard version used with the Altinn app frontend toolkit v4.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for the latest schema version in the repository
# Look for other schema versions in layout files
rg -g '*.json' '"https://altinncdn.no/toolkits/altinn-app-frontend/[0-9]+/schemas/json/layout/layout.schema.v[0-9]+.json"' -A 1

Length of output: 14379


37-45: Verify the component reference integrity.

The Summary2 component references "Input-wrspcN". While this component exists in the current layout, we should verify if this reference remains valid after component updates.

✅ Verification successful

Component reference integrity is valid

The "Input-wrspcN" component exists and is properly defined in the same layout file where it's referenced by the Summary2 component, maintaining referential integrity across all test scenarios.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for references to the Input component across layouts
rg -g '*.json' '"Input-wrspcN"' -B 2 -A 2

Length of output: 5171

backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/layout2/layouts/Side2.json (4)

1-4: LGTM! Schema reference and structure are valid.

The schema URL and overall structure follow the expected standards.


41-46: LGTM! NavigationButtons configuration is valid.

The component configuration is minimal but correctly structured with appropriate defaults.


24-28: Verify target page "Side2-new" exists.

The target references a page with "-new" suffix, suggesting this is part of a rename test. Please verify that corresponding test cases exist for this scenario.

✅ Verification successful

Test coverage for "Side2-new" page is verified

The target page "Side2-new" is part of a properly structured test scenario, with explicit test coverage in UpdateFormLayoutNameTests.cs and corresponding test data files.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for test files containing "Side2-new" to verify test coverage
rg "Side2-new" backend/tests/Designer.Tests/

Length of output: 742


6-10: Verify if empty layoutSet id is intentional.

The target layoutSet has an empty id. While this might be intentional for test data, please verify if this accurately represents the test scenario.

✅ Verification successful

Empty layoutSet id is intentional in test data

The empty id is a deliberate pattern used consistently across multiple test files in this reference updating test scenario. This is verified by finding the same empty id pattern in all related test components (Side1.json and Side2.json) across different directories (layout, layout2, component, component2, layoutSet2).

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for other test files with empty layoutSet ids to confirm if this is a common pattern
rg -l '"type": "layoutSet".*"id": ""' backend/tests/Designer.Tests/_TestData

Length of output: 78


Script:

#!/bin/bash
# Try different patterns to match empty layoutSet id
echo "=== Searching with flexible whitespace ==="
rg '"type"\s*:\s*"layoutSet".*?"id"\s*:\s*""' backend/tests/Designer.Tests/_TestData

echo -e "\n=== Searching with reversed property order ==="
rg '"id"\s*:\s*"".*?"type"\s*:\s*"layoutSet"' backend/tests/Designer.Tests/_TestData

echo -e "\n=== List all layoutSet targets for context ==="
rg -C2 '"type"\s*:\s*"layoutSet"' backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references

Length of output: 7706

backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/component2/layouts/Side2.json (4)

6-13: Verify the empty layoutSet id.

The layoutSet target has an empty id which could cause issues. Please verify if this is intentional or if it should reference a specific layout set.


31-37: Component reference update looks good.

The updated component ID "Input-Om7N3y-new" is consistently referenced in both the overrides section and as a direct target, which aligns with the PR's objective of updating component references.

Also applies to: 48-55


1-64: Overall layout structure is well-organized.

The layout implementation is clean, consistent, and properly supports the PR's objective of updating component references. Each component has a unique ID, and the targeting system is used correctly.


57-61: Verify NavigationButtons text resources.

The NavigationButtons component has empty textResourceBindings. Please verify if text resources for button labels should be defined.

backend/src/Designer/EventHandlers/ProcessTaskIdChanged/ProcessTaskIdChangedLayoutsHandler.cs (1)

32-32: ⚠️ Potential issue

Fix syntax error in list initialization

The initialization of referencesToUpdate at line 32 is incorrect. In C#, you should initialize a List<T> using the new keyword and curly braces { }. The current syntax List<Reference> referencesToUpdate = [ ... ]; is invalid.

Apply this diff to fix the syntax error:

-                List<Reference> referencesToUpdate = [new Reference(ReferenceType.Task, null, notification.OldId, notification.NewId)];
+                var referencesToUpdate = new List<Reference>
+                {
+                    new Reference(ReferenceType.Task, null, notification.OldId, notification.NewId)
+                };

Likely invalid or redundant comment.

backend/src/Designer/Enums/ReferenceType.cs (1)

5-5: Approved: Addition of 'Task' enum value

The addition of the Task value to the ReferenceType enum is appropriate and aligns with the requirements.

backend/src/Designer/Events/LayoutSetIdChangedEvent.cs (1)

1-11: Approved: Addition of 'LayoutSetIdChangedEvent' class

The new event class LayoutSetIdChangedEvent is appropriately defined and follows the conventions.

backend/tests/Designer.Tests/Controllers/AppDevelopmentController/UpdateFormLayoutNameTests.cs (1)

Line range hint 73-181: LGTM! The test implementation is thorough and well-structured.

The test effectively verifies that:

  1. Layout name updates are successful
  2. Associated Summary 2.0 components are correctly updated
  3. File contents match the expected state after the update
backend/src/Designer/EventHandlers/ComponentIdChanged/ComponentIdChangedLayoutsHandler.cs (2)

19-27: LGTM! Constructor properly initializes the new dependency.

The dependency injection and initialization are correctly implemented.


54-57: LGTM! Layout reference updates are properly handled.

The implementation correctly:

  1. Creates a reference for the component ID change
  2. Updates layout references using the service
  3. Combines the result with existing changes
backend/tests/Designer.Tests/Controllers/AppDevelopmentController/SaveFormLayoutTests.cs (1)

140-181: LGTM! The test implementation is comprehensive.

The test effectively verifies that:

  1. Component name updates are successful
  2. Associated Summary 2.0 components are correctly updated
  3. File contents match the expected state after the update
backend/src/Designer/Controllers/AppDevelopmentController.cs (2)

204-221: LGTM! The UpdateFormLayoutName method changes are well-implemented.

The changes correctly:

  1. Add cancellation support
  2. Publish layout page ID change events

412-417: LGTM! The UpdateLayoutSetName method changes are well-implemented.

The changes correctly:

  1. Add cancellation support
  2. Publish layout set ID change events
backend/src/Designer/Services/Implementation/AppDevelopmentService.cs (5)

616-616: LGTM! Good separation of concerns.

The code clearly separates deleted and updated references, making it easier to handle different types of updates (tasks, layout sets, layouts, and components).

Also applies to: 622-626


Line range hint 646-679: LGTM! Robust component ID update logic.

The implementation correctly:

  1. Filters components by layout set
  2. Handles component deletion
  3. Updates component IDs with new values

702-711: LGTM! Proper handling of Subform components.

The code appropriately updates layout set references in Subform components, maintaining consistency across the application.


731-758: LGTM! Comprehensive Summary2 component updates.

The implementation thoroughly handles all reference types in Summary2 components:

  1. Updates target.id for both page and component references
  2. Updates target.taskId for task references
  3. Maintains layout set context for proper reference resolution

771-779: LGTM! Proper handling of component overrides.

The code correctly updates component IDs in the overrides section of Summary2 components.

backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/layoutSet-new/layouts/Side1.json (1)

1-13: LGTM! Valid layout configuration.

The layout follows the schema and contains a properly configured NavigationButtons component.

backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/layout/layouts/Side2-new.json (1)

1-20: LGTM! Valid layout configuration.

The layout follows the schema and contains properly configured Input and NavigationButtons components.

backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/layoutSet-new/layouts/Side2.json (1)

1-20: LGTM! Valid layout configuration.

The layout follows the schema and contains properly configured Input and NavigationButtons components.

backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/component/layouts/Side2.json (1)

6-11: Verify if empty simpleBinding is intentional for test data.

The Input component with ID "Input-Om7N3y-new" has an empty simpleBinding. While this might be intentional for test data, it's good to confirm if this accurately represents the test scenario.

backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/layout/layouts/Side1.json (1)

33-40: Verify component reference exists.

The Summary2 component references "Input-AVRSNf" but this component isn't defined in Side2.json. Verify if this component exists in other layout files.

Run this script to find the referenced component:

✅ Verification successful

Component reference "Input-AVRSNf" exists in Side2-new.json

The referenced component exists in the same app version at: backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/layout/layouts/Side2-new.json

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for Input-AVRSNf component across all layout files
find . -name "*.json" -type f -exec sh -c '
  echo "=== $1 ==="
  jq -r ".data.layout[] | select(.id == \"Input-AVRSNf\") | .id" "$1"
' sh {} \;

Length of output: 103870

backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/layout2/layouts/Side1.json (1)

13-19: Verify if empty layoutSet target is intentional.

The Summary2 component has empty "id" and "taskId" fields in its layoutSet target. While this might be intentional for test data, confirm if this accurately represents the test scenario.

backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/component2/layouts/Side1.json (1)

1-56: LGTM!

The layout structure is consistent with other files and correctly references its components.

backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-summary2-components-after-updating-references/App/ui/component/layouts/Side1.json (1)

1-64: LGTM!

The layout properly references components from Side2.json and includes appropriate display type overrides.

Copy link
Member

@Jondyr Jondyr left a comment

Choose a reason for hiding this comment

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

LGTM ✅, great work!

Copy link

codecov bot commented Jan 24, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.69%. Comparing base (b44b5c5) to head (28f7523).
Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #14489   +/-   ##
=======================================
  Coverage   95.69%   95.69%           
=======================================
  Files        1902     1902           
  Lines       24714    24714           
  Branches     2829     2829           
=======================================
  Hits        23651    23651           
  Misses        802      802           
  Partials      261      261           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@lassopicasso lassopicasso left a comment

Choose a reason for hiding this comment

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

Tested in dev 🚀 🥇

@lassopicasso lassopicasso merged commit bed2631 into main Jan 24, 2025
25 checks passed
@lassopicasso lassopicasso deleted the 14037-automatically-update-all-summary-20-components branch January 24, 2025 12:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend quality/testing Tests that are missing, needs to be created or could be improved. solution/studio/designer Issues related to the Altinn Studio Designer solution.
Projects
None yet
3 participants