unrvld / ODP.VisitorGroups

Visitor Groups for Optimizley Data Platform
MIT License
2 stars 4 forks source link

Embed audience count in first query, and remove subsequent queries #13

Open stefanolsen opened 1 year ago

stefanolsen commented 1 year ago

This PR removes the individual audience size query. They are then replaced with an extra query parameter in the single audience list query.

This reduces the GraphQL query calls considerably, especially when an ODP account has many real-time segments.

davidknipe commented 1 year ago

Hey @stefanolsen - first up thanks so much for the contribution it's great to see you using this package and its great to have you part of the ODP community :)

I have tested this with a site that has lots of RTS' defined (328 in total) and it takes around ~8 to ~10s to populate the dropdown list with a slightly confusing pause while the population takes place (see below):

https://github.com/unrvld/ODP.VisitorGroups/assets/6765939/e7ad7cb5-c960-43a7-8968-91b3147a23b0

Question for the group: Do we think this would be acceptable for accounts with large numbers of RTS? Perhaps with a small number of groups we can use Stefan's new approach and then go with the async population approach if say there are >50 groups?

stefanolsen commented 1 year ago

@davidknipe What would be the average number of RTS in a customer configuration? Could it be configurable, whether or not to include the count in the titles?

andrewmarkham commented 1 year ago

I feel that the delay is too long and could cause confusion to the end user. I appreciate that not many people are likely to have this many RTS, but over time this may happen.

What is the rationale for the change, is it about trying to make the code more tidy?

stefanolsen commented 1 year ago

@andrewmarkham: The though was to make it tidier, reduce the number of API calls, and to reduce the number of cache entries.

@davidknipe: Maybe, we could make the whole logic optional by configuration? So the existing logic is kept, but can be turned off?