Update columns used for ecommerce goal pages #23121
Conversation
|
@nathangavin this looks like it could solve the issue, before a proper review can you please check if any tests need adjusting as well? At a glance there might be some. Thanks! |
There was a problem hiding this comment.
This one is a flaky test and unrelated to this PR, so would be better not to update it here.
| @@ -154,11 +154,43 @@ private function setPropertiesForEcommerceView() | |||
| $this->requestConfig->filter_sort_column = 'goal_ecommerceOrder_revenue'; | |||
There was a problem hiding this comment.
I guess we need to set that based on the displaytype. 'goal_ecommerceOrder_revenue' is only shown for normal. Otherwise the order might look random.
| 'goal_ecommerceOrder_revenue_per_entry', | ||
| ]; | ||
| break; | ||
| default: |
There was a problem hiding this comment.
I'd move the case self::GOALS_DISPLAY_NORMAL: down here, so this is also the default. It shouldn't happen that another display type appears. But might be better to have a defined default to use.
There was a problem hiding this comment.
That test started failing outside of this PR, so better not to update it here. We may need to investigate why it starting failing in a separate issue/PR.
Description:
Current Ecommerce goal pages are not showing the correct data. This is addressed by adding logic to select different columns depending on the display type for the underlying report.
Addresses issue #20857
Review