Skip to content

Fix database condition selector observers#346

Merged
appflowy merged 1 commit into
mainfrom
fix/database-condition-observers
May 15, 2026
Merged

Fix database condition selector observers#346
appflowy merged 1 commit into
mainfrom
fix/database-condition-observers

Conversation

@appflowy
Copy link
Copy Markdown
Contributor

@appflowy appflowy commented May 15, 2026

Summary

  • make filter/sort selectors subscribe to the actual Yjs child arrays/maps they observe
  • handle filters/sorts arrays created after selector mount
  • update filter/sort list selectors for nested field-id changes without extra state churn
  • add regression coverage for filter/sort array creation and field changes

Tests

  • pnpm exec jest src/application/database-yjs/tests/useConditionSelectors.test.tsx src/application/database-yjs/tests/useRowOrdersSelector.test.tsx --no-coverage --runInBand
  • pnpm run type-check
  • pnpm exec eslint --quiet --ext .ts,.tsx src/application/database-yjs/selector.ts src/application/database-yjs/tests/useConditionSelectors.test.tsx src/application/database-yjs/tests/useRowOrdersSelector.test.tsx --ignore-path .eslintignore.web

Summary by Sourcery

Adjust database condition selector hooks to correctly observe Yjs filter and sort structures and keep derived state in sync with underlying field changes.

New Features:

  • Add shared condition reference type and equality helper for comparing filter and sort selector outputs.

Bug Fixes:

  • Ensure filter and sort list selectors subscribe to Yjs child arrays/maps and handle arrays created after hook mount.
  • Keep individual filter and sort selector values in sync when their associated field IDs or Yjs maps change, resetting to null when data is missing.
  • Prevent stale or redundant state updates in filter and sort selectors by avoiding updates when condition references are unchanged.

Enhancements:

  • Simplify selector hook dependencies to rely directly on Yjs view, filters, and sorts collections rather than view IDs.
  • Use deep observation on relevant Yjs collections to react to nested structure changes in filters and sorts.

Tests:

  • Add regression tests covering late creation of filter and sort arrays and field changes propagating through list and item selectors.

@sourcery-ai
Copy link
Copy Markdown

sourcery-ai Bot commented May 15, 2026

Reviewer's Guide

Adjusts database condition selector hooks to observe the correct Yjs child structures and react to dynamic creation/field changes, while adding regression tests for filters and sorts behavior.

Sequence diagram for updated filter observer behavior in condition selector hooks

sequenceDiagram
    actor UserComponent
    participant useFiltersSelector
    participant YjsView
    participant YjsFiltersArray

    UserComponent->>useFiltersSelector: mount
    useFiltersSelector->>YjsView: get(YjsDatabaseKey.views).get(viewId)
    useFiltersSelector->>YjsFiltersArray: view.get(YjsDatabaseKey.filters)
    alt filtersArray exists
        useFiltersSelector->>YjsFiltersArray: observeDeep(observerEvent)
        useFiltersSelector->>useFiltersSelector: getFilters()
        useFiltersSelector-->>UserComponent: setFilters(initial)
    else no filtersArray
        useFiltersSelector-->>UserComponent: setFilters([])
    end

    rect rg
    YjsFiltersArray-->>YjsFiltersArray: [child array/map changes]
    YjsFiltersArray-->>useFiltersSelector: observerEvent via observeDeep
    useFiltersSelector->>useFiltersSelector: getFilters()
    useFiltersSelector->>useFiltersSelector: areConditionReferencesEqual(prev, next)
    alt equal
        useFiltersSelector-->>UserComponent: keep previous filters
    else not equal
        useFiltersSelector-->>UserComponent: setFilters(next)
    end
    end

    UserComponent-->>useFiltersSelector: unmount
    useFiltersSelector->>YjsFiltersArray: unobserveDeep(observerEvent)
Loading

File-Level Changes

Change Details Files
Make filters/sorts list selectors subscribe to the underlying Yjs arrays/maps and avoid unnecessary React state updates.
  • Introduce a shared ConditionReference type and areConditionReferencesEqual helper to compare arrays of {id, fieldId} references and prevent redundant state updates.
  • Refactor useFiltersSelector and useSortsSelector to resolve the view and filters/sorts Yjs arrays outside useEffect, reset state to empty when arrays are missing, and use observeDeep/unobserveDeep to react to nested changes and late creation.
  • Update useEffect dependency arrays in list selectors to depend directly on the Yjs filters/sorts arrays instead of database/view identifiers.
src/application/database-yjs/selector.ts
Ensure individual filter/sort selectors observe the correct Yjs maps and handle missing data robustly.
  • Refactor useFilterSelector to resolve the specific filter map and fields map before the effect, early-returning to null when either is missing, and switch to fields.observeDeep and filter.observeDeep so changes to fieldId or field data propagate.
  • Update useRootFilterInfo, useAdvancedFiltersSelector, and useAdvancedFilterSelector to precompute view/filtersArray, handle absent arrays by resetting to defaults/null, and to rely on observeDeep for nested updates while narrowing effect dependencies to the Yjs structures they read.
  • Refactor useSortSelector to resolve the specific sort map prior to the effect, clear state when missing, and simplify observation to a direct observe/unobserve on the sort while narrowing dependencies to the sort reference.
src/application/database-yjs/selector.ts
Add regression tests covering filters/sorts array creation after mount and field-id change propagation into selectors.
  • Create a Yjs-based ConditionFixture helper that sets up a minimal database doc with fields, views, and a single view for testing condition hooks.
  • Add tests validating that useFiltersSelector and useSortsSelector pick up arrays created after hook mount and that their outputs reflect subsequent field_id changes.
  • Add tests validating that useFilterSelector and useSortSelector update their returned Filter/Sort objects when the underlying Yjs filter/sort field_id is changed.
src/application/database-yjs/__tests__/useConditionSelectors.test.tsx

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've left some high level feedback:

  • In areConditionReferencesEqual, rightItem is accessed as rightItem.fieldId without a null/undefined guard while rightItem?.id is guarded, which can throw when right is shorter than left; consider using rightItem?.fieldId or an early return when !rightItem.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In `areConditionReferencesEqual`, `rightItem` is accessed as `rightItem.fieldId` without a null/undefined guard while `rightItem?.id` is guarded, which can throw when `right` is shorter than `left`; consider using `rightItem?.fieldId` or an early return when `!rightItem`.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@appflowy appflowy merged commit dd49aea into main May 15, 2026
13 checks passed
@appflowy appflowy deleted the fix/database-condition-observers branch May 15, 2026 15:24
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.

1 participant