Skip to content

Conversation

@Kovy95
Copy link
Contributor

@Kovy95 Kovy95 commented Oct 23, 2025

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:
  • I have made corresponding changes to the documentation:
    • Developer documentation
    • User Guides

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Improved the form builder's column count adjustments to properly initialize and manage layout configurations, ensuring smoother handling of form editing scenarios when data structures aren't yet present.

- fix the problem with columns, add datagroup if there is no one and set it number of cols
@Kovy95 Kovy95 requested review from mazarijuraj and tuplle October 23, 2025 11:55
@Kovy95 Kovy95 self-assigned this Oct 23, 2025
@coderabbitai
Copy link

coderabbitai bot commented Oct 23, 2025

Walkthrough

The component now imports DataGroup and LayoutType from @netgrif/petriflow. The changeCols method is updated to check whether a dataGroups array exists before updating column counts; if absent, it creates a new DataGroup with GRID layout instead of failing.

Changes

Cohort / File(s) Summary
Import and logic updates
src/app/form-builder/edit-panel/edit-panel.component.ts
Added imports for DataGroup and LayoutType; modified changeCols method to conditionally create a DataGroup when none exists, rather than assuming one is already present.

Sequence Diagram(s)

sequenceDiagram
    participant Caller
    participant changeCols
    participant dataGroups as transition.dataGroups

    Caller->>changeCols: changeCols(newCols)
    
    alt dataGroups exists with length > 0
        changeCols->>dataGroups: update dataGroups[0].cols
        dataGroups-->>changeCols: cols updated
    else dataGroups missing or empty
        changeCols->>changeCols: create new DataGroup<br/>(id: transId, layout: GRID,<br/>cols: newCols)
        changeCols->>dataGroups: push new DataGroup
        dataGroups-->>changeCols: DataGroup added
    end
    
    changeCols-->>Caller: complete
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Pre-merge checks

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The PR title "NAB-389 Cannot change the number of columns in form editor" directly corresponds to the main change in the pull request. The changeset addresses a bug where the form editor's column count functionality was broken, which is precisely what the title communicates. The title is clear, concise, and includes the issue ticket reference, making it easy for teammates scanning the history to quickly understand that this PR fixes a specific functionality issue with column editing in the form editor. The wording avoids vague terms and accurately represents the primary change being implemented.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 636a552 and 4142aaa.

📒 Files selected for processing (1)
  • src/app/form-builder/edit-panel/edit-panel.component.ts (2 hunks)
⏰ 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). (6)
  • GitHub Check: task-list-completed
  • GitHub Check: task-list-completed
  • GitHub Check: task-list-completed
  • GitHub Check: task-list-completed
  • GitHub Check: Build and test
  • GitHub Check: task-list-completed
🔇 Additional comments (2)
src/app/form-builder/edit-panel/edit-panel.component.ts (2)

6-6: LGTM: Import additions are appropriate.

The new imports (DataGroup and LayoutType) are necessary for creating a DataGroup instance with GRID layout in the changeCols method.

Also applies to: 14-14


229-236: The primary concern in the review comment—a potential runtime error at line 235—is not supported by evidence. Throughout the codebase, dataGroups is accessed directly without defensive checks (e.g., transition.dataGroups.length === 0 in gridster.service.ts line 224, t.dataGroups.forEach(...) in model.service.ts line 472), indicating that the Transition class always initializes dataGroups as an empty array. Therefore, the .push() call at line 235 will not throw a TypeError.

While the code could be improved by caching the getTransition() result to reduce redundant calls and improve readability, the underlying assumption about a runtime error is incorrect.

Likely an incorrect or invalid review comment.


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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants