Skip to content

Commit

Permalink
Update "removed" definition (#829)
Browse files Browse the repository at this point in the history
We'll say that an item is only removed if it (1) has an "in_source"
annotation and it is false. This annotation should always be present
for metrics, but is not currently set for any other object.
  • Loading branch information
wlach authored Sep 14, 2021
1 parent 1173bdd commit 5a4f1bd
Show file tree
Hide file tree
Showing 7 changed files with 46 additions and 28 deletions.
8 changes: 4 additions & 4 deletions src/components/ItemList.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import Label from "./Label.svelte";
import { filterUncollectedItems } from "../state/filter";
import { isExpired } from "../state/metrics";
import { isExpired, isRemoved } from "../state/items";
import { pageState, updateURLState } from "../state/stores";
let DEFAULT_ITEMS_PER_PAGE = 20;
Expand All @@ -21,7 +21,7 @@
export let showFilter = true;
let filteredItems = items.filter((item) => !isExpired(item.expires));
let filteredItems = items.filter((item) => !isExpired(item));
let pagedItems;
let paginated = true;
let search;
Expand Down Expand Up @@ -189,9 +189,9 @@
/>
{/each}
{/if}
{#if !item.in_source}
{#if isRemoved(item)}
<Label text="removed" />
{:else if isExpired(item.expires)}
{:else if isExpired(item)}
<Label text="expired" />
{/if}
{#if item.deprecated}
Expand Down
6 changes: 3 additions & 3 deletions src/pages/MetricDetail.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
import { pageTitle, pageState, updateURLState } from "../state/stores";
import { getBigQueryURL } from "../state/urls";
import { isExpired } from "../state/metrics";
import { isExpired, isRemoved } from "../state/items";
export let params;
Expand Down Expand Up @@ -78,12 +78,12 @@
</script>

{#await metricDataPromise then metric}
{#if !metric.in_source}
{#if isRemoved(metric)}
<AppAlert
status="warning"
message={"This metric is no longer defined in the source code: new data will not be collected by the application."}
/>
{:else if isExpired(metric.expires)}
{:else if isExpired(metric)}
<AppAlert
status="warning"
message={"This metric has expired: new data will not be collected by the application."}
Expand Down
4 changes: 2 additions & 2 deletions src/state/filter.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { isExpired } from "./metrics";
import { isExpired, isRemoved } from "./items";

export const filterUncollectedItems = (items, showUncollected) =>
items.filter(
(item) => showUncollected || (!isExpired(item.expires) && item.in_source)
(item) => showUncollected || (!isExpired(item) && !isRemoved(item))
);
12 changes: 12 additions & 0 deletions src/state/items.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
export const isRemoved = (item) => {
// only consider an item expired if it has an in_source property
// and it is false
return item.in_source === false;
};

export const isExpired = (item) => {
if (item.expires === "never" || item.expires === undefined) {
return false;
}
return new Date() > new Date(item.expires);
};
6 changes: 0 additions & 6 deletions src/state/metrics.js

This file was deleted.

25 changes: 25 additions & 0 deletions tests/state.items.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { isExpired, isRemoved } from "../src/state/items";

describe("checking if a date has expired", () => {
it("returns True if input is 'never' or undefined", () => {
expect(isExpired({ expires: "never" }) || isExpired({})).toEqual(false);
});
it("returns True if input is a date from the past", () => {
expect(isExpired({ expires: "2021-01-01" })).toEqual(true);
});
it("returns False if input is a date from the future", () => {
expect(isExpired({ expires: "3021-01-01" })).toEqual(false);
});
});

describe("isRemoved works as expected", () => {
it("returns true if in_source is false", () => {
expect(isRemoved({ in_source: false })).toEqual(true);
});
it("returns false if in_source is undefined", () => {
expect(isRemoved({})).toEqual(false);
});
it("returns false if in_source is true", () => {
expect(isRemoved({ in_source: true })).toEqual(false);
});
});
13 changes: 0 additions & 13 deletions tests/state.metrics.test.js

This file was deleted.

0 comments on commit 5a4f1bd

Please sign in to comment.