Add support for batch assignment overrides via web extension#145
Conversation
|
Needs somebody from @Betterment/test_track_core to claim domain review Use the shovel operator to claim, e.g.:
|
| force: true | ||
| ) | ||
| end | ||
| end |
There was a problem hiding this comment.
i decided that we really only need to test that it hands off to the other class because there's a robust suite of tests on that other class.
| context "with multiple assignments in the payload" do | ||
| let(:split2) { FactoryBot.create(:split, registry: { control: 50, treatment: 50 }) } | ||
|
|
||
| let(:create_params) do | ||
| { | ||
| visitor_id: visitor.id, | ||
| assignments: [ | ||
| { | ||
| split_name: split.name, | ||
| variant: "treatment", | ||
| mixpanel_result: "success", | ||
| context: "context" | ||
| }, | ||
| { | ||
| split_name: split2.name, | ||
| variant: "treatment", | ||
| mixpanel_result: "success", | ||
| context: "context" | ||
| } | ||
| ] | ||
| } | ||
| end | ||
|
|
||
| it "creates an assignment for each" do | ||
| expect { | ||
| post :create, params: create_params | ||
| }.to change { Assignment.count }.by(2) | ||
|
|
||
| expect(response).to have_http_status :no_content | ||
|
|
||
| assignment = Assignment.first | ||
| expect(assignment.variant).to eq "treatment" | ||
| expect(assignment.visitor).to eq visitor | ||
| expect(assignment.split).to eq split | ||
| expect(assignment.mixpanel_result).to eq "success" | ||
| expect(assignment.context).to eq "context" | ||
| expect(assignment.force).to eq true | ||
|
|
||
| assignment = Assignment.second | ||
| expect(assignment.variant).to eq "treatment" | ||
| expect(assignment.visitor).to eq visitor | ||
| expect(assignment.split).to eq split2 | ||
| expect(assignment.mixpanel_result).to eq "success" | ||
| expect(assignment.context).to eq "context" | ||
| expect(assignment.force).to eq true | ||
| end | ||
| end | ||
| end |
There was a problem hiding this comment.
all the tests in this file except for this one are duplicated from the controller tests for the singular assignment override endpoint.
this one shows that the behavior is correct when there are multiple inputs
| # Shared secret-based batch assignment override for chrome extension | ||
| match 'batch_assignment_override', to: '/api/v1/cors#allow', via: :options | ||
| resource :batch_assignment_override, only: :create |
There was a problem hiding this comment.
i'm open to suggestions, but i opted to drop this right into the /v1 namespace alongside the existing endpoint because it's purely additive.
| ActiveRecord::Base.transaction do | ||
| assignments.each do |assignment| | ||
| ArbitraryAssignmentCreation.create! assignment.merge(visitor_id: visitor_id, force: force) | ||
| end | ||
| end |
There was a problem hiding this comment.
i think that this new behavior is as simple as this.
|
<< domain tafn! |
|
@samandmoore needs to incorporate feedback from @aburgel. Bump when done. |
| private | ||
|
|
||
| def create_params | ||
| params.permit(:visitor_id, assignments: %i(split_name variant context)) |
There was a problem hiding this comment.
visitor_id comes from the path, but this works all the same way
right now this is expecting the payload will be:
{
assignments: [...]
}
sounds good?
|
bump! |
|
Needs @aburgel to provide domain review When you finish a round of review, be sure to say you've finished or sign off on the PR, e.g.:
If you're too busy to review, unclaim the PR, e.g.:
|
|
domainlgtm! |
|
Approved! 👯 🔩 🌮 |
3 participants
Summary
It's common for users of the web extension to edit multiple split assignments at once. Up until now, this requires a separate API call for each assignment. This is inefficient, so we're introducing a new endpoint that can process a batch of assignment overrides all at once.
Other Information
To take advantage of this, the web extension will need to be updated, and probably the JS client as well.
In the meantime, the old endpoint will keep on working 👍🏻
/domain @Betterment/test_track_core
/no-platform
NOTE: i'm having issues installing libv8 + therubyracer locally. it might be caused by xcode 12, but i'm not sure. so there may be failing tests right now. i wrote them up but wasn't able to run them locally 😬I've got a workaround, use
mini_racerlocally 😄