[NAB-389] Cannot change the number of columns in form editor

[Kovy95]

Description

• fix the problem with columns, add datagroup if there is no one and set it number of cols

Fixes NAB-389

Dependencies

none

Third party dependencies

• No new dependencies were introduced

Blocking Pull requests

There are no dependencies on other PR

How Has Been This Tested?

manually

Test Configuration

<Please describe configuration for tests to run if applicable, like program parameters, host OS, VM configuration etc. You can use >

Name	Tested on
OS	lunux mint 21
Runtime	20.18.0
Dependency Manager	10.8.2
Framework version	angular 17
Run parameters
Other configuration

Checklist:

• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• My changes have been checked, personally or remotely, with @...
• I have commented my code, particularly in hard-to-understand areas
• I have resolved all conflicts with the target branch of the PR
• I have updated and synced my code with the target branch
• I have added tests that prove my fix is effective or that my feature works
• New and existing tests pass locally with my changes:
  • Lint test
  • Unit tests
  • Integration tests
• I have checked my contribution with code analysis tools:
  • SonarCloud
  • Snyk
• I have made corresponding changes to the documentation:
  • Developer documentation
  • User Guides

Summary by CodeRabbit

• Refactor
  • Reorganized form builder data initialization to ensure default data groups are consistently created with proper grid layout configuration when not previously specified.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

FormBuilderComponent now injects SelectedTransitionService and adds logic to create a default DataGroup if a transition's dataGroups array is empty, setting layout to GRID with default column configuration. GridsterService removes its own default DataGroup creation logic and directly accesses dataGroups[0].

Changes

Cohort / File(s)	Summary
Default DataGroup Initialization src/app/form-builder/form-builder.component.ts	Injects SelectedTransitionService dependency. Adds runtime check to create default DataGroup with GRID layout and ModelerConfig.LAYOUT_DEFAULT_COLS if dataGroups array is empty. Adds imports for ModelerConfig, SelectedTransitionService, DataGroup, and LayoutType.
Default DataGroup Removal src/app/form-builder/gridster/gridster.service.ts	Removes automatic default DataGroup creation logic from addDataRef method. Cleans up imports by removing LayoutType from Petriflow imports and ComponentDef from field-list imports.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Verify FormBuilderComponent's default DataGroup initialization executes at the correct lifecycle stage and interacts properly with SelectedTransitionService.
• Confirm GridsterService's addDataRef method handles cases where dataGroups might be empty and doesn't cause runtime errors accessing dataGroups[0].
• Validate that all code paths accessing transition.dataGroups still function correctly with the relocated initialization logic.
• Check for any other components that may depend on GridsterService's prior default DataGroup creation behavior.

Pre-merge checks

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly addresses the main change: fixing the inability to change column numbers in the form editor by ensuring a default datagroup with configurable columns is created.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

src/app/form-builder/gridster/gridster.service.ts (2)

176-176: Runtime error if dataGroups[0] is undefined.

The type assertion as DataGroup doesn't protect against undefined values. If dataGroups[0] is undefined, this line will throw a runtime error.

Apply this diff to add a safety check:

-        (this.transition.dataGroups[0] as DataGroup).removeDataRef(id);
+        const dataGroup = this.transition.dataGroups[0] as DataGroup;
+        if (!dataGroup) {
+            throw new Error('Cannot remove DataRef: transition has no DataGroup');
+        }
+        dataGroup.removeDataRef(id);

Or use optional chaining if the operation should silently fail:

-        (this.transition.dataGroups[0] as DataGroup).removeDataRef(id);
+        (this.transition.dataGroups[0] as DataGroup)?.removeDataRef(id);

---

224-224: Add defensive check before calling addDataRef() on line 224.

Line 224 directly accesses transition.dataGroups[0] without verifying the array is populated. Since default DataGroup initialization was removed from this service, this will throw a runtime error if FormBuilderComponent hasn't initialized dataGroups. Line 154 already uses optional chaining as the correct pattern in this file.

Apply optional chaining:

-        transition.dataGroups[0].addDataRef(dataRef);
+        transition.dataGroups[0]?.addDataRef(dataRef);

Note: Line 176 has the same issue and should also be updated:

-        (this.transition.dataGroups[0] as DataGroup).removeDataRef(id);
+        (this.transition.dataGroups[0] as DataGroup)?.removeDataRef(id);

📜 Review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4142aaa and 62182da.

📒 Files selected for processing (2)

• src/app/form-builder/form-builder.component.ts (2 hunks)
• src/app/form-builder/gridster/gridster.service.ts (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

src/app/form-builder/form-builder.component.ts (2)

src/app/modeler/services/canvas/canvas-listener-tool.ts (2)

• router (424-426)
• transitionService (428-430)

src/app/modeler/modeler-config.ts (1)

• ModelerConfig (3-40)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Build and test

🔇 Additional comments (2)

src/app/form-builder/form-builder.component.ts (2)

5-7: LGTM!

The new imports are appropriate for the default DataGroup creation logic.

---

18-18: LGTM!

The SelectedTransitionService injection follows proper Angular patterns and is necessary for accessing the current transition.