Display Reward Metric Section on Stepper and Details Page

[zackcl]

Resolves #2203

[danoswaltCL]

We didn't really talk about the metrics view details component part, we should have talked about the design because there is an awkward thing would happen here... once the backend part is implemented and the reward metric + query are created on submit, we are going to see that query show up as a normal metric query in this view. So we would basically see this information twice, once as a regular metric query, and once as the reward metric query.

BUT.... I think the design is right, we just need to actually update the Experiment model to include the rewardMetricKey as an optional property for mooclet experiments to make this simple. That feels like the right long-term approach anyway, because then this key is generated only once...the backend can then do a couple thing more simply also, and then for the details view you'll have exactly what you need.

We should talk in refinement about this one, I didn't want to mess with the experiment object but it seems like the right thing, I will need to modify my backend PR a little to support this first if others agree (cc @kawcl).

[zackcl]

@danoswaltCL I made 2 commits to include rewardMetricKey in the submitted experiment data (36ffbf0) and to display the Reward Metric table on the details page only when rewardMetricKey exists in the fetched experiment data (4b7f519).

Please feel free to review them.

[danoswaltCL]

I think the only suggestion I'd make is to make this reward object for the display table a template that can be used in both places. It's really the same thing, would just need to modify the key names and data structures a little to match and pretty much use the same table html (technically the whole thing could made it's own component probably). Then you could do something like this:

const getRewardMetricTemplate(rewardMetricKey?: string) {
  return {
          keys: rewardMetricKey || `${experimentName.trim().toUpperCase().replace(/ /g, '_')}_REWARD`,
          operationType: 'Percentage (Success)',
          queryName: 'Success Rate',
        };
} 

Then in the stepper you could call getRewardMetricTemplate() to generate a template with a new key, and here you could use getRewardMetricTemplate(experiment.rewardMetricKey) to create the object with the existing key.

[danoswaltCL]

When I pulled down, I saw no rewardMetricKey being sent on experiment when submitted. The reason was that the property being used here was keys.

// undefined
monitoredMetricsFormData.rewardMetricKey = rewardMetricData[0].keys;

// sends correctly
monitoredMetricsFormData.rewardMetricKey = rewardMetricData[0].metric_Key[0];

That will work, but that points to the need for a type here, because the BehaviorSubject is typed any, so it didn't catch that keys isn't a valid property.

So two things: can you create an interface in experiments.model.ts and use it for typing this value wherever needed. This is what would work currently:

export interface RewardMetricData {
  metric_Key: string[];
  metric_Operation: string[];
  metric_Name: string;
}

Why are we creating the object using arrays for keys instead of a single string though? We know it will always just be a string needed for the reward metric displays.

getRewardMetricData(rewardMetricKey: string): RewardMetricData {
    return {
      metric_Key: [rewardMetricKey], // can this be a string instead of array?
      metric_Operation: ['Percentage (Success)'], // can this be a string instead of array?
      metric_Name: 'Success Rate',
    };
  }

[danoswaltCL]

hey @zackcl , I pushed up the changes I mentioned, had to mess around with the table html a little to make it standardized between the two reward-metric tables to display the data the same way, hope that's okay.

The other thing you'll notice is that I added a check to hide the reward metric query from the standard metric view in both the stepper (when in edit view) and the view-component, otherwise it'll show twice because it will exist in the experiment.queries array. I don't love how I had to code it up, I'm open to suggestions about what to do about this.

done

[danoswaltCL]

hm somehow i hadn't pushed up the changes, thought I had. i was sleepy. my commit is up now.

[zackcl]

@danoswaltCL I've further refined the styles for the Reward Metric table on both the stepper and details page.