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: Auto-update taskId in layout files when taskId is updated from process editor #13648

Merged
merged 27 commits into from
Oct 15, 2024

Conversation

ErlingHauan
Copy link
Contributor

@ErlingHauan ErlingHauan commented Sep 26, 2024

Description

The purpose of this PR is to enable automatic updates to the taskId reference in the Summary2 config, when taskId is changed in the process editor.

When updating taskId from the process editor, the backend raises a notification that contains the old and the new taskId.
A new handler then iterates through every field in every layout file, until it finds a taskId property. If the taskId in the layout matches the old taskId in the notification, it gets replaced by the new.

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)

@github-actions github-actions bot added the solution/studio/designer Issues related to the Altinn Studio Designer solution. label Sep 26, 2024
@ErlingHauan ErlingHauan changed the title Add event handler for Task IDs in layouts feat: automatically change task ids in layout files when updated from process editor Sep 26, 2024
@ErlingHauan ErlingHauan linked an issue Sep 26, 2024 that may be closed by this pull request
2 tasks
@ErlingHauan ErlingHauan changed the title feat: automatically change task ids in layout files when updated from process editor feat: Automatically update taskId in layout files when taskId is updated from process editor Sep 26, 2024
@github-actions github-actions bot added the quality/testing Tests that are missing, needs to be created or could be improved. label Sep 27, 2024
@ErlingHauan ErlingHauan marked this pull request as ready for review September 27, 2024 12:49
Copy link

codecov bot commented Sep 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.57%. Comparing base (43fdf78) to head (7f3e491).
Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #13648   +/-   ##
=======================================
  Coverage   94.57%   94.57%           
=======================================
  Files        1623     1623           
  Lines       21798    21798           
  Branches     2570     2570           
=======================================
  Hits        20616    20616           
  Misses        939      939           
  Partials      243      243           

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

@ErlingHauan ErlingHauan changed the title feat: Automatically update taskId in layout files when taskId is updated from process editor feat: Auto-update taskId in layout files when taskId is updated from process editor Sep 27, 2024
@Jondyr
Copy link
Member

Jondyr commented Oct 14, 2024

Tested and seems to work in general except for one issue around interactions with PR #13752:

taskid-summary2-select.mp4

Selecting a summary2 component after changing the referenced taskId throws an error. Does not seem to happen after uploading changes, so might be a cache invalidation issue?

@ErlingHauan
Copy link
Contributor Author

@Jondyr I'll take a look. Thanks for providing the video with console 😄

@ErlingHauan
Copy link
Contributor Author

@Jondyr It's fixed now.
Had to invalidate FormLayouts, which gets updated in the backend when updating taskId in process editor.
For good measure, I also invalidated LayoutSets, since it also contains the updated taskId.

Spiller.inn.2024-10-14.111610.mp4

@Jondyr
Copy link
Member

Jondyr commented Oct 14, 2024

@ErlingHauan Re-tested locally and I can confirm it now works 👍

But while retesting I noticed another issue. When a subForm layoutset is present in the project, changing a taskId returns an error from the sync-hub websocket:

{
	"type":1,
	"target":"FileSyncError",
	"arguments": [{
		"errorCode":"LayoutSetsTaskIdSyncError",
		"source":{"name":"layout-sets.json","path":"App/ui/layout-sets.json"},
		"details":"Object reference not set to an instance of an object."
	}]
}

@ErlingHauan
Copy link
Contributor Author

@Jondyr
A null check was missing before trying to access layoutSet.Tasks.
I re-tested now, and it looks fine in the websocket. Can you test one more time, just to be sure?

@Jondyr
Copy link
Member

Jondyr commented Oct 15, 2024

@ErlingHauan re-tested, looks good now ✅

@github-actions github-actions bot added the area/ui-editor Area: Related to the designer tool for assembling app UI in Altinn Studio. label Oct 15, 2024
@ErlingHauan
Copy link
Contributor Author

@Jondyr
As discussed, the invalidateQueries in useBpmnMutation has been reverted, and the path in ProcessTaskIdChangedLayoutsHandler has been set to the layouts folder.

Additionally, the forEach loop outside ExecuteWithExceptionHandlingAndConditionalNotification has now been moved inside it:

foreach (string layoutSetName in layoutSetsFile.Sets.Select(layoutSet => layoutSet.Id))
        {
            string[] layoutNames;
            try
            {
                layoutNames = repository.GetLayoutNames(layoutSetName);
            }
            catch (FileNotFoundException)
            {
                continue;
            }

            await _fileSyncHandlerExecutor.ExecuteWithExceptionHandlingAndConditionalNotification(
                notification.EditingContext,
                SyncErrorCodes.LayoutTaskIdSyncError,
                $"App/ui/{layoutSetName}/layouts",
                async () =>
                {
                    bool hasChanged = false;

                    foreach (string layoutName in layoutNames)
                    {
                        var layout = await repository.GetLayout(layoutSetName, layoutName, cancellationToken);
                        if (TryChangeLayoutTaskIds(layout, notification.OldId, notification.NewId))
                        {
                            await repository.SaveLayout(layoutSetName, layoutName, layout, cancellationToken);
                            hasChanged = true;
                        }
                    }

                    return hasChanged;
                });
        }
Spiller.inn.2024-10-15.122842.mp4

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.

Tested ✅

@Jondyr Jondyr merged commit 0726885 into main Oct 15, 2024
16 checks passed
@Jondyr Jondyr deleted the summary2-taskid-event-handling branch October 15, 2024 11:58
@Jondyr Jondyr added skip-releasenotes Issues that do not make sense to list in our release notes and removed skip-releasenotes Issues that do not make sense to list in our release notes labels Oct 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ui-editor Area: Related to the designer tool for assembling app UI in Altinn Studio. backend frontend 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. team/studio-domain2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Backend: should add summary 2.0 to task-id sync-event
3 participants