Skip to content

Commit

Permalink
Fix empty subject filtering.
Browse files Browse the repository at this point in the history
  • Loading branch information
fniessink committed Dec 8, 2023
1 parent 2c40206 commit 251971f
Show file tree
Hide file tree
Showing 12 changed files with 150 additions and 59 deletions.
18 changes: 12 additions & 6 deletions components/frontend/src/metric/MetricDetails.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,13 @@ import { MetricTypeHeader } from './MetricTypeHeader';
import { TrendGraph } from './TrendGraph';
import { datePropType, reportPropType, reportsPropType, stringsPropType, stringsURLSearchQueryPropType} from '../sharedPropTypes';

function Buttons({ isFirstMetric, isLastMetric, metric_uuid, reload, stopSorting }) {
function Buttons({ isFirstMetric, isLastMetric, metric_uuid, reload, stopFilteringAndSorting }) {
return (
<ReadOnlyOrEditable requiredPermissions={[EDIT_REPORT_PERMISSION]} editableComponent={
<div style={{ marginTop: "20px" }}>
<ReorderButtonGroup
first={isFirstMetric} last={isLastMetric} moveable="metric" slot="row"
onClick={(direction) => { stopSorting(); set_metric_attribute(metric_uuid, "position", direction, reload) }} />
onClick={(direction) => { stopFilteringAndSorting(); set_metric_attribute(metric_uuid, "position", direction, reload) }} />
<DeleteButton item_type="metric" onClick={() => delete_metric(metric_uuid, reload)} />
</div>}
/>
Expand All @@ -36,7 +36,7 @@ Buttons.propTypes = {
isLastMetric: PropTypes.bool,
metric_uuid: PropTypes.string,
reload: PropTypes.func,
stopSorting: PropTypes.func
stopFilteringAndSorting: PropTypes.func
}

function fetchMeasurements(reportDate, metric_uuid, setMeasurements) {
Expand All @@ -63,7 +63,7 @@ export function MetricDetails({
report_date,
reports,
report,
stopSorting,
stopFilteringAndSorting,
subject_uuid,
visibleDetailsTabs,
}) {
Expand Down Expand Up @@ -159,7 +159,13 @@ export function MetricDetails({
<>
<MetricTypeHeader metricType={metricType} />
<Tab panes={panes} defaultActiveIndex={defaultActiveTab} onTabChange={onTabChange} />
<Buttons metric_uuid={metric_uuid} isFirstMetric={isFirstMetric} isLastMetric={isLastMetric} stopSorting={stopSorting} reload={reload} />
<Buttons
metric_uuid={metric_uuid}
isFirstMetric={isFirstMetric}
isLastMetric={isLastMetric}
reload={reload}
stopFilteringAndSorting={stopFilteringAndSorting}
/>
</>
);
}
Expand All @@ -172,7 +178,7 @@ MetricDetails.propTypes = {
report_date: datePropType,
reports: reportsPropType,
report: reportPropType,
stopSorting: PropTypes.func,
stopFilteringAndSorting: PropTypes.func,
subject_uuid: PropTypes.string,
visibleDetailsTabs: stringsURLSearchQueryPropType
}
4 changes: 2 additions & 2 deletions components/frontend/src/metric/MetricDetails.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ const data_model = {
subjects: { subject_type: { metrics: ["violations"] } }
}

async function renderMetricDetails(stopSorting, connection_error) {
async function renderMetricDetails(stopFilteringAndSorting, connection_error) {
measurement_api.get_metric_measurements.mockImplementation(() => Promise.resolve({
ok: true,
measurements: [
Expand All @@ -69,7 +69,7 @@ async function renderMetricDetails(stopSorting, connection_error) {
metric_uuid="metric_uuid"
report={report}
reports={[report]}
stopSorting={stopSorting}
stopFilteringAndSorting={stopFilteringAndSorting}
subject_uuid="subject_uuid"
visibleDetailsTabs={settings.visibleDetailsTabs}
/>
Expand Down
2 changes: 1 addition & 1 deletion components/frontend/src/report/Report.js
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ export function Report({
report_date={report_date}
settings={settings}
/>
<SubjectsButtonRow reload={reload} report={report} reports={reports} />
<SubjectsButtonRow reload={reload} report={report} reports={reports} settings={settings} />
</div>
)
}
Expand Down
6 changes: 6 additions & 0 deletions components/frontend/src/sharedPropTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,10 @@ export const sourcePropType = PropTypes.shape({
source_uuid: PropTypes.string,
})

export const subjectPropType = PropTypes.shape({
type: PropTypes.string
})

export const metricPropType = PropTypes.shape({
accept_debt: PropTypes.bool,
debt_end_date: PropTypes.string,
Expand All @@ -115,6 +119,8 @@ export const metricPropType = PropTypes.shape({
issue_status: PropTypes.arrayOf(issueStatusPropType)
})

export const metricsPropType = PropTypes.arrayOf(metricPropType)

export const reportPropType = PropTypes.shape({
desired_response_times: PropTypes.object,
issue_tracker: PropTypes.object,
Expand Down
33 changes: 30 additions & 3 deletions components/frontend/src/subject/Subject.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { DataModel } from '../context/DataModel';
import {
datesPropType,
optionalDatePropType,
metricsPropType,
reportPropType,
reportsPropType,
settingsPropType,
Expand Down Expand Up @@ -39,12 +40,12 @@ function sortMetrics(datamodel, metrics, sortDirection, sortColumn, report, meas
measurement: (m1, m2) => {
const m1_measurement = get_metric_value(m1[1]);
const m2_measurement = get_metric_value(m2[1]);
return m1_measurement.localeCompare(m2_measurement, undefined, {numeric: true})
return m1_measurement.localeCompare(m2_measurement, undefined, { numeric: true })
},
target: (m1, m2) => {
const m1_target = get_metric_target(m1[1]);
const m2_target = get_metric_target(m2[1]);
return m1_target.localeCompare(m2_target, undefined, {numeric: true})
return m1_target.localeCompare(m2_target, undefined, { numeric: true })
},
comment: (m1, m2) => {
const m1_comment = get_metric_comment(m1[1]);
Expand Down Expand Up @@ -95,6 +96,30 @@ function sortMetrics(datamodel, metrics, sortDirection, sortColumn, report, meas
}
}

function subjectIsEmptyDueToFilters(atReportsOverview, filteredMetrics, metrics, settings) {
// Hide the subject if it has no metrics after filtering and at least one of the following conditions holds:
// - the user is at the reports overview page
// - the user is hiding all metrics or metrics that don't require action
// - the user is hiding metrics by tag
// - the subject has metrics before filtering
// This ensures (in combination with resetting the metricsToHide and hiddenTags settings after adding a subject)
// that newly added subjects are not immediately hidden
return (
Object.keys(filteredMetrics).length === 0 && (
atReportsOverview ||
!settings.metricsToHide.isDefault() ||
!settings.hiddenTags.isDefault() ||
Object.keys(metrics).length !== 0
)
)
}
subjectIsEmptyDueToFilters.propTypes = {
atReportsOverview: PropTypes.bool,
filteredMetrics: metricsPropType,
metrics: metricsPropType,
settings: settingsPropType,
}

export function Subject({
atReportsOverview,
changed_fields,
Expand All @@ -113,7 +138,9 @@ export function Subject({
const subject = report.subjects[subject_uuid];
const metrics = visibleMetrics(subject.metrics, settings.metricsToHide.value, settings.hiddenTags.value)
const dataModel = useContext(DataModel)
if ((!settings.metricsToHide.isDefault() || !settings.hiddenTags.isDefault()) && Object.keys(metrics).length === 0) { return null }
if (subjectIsEmptyDueToFilters(atReportsOverview, metrics, subject.metrics, settings)) {
return null
}
let metricEntries = Object.entries(metrics);
if (settings.sortColumn.value !== "") {
sortMetrics(dataModel, metricEntries, settings.sortDirection.value, settings.sortColumn.value, report, measurements);
Expand Down
13 changes: 11 additions & 2 deletions components/frontend/src/subject/SubjectTable.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,11 @@ export function SubjectTable({
report_date={reportDate}
reports={reports}
report={report}
stopSorting={() => handleSort(null)}
stopFilteringAndSorting={() => {
handleSort(null)
settings.hiddenTags.reset()
settings.metricsToHide.reset()
}}
subject_uuid={subject_uuid}
visibleDetailsTabs={settings.visibleDetailsTabs}
/>
Expand Down Expand Up @@ -137,7 +141,12 @@ export function SubjectTable({
subject={subject}
reload={reload}
reports={reports}
stopSorting={() => handleSort(null)} />
stopFilteringAndSorting={() => {
handleSort(null)
settings.hiddenTags.reset()
settings.metricsToHide.reset()
}}
/>
</Table>
)
}
Expand Down
82 changes: 55 additions & 27 deletions components/frontend/src/subject/SubjectTable.test.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,22 @@
import React from 'react';
import { fireEvent, render, renderHook, screen } from '@testing-library/react';
import { act, fireEvent, render, renderHook, screen } from '@testing-library/react';
import history from 'history/browser';
import { DataModel } from '../context/DataModel';
import { EDIT_REPORT_PERMISSION, Permissions } from "../context/Permissions";
import { useVisibleDetailsTabsSearchQuery } from '../app_ui_settings';
import * as fetch_server_api from '../api/fetch_server_api';
import { createTestableSettings } from '../__fixtures__/fixtures';
import { SubjectTable } from './SubjectTable';

jest.mock("../api/fetch_server_api.js")
fetch_server_api.fetch_server_api = jest.fn().mockResolvedValue({ ok: false });

const metric = {
unit: "testUnit",
scale: "count",
type: "metric_type",
name: "name_1",
sources: {},
tags: ["Tag 1"]
}
const metric2 = {
Expand All @@ -21,7 +27,10 @@ const metric2 = {
}
const datamodel = {
metrics: {
metric_type: { name: "Metric type", tags: [] }
metric_type: { name: "Metric type", sources: ["source_type"], tags: [] }
},
sources: {
source_type: { name: "Source type"}
},
subjects: {
subject_type: { metrics: ["metric_type"] }
Expand All @@ -41,28 +50,31 @@ function renderSubjectTable(
} = {}
) {
let settings = createTestableSettings()
if (visibleDetailsTabs) { settings.visibleDetailsTabs = visibleDetailsTabs}
if (visibleDetailsTabs) { settings.visibleDetailsTabs = visibleDetailsTabs }
render(
<DataModel.Provider value={datamodel}>
<SubjectTable
dates={dates}
reportDate={new Date("2020-01-15T00:00:00+00:00")}
report={{ report_uuid: "report_uuid", subjects: { subject_uuid: { type: "subject_type", metrics: { 1: metric, 2: metric2 } } } }}
measurements={
[
{ metric_uuid: "1", start: "2020-01-14T00:00:00+00:00", end: "2020-01-15T00:00:00+00:00", count: { status: "target_met" } },
{ metric_uuid: "1", start: "2020-01-15T00:00:00+00:00", end: "2020-01-16T00:00:00+00:00", count: { status: "target_met" } },
{ metric_uuid: "1", start: "2020-01-16T00:00:00+00:00", end: "2020-01-17T00:00:00+00:00", count: { status: "target_not_met" } },
{ metric_uuid: "2", start: "2020-01-10T00:00:00+00:00", end: "2020-01-10T00:00:00+00:00", count: { status: "target_not_met" } },
{ metric_uuid: "3", start: "2020-01-14T00:00:00+00:00", end: "2020-01-15T00:00:00+00:00", count: { status: "target_not_met" } },
]
}
metricEntries={Object.entries({ 1: metric, 2: metric2 })}
settings={settings}
subject={{ type: "subject_type", metrics: { 1: metric, 2: metric2 } }}
subject_uuid="subject_uuid"
/>
</DataModel.Provider>
<Permissions.Provider value={[EDIT_REPORT_PERMISSION]}>
<DataModel.Provider value={datamodel}>
<SubjectTable
dates={dates}
handleSort={jest.fn()}
reportDate={new Date("2020-01-15T00:00:00+00:00")}
report={{ report_uuid: "report_uuid", subjects: { subject_uuid: { type: "subject_type", metrics: { 1: metric, 2: metric2 } } } }}
measurements={
[
{ metric_uuid: "1", start: "2020-01-14T00:00:00+00:00", end: "2020-01-15T00:00:00+00:00", count: { status: "target_met" } },
{ metric_uuid: "1", start: "2020-01-15T00:00:00+00:00", end: "2020-01-16T00:00:00+00:00", count: { status: "target_met" } },
{ metric_uuid: "1", start: "2020-01-16T00:00:00+00:00", end: "2020-01-17T00:00:00+00:00", count: { status: "target_not_met" } },
{ metric_uuid: "2", start: "2020-01-10T00:00:00+00:00", end: "2020-01-10T00:00:00+00:00", count: { status: "target_not_met" } },
{ metric_uuid: "3", start: "2020-01-14T00:00:00+00:00", end: "2020-01-15T00:00:00+00:00", count: { status: "target_not_met" } },
]
}
metricEntries={Object.entries({ 1: metric, 2: metric2 })}
settings={settings}
subject={{ type: "subject_type", metrics: { 1: metric, 2: metric2 } }}
subject_uuid="subject_uuid"
/>
</DataModel.Provider>
</Permissions.Provider>
);
}

Expand All @@ -79,7 +91,7 @@ it('displays all the metrics', () => {
});

it('shows the date columns', () => {
renderSubjectTable({dates: dates})
renderSubjectTable({ dates: dates })
dates.forEach((date) => {
expect(screen.queryAllByText(date.toLocaleDateString()).length).toBe(1)
})
Expand Down Expand Up @@ -113,7 +125,7 @@ it('does not show the overrun column when showing one date', () => {
})

it('shows the overrun column when showing multiple dates', () => {
renderSubjectTable({ dates: dates} )
renderSubjectTable({ dates: dates })
expect(screen.queryAllByText(/[Oo]verrun/).length).toBe(1)
})

Expand Down Expand Up @@ -160,7 +172,7 @@ it('hides the tags column', () => {

it('expands the details via the button', () => {
const visibleDetailsTabs = renderHook(() => useVisibleDetailsTabsSearchQuery())
renderSubjectTable({ visibleDetailsTabs: visibleDetailsTabs.result.current})
renderSubjectTable({ visibleDetailsTabs: visibleDetailsTabs.result.current })
const expand = screen.getAllByRole("button")[0];
fireEvent.click(expand);
visibleDetailsTabs.rerender()
Expand All @@ -170,7 +182,7 @@ it('expands the details via the button', () => {
it('collapses the details via the button', () => {
history.push("?tabs=1:0")
const visibleDetailsTabs = renderHook(() => useVisibleDetailsTabsSearchQuery())
renderSubjectTable({visibleDetailsTabs: visibleDetailsTabs.result.current})
renderSubjectTable({ visibleDetailsTabs: visibleDetailsTabs.result.current })
const expand = screen.getAllByRole("button")[0];
fireEvent.click(expand);
visibleDetailsTabs.rerender()
Expand All @@ -184,3 +196,19 @@ it('expands the details via the url', () => {
renderSubjectTable()
expect(screen.queryAllByText("Configuration").length).toBe(1)
})

it('moves a metric', async () => {
history.push("?tabs=1:2")
renderSubjectTable()
await act(async () => fireEvent.click(await screen.findByLabelText("Move metric to the next row")))
expect(fetch_server_api.fetch_server_api).toHaveBeenCalledWith("post", "metric/1/attribute/position", { position: "next" });
})

it('adds a source', async () => {
history.push("?tabs=1:2")
renderSubjectTable()
const addButton = await screen.findByText("Add source")
await act(async () => fireEvent.click(addButton))
fireEvent.click(await screen.findByText("Source type"))
expect(fetch_server_api.fetch_server_api).toHaveBeenCalledWith("post", "source/new/1", { type: "source_type" });
})
17 changes: 13 additions & 4 deletions components/frontend/src/subject/SubjectTableFooter.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
import { useContext } from "react";
import PropTypes from 'prop-types';
import { Table } from "semantic-ui-react";
import { add_metric, copy_metric, move_metric } from "../api/metric";
import { DataModel } from "../context/DataModel";
import { EDIT_REPORT_PERMISSION, ReadOnlyOrEditable } from "../context/Permissions";
import { AddDropdownButton, CopyButton, MoveButton } from "../widgets/Button";
import { metric_options } from "../widgets/menu_options";
import { metricTypeOptions } from "../metric/MetricType";
import { reportsPropType, subjectPropType } from '../sharedPropTypes';

export function SubjectTableFooter({ subject, subjectUuid, reload, reports, stopSorting }) {
export function SubjectTableFooter({ subject, subjectUuid, reload, reports, stopFilteringAndSorting }) {
const dataModel = useContext(DataModel)
return (
<ReadOnlyOrEditable requiredPermissions={[EDIT_REPORT_PERMISSION]} editableComponent={
Expand All @@ -18,22 +20,22 @@ export function SubjectTableFooter({ subject, subjectUuid, reload, reports, stop
item_type="metric"
item_subtypes={metricTypeOptions(dataModel, subject.type)}
onClick={(subtype) => {
stopSorting()
stopFilteringAndSorting()
add_metric(subjectUuid, subtype, reload);
}}
/>
<CopyButton
item_type="metric"
onChange={(source_metric_uuid) => {
stopSorting()
stopFilteringAndSorting()
copy_metric(source_metric_uuid, subjectUuid, reload);
}}
get_options={() => metric_options(reports, dataModel, subject.type)}
/>
<MoveButton
item_type="metric"
onChange={(source_metric_uuid) => {
stopSorting()
stopFilteringAndSorting()
move_metric(source_metric_uuid, subjectUuid, reload);
}}
get_options={() => metric_options(reports, dataModel, subject.type, subjectUuid)}
Expand All @@ -44,3 +46,10 @@ export function SubjectTableFooter({ subject, subjectUuid, reload, reports, stop
/>
)
}
SubjectTableFooter.propTypes = {
subject: subjectPropType,
subjectUuid: PropTypes.string,
reload: PropTypes.func,
reports: reportsPropType,
stopFilteringAndSorting: PropTypes.func
}
Loading

0 comments on commit 251971f

Please sign in to comment.