Skip to content

Commit

Permalink
fix duplicate active values in leaderboard
Browse files Browse the repository at this point in the history
  • Loading branch information
bcolloran committed Oct 19, 2023
1 parent 04896b0 commit ba5a661
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 16 deletions.
19 changes: 13 additions & 6 deletions web-common/src/features/dashboards/leaderboard/Leaderboard.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -77,11 +77,18 @@
dimensionName
);
let activeValues: Array<unknown>;
$: activeValues =
$dashboardStore?.filters[filterKey]?.find((d) => d.name === dimension?.name)
?.in ?? [];
$: atLeastOneActive = !!activeValues?.length;
// FIXME: it is possible for this way of accessing the filters
// to return the same value twice, which would seem to indicate
// a bug in the way we're setting the filters / active values.
// Need to investigate further to determine whether this is a
// problem with the runtime or the client, but for now wrapping
// it in a set dedupes the values.
$: activeValues = new Set(
($dashboardStore?.filters[filterKey]?.find(
(d) => d.name === dimension?.name
)?.in as (number | string)[]) ?? []
);
$: atLeastOneActive = activeValues?.size > 0;
const timeControlsStore = useTimeControlStore(stateManagers);
Expand Down Expand Up @@ -141,7 +148,7 @@
getLabeledComparisonFromComparisonRow(r, measure.name)
) ?? [],
slice,
activeValues,
[...activeValues],
unfilteredTotal,
filterExcludeMode
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,25 +138,20 @@ export function prepareLeaderboardItemData(
// selected values that _are_ in the API results.
//
// We also need to retain the original selection indices
let selectedButNotInAPIResults: [string | number, number][] =
selectedValues.map((v, i) => [v, i]);
const selectedButNotInAPIResults = new Map<string | number, number>();
selectedValues.map((v, i) => selectedButNotInAPIResults.set(v, i));

values.forEach((v, i) => {
const selectedIndex = selectedValues.findIndex(
(value) => value === v.dimensionValue
);
// if we have found this selected value in the API results,
// remove it from the selectedButNotInAPIResults array
if (selectedIndex > -1)
selectedButNotInAPIResults = selectedButNotInAPIResults.filter(
(value) => value[0] !== v.dimensionValue
);
if (selectedIndex > -1) selectedButNotInAPIResults.delete(v.dimensionValue);

const defaultComparedIndex = comparisonDefaultSelection.findIndex(
(value) => value === v.dimensionValue
);
// if we have found this selected value in the API results,
// remove it from the selectedButNotInAPIResults array

const cleanValue = cleanUpComparisonValue(
v,
Expand All @@ -182,7 +177,7 @@ export function prepareLeaderboardItemData(
// that pushes it out of the top N. In that case, we will follow
// the previous strategy, and just push a dummy value with only
// the dimension value and nulls for all measure values.
selectedButNotInAPIResults.forEach(([dimensionValue, selectedIndex]) => {
for (const [dimensionValue, selectedIndex] of selectedButNotInAPIResults) {
const defaultComparedIndex = comparisonDefaultSelection.findIndex(
(value) => value === dimensionValue
);
Expand All @@ -197,7 +192,7 @@ export function prepareLeaderboardItemData(
deltaRel: null,
deltaAbs: null,
});
});
}

const noAvailableValues = values.length === 0;
const showExpandTable = values.length > numberAboveTheFold;
Expand Down

0 comments on commit ba5a661

Please sign in to comment.