NAS-140339 / 27.0.0-BETA.1 / Enable / disable toggles for shares on exported pools in sharing form appear active but don't do anything

[AlexKarpov98]

Testing: see ticket.

Preview:

NAS-140339.mov

[bugclerk]

Jira URL: https://ixsystems.atlassian.net/browse/NAS-140339

[claude]

Claude finished @AlexKarpov98's task in 2m 40s —— View job

---

Review of PR #13365

• Read review guidelines
• Read all changed files
• Analyze code changes
• Post review feedback

---

Really solid work! The utility extraction into share-exported-pool.utils.ts is clean, the globalStore deduplication fix is a nice improvement, and the test coverage across all four components is thorough 👏

Global Store Deduplication — Potential Race Condition

The in-flight deduplication added to global-store.service.ts is a great improvement to prevent multiple concurrent pool.query calls. However, there's a subtle issue with the interaction between invalidate() and in-flight requests.

Consider this sequence:

1. Subscriber A triggers call → API request starts, callInFlight$ is set
2. invalidate() is called → callResult$ emits undefined, callInFlight$ set to null
3. The in-flight observable from step 1 still completes → tap fires, sets callResult$ to old/stale data and sets callInFlight$ to null
4. Meanwhile, any new subscriber in step 2's switchMap will create a new request (correct), but subscribers from step 1 might receive stale data via the shareReplay

Because switchMap is used on callResult$, step 2's emission of undefined will cause existing subscribers to unsubscribe from the step-1 observable and resubscribe to a new one. So this is actually mostly safe thanks to switchMap's cancellation semantics. But refCount: false on shareReplay means the source subscription stays alive even after all downstream subscribers switch away, so the tap side-effect in step 3 will still fire, writing a stale result into callResult$ after invalidation. A subsequent subscriber would then get stale cached data instead of making a fresh API call.

This may not be easily triggered in practice (the timing window is narrow), but it could cause hard-to-debug staleness issues. A fix would be to use a generation counter or check if the in-flight observable is still the current one before writing to callResult$:

// In the tap callback:
tap((result) => {
  if (this.callInFlight$ === currentInFlight$) {
    this.callResult$.next(result);
    this.callInFlight$ = null;
  }
})

Not blocking, but worth considering for robustness.

Fix this →

Toggle & Action disabled Callbacks Are Evaluated Eagerly

In smb-card.component.ts:143 and smb-card.component.ts:150, the disabled callbacks for the "Edit Share ACL" and "Edit Filesystem ACL" actions return of(...) wrapping a synchronous check:

disabled: (row) => of(isShareUnavailable(row, this.activePoolPaths())),

This works correctly because this.activePoolPaths() reads the signal at call time. However, when the table calls this function it gets a "snapshot" observable that never updates. If activePoolPaths changes while the menu is open, the disabled state won't reflect the new value. This is fine for the current UX (pools don't change while a menu is visible), just noting the pattern for awareness.

Consistency Note: SMB Card vs NFS Card Edit Action

In smb-card.component.ts:137, the Edit action has a disabled callback tied to loadingMap$:

disabled: (row) => this.loadingMap$.pipe(map((ids) => Boolean(ids.get(row.id)))),

But in nfs-card.component.ts:112-127, the NFS card's Edit action has no disabled callback at all. This means users can click Edit on an NFS share while a toggle update is in-flight, which could lead to editing stale data. Worth considering for consistency, though it's a pre-existing pattern.

Overall

Clean, well-structured PR. The utility functions are well-tested, the approach of using a shared poolStore across components is sound, and the test coverage for active/exported/locked scenarios is comprehensive.

[codecov]

Codecov Report

❌ Patch coverage is 90.20979% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.94%. Comparing base (384f44b) to head (fccd387).
⚠️ Report is 10 commits behind head on master.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
...ts/shares-dashboard/smb-card/smb-card.component.ts	80.76%	5 Missing ⚠️
...p/pages/sharing/smb/smb-list/smb-list.component.ts	84.61%	4 Missing ⚠️
...ts/shares-dashboard/nfs-card/nfs-card.component.ts	84.61%	2 Missing ⚠️
...p/pages/sharing/nfs/nfs-list/nfs-list.component.ts	84.61%	2 Missing ⚠️
...s-with-menu/ix-cell-actions-with-menu.component.ts	83.33%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #13365      +/-   ##
==========================================
+ Coverage   86.93%   86.94%   +0.01%     
==========================================
  Files        1874     1875       +1     
  Lines       71943    72034      +91     
  Branches     8417     8428      +11     
==========================================
+ Hits        62545    62633      +88     
- Misses       9398     9401       +3     

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

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[aervin]

Works well here. Just a note, the Storage page invalidates the pool cache, which results in 3 pool.query calls when navigating back to the Shares page. Doesn't need to block this PR though. Up to you.

Screen.Recording.2026-03-24.at.1.22.51.PM.mov

[AlexKarpov98]

@aervin Issue is addressed now:

Screen.Recording.2026-03-25.at.14.40.49.mov

[bugclerk]

This PR has been merged and conversations have been locked.
If you would like to discuss more about this issue please use our forums or raise a Jira ticket.