feat(flags): Add relative date operators

[neilkakkar]

Problem

This PR adds targeting by distinct id, group keys, and relative date targeting.

I feel I've introduced more hacks here than the last year combined, particularly because all of these are edge-casy things: all apply only to flags, so others shouldn't show them yet, and try to reuse standard components as much as possible at the same time.

Things seem much nicer now, although there are still some adjustments I've made to existing components to make this work nicely.

Along the way, fix some small bugs with the RollingDateRange component.

(@Twixes , @mariusandra - adding you for review if you know a better way to get these things done, since touches taxonomicFilters and the like)

See demo: https://posthog.slack.com/archives/C034XD440RK/p1705426858833459 (too large for github :P )

TODO in follow up:

1. Backend validate relative date op values
2. Add frontend tests
3. Figure out autocomplete for current distinct id, and group key, if possible
4. Make RollingDateRange onBlur/onSelect nicer, this is annoying for both insights & now flags - need to click on 'in the last' for it to take effect.

Changes

👉 Stay up-to-date with PostHog coding conventions for a smoother review.

How did you test this code?

[mariusandra]

Hola, nice going! Though it now feels like this should be split up in three.

PR 1:

• The first PR should just add a metadata column with only distinct_id in it.
• Adding about 3 lines of code to hogql/property.py's property_to_expr will make this metadata automatically usable in a lot of places (events list, refactored insights, etc). Would be nice 🙃.

PR 2:

• I'm a bit unconvinced of the UX with regards to selecting group keys 😅. This shouldn't block us from merging the fix for distinct ids.
• I'm also not sure of the $group_key field. Wouldn't it be better to stick to the actual SQL fields like $group_0?

• Even better, we already have aliases for group tables (last part of screenshot), why not add extra virtual fields like account_key as an alias to $group_0?
• This should require just a few lines one line in database.py
• This way we could show "Organization key" in the first part of the filter, and just the keys in the second part, making the experience meh, but ok.
• An autocomplete with actual names would be great, but I'm happy to leave that as PR 4 😅
• We can now also add "account_key", "organization_key", etc as entries into the metadata table.

PR 3:

• The date range improvements.
• These seem okay to me, though I'd defer a full review to someone with more recent frontend experience

[neilkakkar]

Agree with this being too much for one PR 😅 , will split into two: distinctID + groups; and relative date stuff.

I'm also not sure of the $group_key field. Wouldn't it be better to stick to the actual SQL fields like $group_0?

This might be true in events land, but isn't true in feature flags land when trying to query groups, because there is only a group key per group, no concept of $group_0.

A problem that will soon crop up with the metadata field is that given how taxonomic filters currently work, each pill converts to properties of the same type; so not too big a fan of putting it all in metadata here.

(Edit: The above is possibly easy to resolve, but I'm not sure yet what all will go in metadata here & where all it will be used, so refraining from over-optimising right now)

The current way for groups gives us autocomplete and searching by names as well, so 🙌 . Although, the meh bit is trying to then add a second key to the same filter, there autocomplete doesn't really work well.

---

I'm not really looking at it from the hogql side of things, and agree the current UX for groups wouldn't work in event filtering, specially since you're dealing with multiple groups on the same event, but this imo is too different from flags to try and incorporate into one.

[mariusandra]

Agree with this being too much for one PR 😅 , will split into two: distinctID + groups; and relative date stuff.

I'm also not sure of the $group_key field. Wouldn't it be better to stick to the actual SQL fields like $group_0?

This might be true in events land, but isn't true in feature flags land when trying to query groups, because there is only a group key per group, no concept of $group_0.

Aha... I understand better now.

We all do share the filtering interface, so let's figure out a way forward that works for both. I'd like to one day have a metadata field that has at least timestamp, event, distinct_id, person_id as "sort-of properties" you can filter by. People have been asking for them, and the current workaround is "use hogql", which isn't ideal considering we're talking about just adding another menu item.

I might also see adding things like $group_0 (aka first, second, etc group's key) into the list, but that hasn't been important so far.

I get that your metadata is based on a different structure, with some overlap. The question is, how can we make an interface that lets us do both? If $group_key is how the property is saved on the object you're comparing, I have of course nothing against you using that property in the filter.

However then, should we a) find a way to show/hide the metadata pill's fields based on some custom logic, b) just split this into flag_metadata and event_metadata? The second option probably makes sense then, or what do you think?

A problem that will soon crop up with the metadata field is that given how taxonomic filters currently work, each pill converts to properties of the same type; so not too big a fan of putting it all in metadata here.

There is also the group_type_index field on the data coming from the filter. Could this be used here? I'm not sure I fully understand.

[neilkakkar]

I'm kinda keen to keep one metadata pill here.

a) find a way to show/hide the metadata pill's fields based on some custom logic

Yes, I've incorporated this already in here, metadata currently will only show options loaded via optionsFromProps, so this wouldn't show up if you don't want it to show up. This means you could add $group_0 to this metadata and it will work fine everywhere, and for flags as well, because flags discard these / choose not to show these.

We can later also extend this into a propertyAllowList if need be.

For $group_key specifically, I opted not to use metadata but the "GroupNamesPrefix" taxonomic group because it seems to work better here - and wasn't used in taxonomic filters before, so I can customise it appropriately for flags.

[github-actions]

Size Change: +212 kB (+11%) ⚠️

Total Size: 2.21 MB

Filename	Size	Change
frontend/dist/toolbar.js	2.21 MB	+212 kB (+11%)	⚠️

_{compressed-size-action}

[neilkakkar]

Wait what, toolbar?? 👀

[mariusandra]

Not sure of the +200kb for toolbar though...?

[neilkakkar]

ah it's legit, it's because toolbar uses the DateFilter component as well, which now has another fixed date option, so it's bundled too.

[Twixes]

I think we're much closer in the UX and it's fine for a flagged version, but this will be confusing to users:

1. There are no "overrides" happening here, that's terminology from the dashboard date range selector. In other places, such as this, we should just say "Select a value" as the empty state.
2. The tooltip is lying, because this option does not mean "January 17". It means "in the 7 days preceding the flag being evaluated" (as far as I understand), so we should say something to that effect

[Twixes]

At this point, with all the nesting, it's pretty hard to read what's going on in this element and why. Can it be its own component?

[neilkakkar]

Will do

[neilkakkar]

There are no "overrides" happening here, that's terminology from the dashboard date range selector. In other places, such as this, we should just say "Select a value" as the empty state.

Yep makes sense 👍

The tooltip is lying, because this option does not mean "January 17". It means "in the 7 days preceding the flag being evaluated" (as far as I understand), so we should say something to that effect

Hmm, very good point, but I also want some computation here as a sanity check. Maybe something like 'Matches all values before 18 Jan if evaluated today' ? ehh.

---

In either case, will fix these before rolling out, want to get this in first to test SDKs / full e2e flow.