Skip to content

fix(components/data-manager): address data mutation during view updates (#4309)#4311

Merged
johnhwhite merged 1 commit intomainfrom
cherry-pick_328efabc7ef53c81f59db64cc8619faa3b4c033f_1773766740603
Mar 17, 2026
Merged

fix(components/data-manager): address data mutation during view updates (#4309)#4311
johnhwhite merged 1 commit intomainfrom
cherry-pick_328efabc7ef53c81f59db64cc8619faa3b4c033f_1773766740603

Conversation

@blackbaud-sky-build-user
Copy link
Copy Markdown
Collaborator

@blackbaud-sky-build-user blackbaud-sky-build-user commented Mar 17, 2026

🍒 Cherry picked from #4309 fix(components/data-manager): address data mutation during view updates

AB#3705639

Summary by CodeRabbit

  • Refactor

    • Improved state management consistency through immutable state updates in the data manager, enhancing data integrity and reliability.
  • Tests

    • Expanded test coverage for edge cases in state handling and column configuration with improved test infrastructure.

…es (#4309)

[AB#3705639](https://dev.azure.com/blackbaud/f565481a-7bc9-4083-95d5-4f953da6d499/_workitems/edit/3705639)

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Tests**
* Added extensive test suite covering data state emission, in-place
mutations, view preservation with multiple views, and state options
cloning behavior to validate correct data handling.

* **Refactor**
* Improved internal state management by creating new state object copies
instead of mutating originals, enhancing data consistency and reducing
side effects throughout the module.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 17, 2026

📝 Walkthrough

Walkthrough

The pull request refactors the data-manager library and ag-grid adapter to use immutable-style state updates instead of in-place mutations. A new safeStructuredClone utility function is introduced to safely clone state properties, and multiple modules are updated to create new state instances when applying changes. Comprehensive tests validate the immutable state handling and cloning semantics.

Changes

Cohort / File(s) Summary
Ag-grid adapter directive
ag-grid-data-manager-adapter.directive.ts
Replaced in-place mutations of SkyDataManagerState with immutable updates by creating new state instances for selectedIds, activeSortOption, and view-level state (displayedColumnIds, columnWidths) in event handlers.
Data-manager service core
data-manager.service.ts
Introduces updatedViewState copies to replace direct mutations; creates new state instances when updating views via addOrUpdateView instead of mutating existing view state objects.
Data-manager state models
data-manager-state.ts, data-view-state.ts
Integrated safeStructuredClone utility for cloning public state properties; addOrUpdateView creates shallow copies of views array and returns new SkyDataManagerState instances; getViewStateOptions returns defensive copies of arrays and columnWidths objects.
Safe structured clone utility
safe-structured-clone.ts, safe-structured-clone.spec.ts
New internal utility function that safely clones values using structuredClone with graceful fallback to original value on failure or for non-cloneable types; includes comprehensive test coverage for edge cases.
Data-manager service tests
data-manager.service.spec.ts
Centralized subscription lifecycle management using unified Subscription tracking; added new "distinctUntilChanged with shared mutable state" test block validating immutable state handling, mutation behavior, and cloning semantics.
Data-view state tests
data-view-state.spec.ts
New test file validating columnWidths normalization in both SkyDataViewState constructor and getViewStateOptions to ensure xs and sm properties are always defined objects.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

Suggested reviewers

  • Blackbaud-SteveBrush
  • johnhwhite

Poem

🐰 With whiskers twitched and hop of glee,
No more mutations here shall be!
New states rise fresh, old refs untouched,
Immutable patterns we've clutched.
A safer grove for data's song! 🌿✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically identifies the main change: addressing data mutation issues during view updates in the data-manager component, which aligns with the core refactoring across multiple files to replace in-place mutations with immutable-style state updates.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch cherry-pick_328efabc7ef53c81f59db64cc8619faa3b4c033f_1773766740603
📝 Coding Plan
  • Generate coding plan for human review comments

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

@nx-cloud
Copy link
Copy Markdown

nx-cloud bot commented Mar 17, 2026

View your CI Pipeline Execution ↗ for commit 4ee10d4

Command Status Duration Result
nx build code-examples-playground --baseHref=ht... ✅ Succeeded 3m 57s View ↗
nx build playground --baseHref=https://blackbau... ✅ Succeeded 1m 47s View ↗
nx build integration --baseHref=https://blackba... ✅ Succeeded 34s View ↗

☁️ Nx Cloud last updated this comment at 2026-03-17 17:07:51 UTC

@blackbaud-sky-build-user
Copy link
Copy Markdown
Collaborator Author

@johnhwhite johnhwhite enabled auto-merge (squash) March 17, 2026 17:10
@johnhwhite johnhwhite merged commit 6cadb59 into main Mar 17, 2026
37 checks passed
@johnhwhite johnhwhite deleted the cherry-pick_328efabc7ef53c81f59db64cc8619faa3b4c033f_1773766740603 branch March 17, 2026 17:16
johnhwhite pushed a commit that referenced this pull request Mar 17, 2026
##
[14.0.0-alpha.9](14.0.0-alpha.8...14.0.0-alpha.9)
(2026-03-17)


### ⚠ BREAKING CHANGES

* **components/indicators:** replace `@angular/animations` in tokens
component with CSS transitions (#4303)
* **components/tabs:** replace `@angular/animations` with CSS
transitions (#4288)
* **components/flyout:** replace `@angular/animations` with CSS
transitions (#4285)

### Features

* **components/lists:** deprecate filter summary
([#4307](#4307))
([b5567d9](b5567d9)),
closes
[AB#3611503](https://dev.azure.com/blackbaud/Products/_workitems/edit/3611503)
* **components/modals:** add modal banner
([#4275](#4275))
([#4280](#4280))
([b186fc2](b186fc2))


### Bug Fixes

* **components/core:** transition handler emits via microtask when
animations disabled
([#4304](#4304))
([3b9cd2d](3b9cd2d))
* **components/data-manager:** address data mutation during view updates
([#4309](#4309))
([#4311](#4311))
([6cadb59](6cadb59)),
closes
[AB#3705639](https://dev.azure.com/blackbaud/Products/_workitems/edit/3705639)
* **components/flyout:** replace `@angular/animations` with CSS
transitions ([#4285](#4285))
([96aa341](96aa341))
* **components/indicators:** replace `@angular/animations` in tokens
component with CSS transitions
([#4303](#4303))
([bcb9d88](bcb9d88))
* **components/tabs:** replace `@angular/animations` with CSS
transitions ([#4288](#4288))
([90b183b](90b183b))

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Breaking Changes**
* Indicators, tabs, and flyout components now use CSS transitions
instead of animations.

* **New Features**
  * Added modal banner component.

* **Deprecations**
  * Filter summary in list components deprecated.

* **Bug Fixes**
* Animation behavior when disabled, data handling during updates, and
transition styling improvements.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk level (author): 1 No additional bugs expected from this change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants