Filtering by multiple custom properties

[macobo]

Changes

This PR adds support to allow:

• filtering by multiple custom properties (both frontend and backend)
• (Under behavioral properties view) filtering by other properties when custom property filter applied

Follow-up to #3708

Constraints:

• The same property cannot be filtered on multiple times (as that would require changing the request/query data schema)

Other notes:

• I'm not super happy with the UI/UX. Let me know if you have suggestions!
• I have yet to update the docs - looking to pair with someone for that?

Click here to see some screenshots!

Tests

• Automated tests have been added
• This PR does not require tests

Changelog

• Entry has been added to changelog
• This PR does not make a user-facing change

Documentation

• Docs have been updated
• This change does not need a documentation update

[RobertJoonas]

The code looks good to me, but before approving I'd like to verify the UI on staging 👍

[RobertJoonas]

Some more comments after having tested the UI out on staging.

• Looks like contains filter breaks when multiple clauses are selected.

E.g.:

will return 500 in all the reports.

• In the custom prop breakdown report we’re allowing the user to pick any property from the dropdown.

I think only the prop_keys that have hits should be returned. What I mean is, it should not be possible to break down by a prop and then only see the (none) value in the returned list.



I suspect line 137 in filter_suggestions.ex is to blame, since it’s calling Query.remove_event_filters(query, [:props]). I don’t think we should do that anymore.

[ukutaht]

One comment about UX. Take this case:

1. Add a filter for propA prop
2. Change prop breakdown key to propB
3. Click on one entry from the list

The result is that the filter for the propA filter gets overwritten. Was that intentional? I was expecting the UI to keep the propA filter and add the propB filter as well.

[macobo]

PR has been updated @RobertJoonas @ukutaht - please give it another look.

To make reviewing the changes easier here's an overview of new changes:

• I've added support for matches_multiple in c517520. Not part of the original scope of this PR, but as result of my changes it started throwing 500s instead of silently failing.
• We now show multiple filter boxes for property filters instead of chucking them into a single one. 5bdc9b8
• I've fixed some issues around adding/removing filters and related behavior: 6195a38
• Navigation from behavioral filters rows no longer removes other property filters: 4ecc344
• We no longer show properties which only have (none) values in properties behavior tab: d38928b

[RobertJoonas]

Nice, we're getting there! Not quite done with feedback yet, sorry... 😄

In addition to the inline comments I had about the code, I've tested things out locally, and noticed a couple of new things:

1. In the prop filter modal, the blue "Add filter" button/link boundaries are huge and it feels a little weird when you click on a blank space and it will append new input fields into the view.

2. It looks like prop_key suggestions are returned without the constraint of having them set up in site.allowed_event_props. We implemented this to "filter out garbage props" that we don't want displayed there. And another reason is to enforce customers to configure the props they want to see in the dashboard. I made sure it's working on prod, so I suppose it's introduced with this PR?

3. Question: Is it the intended behaviour that the selected property in the behaviours (property breakdown) section will change when you add/remove filters? When I interact with the filters, I don't really expect the breakdown property to change 🤔

Screen.Recording.2024-01-31.at.17.36.01.mov

[macobo]

Nice, we're getting there! Not quite done with feedback yet, sorry... 😄

Thank you for the review, no need to apologize, you are great! 🙇

1. In the prop filter modal, the blue "Add filter" button/link boundaries are huge and it feels a little weird when you click on a blank space and it will append new input fields into the view.

Fixed.

2. It looks like prop_key suggestions are returned without the constraint of having them set up in site.allowed_event_props. We implemented this to "filter out garbage props" that we don't want displayed there. And another reason is to enforce customers to configure the props they want to see in the dashboard. I made sure it's working on prod, so I suppose it's introduced with this PR?

That's odd - I didn't change anything here. I tried to reproduce by going to site settings > delete a prop > back to dashboard and the deleted property no longer showed.

3. Question: Is it the intended behaviour that the selected property in the behaviours (property breakdown) section will change when you add/remove filters? When I interact with the filters, I don't really expect the breakdown property to change 🤔

Good question. This behavior existed pre-this-pr to avoid showing a blank screen when user would like to break down by a property that's not available. I think this is an easy fix but requires a bit of back-and-forth - would you be okay if I fixed this in a separate PR (for easier reviewability)?

[RobertJoonas]

That's odd - I didn't change anything here. I tried to reproduce by going to site settings > delete a prop > back to dashboard and the deleted property no longer showed.

Hmm weird, I can't reproduce either anymore. You can ignore it for now then

Good question. This behavior existed pre-this-pr to avoid showing a blank screen when user would like to break down by a property that's not available. I think this is an easy fix but requires a bit of back-and-forth - would you be okay if I fixed this in a separate PR (for easier reviewability)?

Sure, makes sense!

[RobertJoonas]

Nice! Only docs left 👍

[ukutaht]

🚀