Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(autocomplete): Ignore missing aliases and return whether the property list is complete #20334

Merged
merged 2 commits into from
Feb 14, 2024

Conversation

Gilbert09
Copy link
Member

Problem

  • On teams with a lot of event properties, we don't return all the properties to the client
  • Using the query from the kea logic wasn't always up to date
  • Using an alias that didn't exist in the query would return the wrong suggestions

Changes

  • monaco-editor supports "incomplete" suggestion lists, meaning it'll query on every keystroke - this is enabled for teams with more than 220 properties
  • Use input from monaco-editor directly instead of kea logic
  • Ignore aliases that don't exist

How did you test this code?

  • unit tests + browser

@Gilbert09 Gilbert09 requested a review from a team February 14, 2024 11:39
Copy link
Collaborator

@mariusandra mariusandra left a comment

Choose a reason for hiding this comment

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

Why not, though why this 220 limit? I'd make everything go through the same path 🤷

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

  • chromium: 0 added, 0 modified, 0 deleted
  • webkit: 0 added, 1 modified, 0 deleted (diff for shard 2)

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@Gilbert09
Copy link
Member Author

Why not, though why this 220 limit? I'd make everything go through the same path 🤷

See the "Property Definitions" section of #20198 for why the 220 limit. The property definitions are the only things we want to limit right now to avoid sending 1m+ suggestions to the frontend

Copy link
Contributor

Size Change: 0 B

Total Size: 2.22 MB

ℹ️ View Unchanged
Filename Size
frontend/dist/toolbar.js 2.22 MB

compressed-size-action

@Gilbert09 Gilbert09 merged commit b8f51e9 into master Feb 14, 2024
79 checks passed
@Gilbert09 Gilbert09 deleted the tom/autocomplete-3 branch February 14, 2024 12:22
tkaemming pushed a commit that referenced this pull request Feb 14, 2024
…perty list is complete (#20334)

* Ignore missing aliases and return whether the property list is complete

* Update UI snapshots for `webkit` (2)

---------

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
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.

3 participants