Browse Source

fix(arithmetic): Treat equations reordering as different requests (#26576)

- This is cause equations are aliased via their index so if the order of
  the equations change we need to make a new request.
- We still won't make a request if fields change order, or equations
  stay in the same order eg.
  - start with: field-0, field-1, equation[0], equation[1]
    - field-0, equation[0], field-1, equation[1] <- no request
    - field-0, field-1, equation[1], equation[0] <- new request
William Mak 3 years ago
parent
commit
2a54b2030c

+ 1 - 0
static/app/actionCreators/events.tsx

@@ -98,6 +98,7 @@ export const doEventsRequest = (
 
 export type EventQuery = {
   field: string[];
+  equation?: string[];
   team?: string | string[];
   project?: string | string[];
   sort?: string | string[];

+ 7 - 4
static/app/utils/discover/eventView.tsx

@@ -1269,12 +1269,15 @@ export const isAPIPayloadSimilar = (
 
   for (const key of currentKeys) {
     const currentValue = current[key];
-    const currentTarget = Array.isArray(currentValue)
-      ? new Set(currentValue)
-      : currentValue;
+    // Exclude equation from becoming a set for comparison cause its order matters
+    const currentTarget =
+      Array.isArray(currentValue) && key !== 'equation'
+        ? new Set(currentValue)
+        : currentValue;
 
     const otherValue = other[key];
-    const otherTarget = Array.isArray(otherValue) ? new Set(otherValue) : otherValue;
+    const otherTarget =
+      Array.isArray(otherValue) && key !== 'equation' ? new Set(otherValue) : otherValue;
 
     if (!isEqual(currentTarget, otherTarget)) {
       return false;

+ 71 - 0
tests/js/spec/utils/discover/eventView.spec.jsx

@@ -3136,6 +3136,77 @@ describe('isAPIPayloadSimilar', function () {
 
       expect(results).toBe(false);
     });
+
+    it('it is similar if column order changes', function () {
+      const thisEventView = new EventView(state);
+      const location = {};
+      const thisAPIPayload = thisEventView.getEventsAPIPayload(location);
+
+      state.fields.reverse();
+      const otherEventView = new EventView(state);
+      const otherLocation = {};
+      const otherAPIPayload = otherEventView.getEventsAPIPayload(otherLocation);
+
+      const results = isAPIPayloadSimilar(thisAPIPayload, otherAPIPayload);
+
+      expect(results).toBe(true);
+    });
+
+    it('it is similar if equation order relatively same', function () {
+      const equationField = {field: 'equation|failure_count() / count()'};
+      const otherEquationField = {field: 'equation|failure_count() / 2'};
+      state.fields = [
+        {field: 'project.id'},
+        {field: 'count()'},
+        equationField,
+        otherEquationField,
+      ];
+      const thisEventView = new EventView(state);
+      const location = {};
+      const thisAPIPayload = thisEventView.getEventsAPIPayload(location);
+
+      state.fields = [
+        equationField,
+        {field: 'project.id'},
+        {field: 'count()'},
+        otherEquationField,
+      ];
+      const otherEventView = new EventView(state);
+      const otherLocation = {};
+      const otherAPIPayload = otherEventView.getEventsAPIPayload(otherLocation);
+
+      const results = isAPIPayloadSimilar(thisAPIPayload, otherAPIPayload);
+
+      expect(results).toBe(true);
+    });
+
+    it('it is not similar if equation order changes', function () {
+      const equationField = {field: 'equation|failure_count() / count()'};
+      const otherEquationField = {field: 'equation|failure_count() / 2'};
+      state.fields = [
+        {field: 'project.id'},
+        {field: 'count()'},
+        equationField,
+        otherEquationField,
+      ];
+      const thisEventView = new EventView(state);
+      const location = {};
+      const thisAPIPayload = thisEventView.getEventsAPIPayload(location);
+
+      state.fields = [
+        {field: 'project.id'},
+        {field: 'count()'},
+        otherEquationField,
+        equationField,
+      ];
+      const otherEventView = new EventView(state);
+      const otherLocation = {};
+      const otherAPIPayload = otherEventView.getEventsAPIPayload(otherLocation);
+
+      const results = isAPIPayloadSimilar(thisAPIPayload, otherAPIPayload);
+
+      expect(results).toBe(false);
+    });
   });
 
   describe('getFacetsAPIPayload', function () {