Add admin page to show current test track assignments #249
Conversation
stevenjackson
force-pushed
the
sj/show-assignments
branch
from
0182c8b to
258b5ef
Compare
stevenjackson
force-pushed
the
sj/show-assignments
branch
from
258b5ef to
e1acd30
Compare
| def index | ||
| split = Split.find(params.require(:split_id)) | ||
| @split_name = split.name | ||
| @assignments = split.assignments.includes(visitor: { identifiers: :identifier_type }).order(updated_at: :desc).limit(VIEW_LIMIT) |
There was a problem hiding this comment.
Someone might have a stronger opinion on whether an index is necessary here. I think it's optional, because we want to preserve insert/update efficiency, and this query is not in the critical path (it is loaded by admin users on demand)
There was a problem hiding this comment.
One thing that might make this slow on big assignment datasets is having an order by clause, which requires fetching the whole assignment set even in light of the limit clause. We could consider removing it. My guess is it'd make this very fast regardless of assignment count.
There was a problem hiding this comment.
I think that's reasonable. My only hesitation is our experience has been the assignments you care about most are the ones you manually assigned last week. But I'm already leaning YAGNI - either the data set is skimmable or you need search, there's not a lot of value in the in-between states.
There was a problem hiding this comment.
Yeah, if we see this as a scrappy solution for low-scale assignments it makes sense, and we can iterate if we see value.
There was a problem hiding this comment.
Sorting in ruby after retrieval might be a nice feature for skimmability under 1k rows, though it might mislead people to think that absence of a sorted row means something.
For longer running experiments with a specific audience in my mind (like preferred beta testers) it's useful to use the bulk assignments capability to enable those features. However if there are mulitple instances of "adding users to the beta", one doesn't have visibility into what assignments may already be present. This view is intended to give admins more confidence that the intended assignments are in place.
stevenjackson
force-pushed
the
sj/show-assignments
branch
from
e1acd30 to
2638862
Compare
Co-authored-by: Ivan Greene <ivan@ivan.sh>
The value of the sort is questionable - when the dataset is small you don't need it - when the dataset is huge it raises the query cost - when its several dozens of assignments you might see the value A future version could sort the rows after the query, but that could also be misleading on huge datasets.
| @@ -11,11 +11,12 @@ Betterment/NonStandardActions: | |||
| Exclude: | |||
| - 'config/routes.rb' | |||
|
|
|||
| # Offense count: 13 | |||
| # Offense count: 14 | |||
There was a problem hiding this comment.
Note to self I think this cop can be configured with models that aren't considered "owned" which we should use to knock this exclusion list out.
|
You said you didn't want to take on pagination, but does that have to be much harder than pulling in kaminari? I'm fine with landing this and adding pagination separately if the concern is mostly just a big ol' diff. Just feels oddly limiting to only show the first 1K with no way to see the rest. This also makes me think we should build a "view visitor" page where you can punch in an identifier and see the details for that visitor. |
|
I'm mostly not sure pagination makes sense either. If I have 570K overrides, how many pages would I click through? Agree the arbitrary limit feels strange. My other thought was perhaps allowing overriding limit via url param - if the admin really wants to comb through a 500K table, getting it in one view may be preferred. Then I like the idea of a visitor page! I think that gets much closer to what I would be curious about. I see use cases like:
|
|
Yea that tracks too. That's exactly what lead me to suggesting a way to look up an individual visitor. Well, if you find yourself working on that, I will def give it a review 🙂 Thanks for improving the admin experience! |
Summary
For longer running experiments with a specific audience in my mind (like preferred beta testers) it's useful to use the bulk assignments capability to enable those features. However if there are mulitple instances of "adding users to the beta", one doesn't have visibility into what assignments may already be present. This view is intended to give admins more confidence that the intended assignments are in place.
Other Information
I've arbitrarily selected a limit of 1000 assignments to avoid performance issues for splits that have massive bulk assignments of huge populations. This is entirely too many records to skim IMO, but I didn't want to take on pagination as part of this minor quality of life improvement.
This is checked via a system spec, but creating those assignments results in at least 5000 inserts and adds about 5 seconds to the test. I debated testing with passing the limit as an optional query parameter, but wasn't sure that would give the necessary confidence that TT wouldn't allow a user to exceed the limit. But maybe that's ok - allowing
?limit=anywhile defaulting to a small number of records would allow for a speedier test and allow for use cases with larger data sets.Note: Rendering the page locally is quite quick (median < 200ms).
Screenshots
/domain @Betterment/test_track_core