Skip to content

Commit

Permalink
feat: add config properties to customize reviewer and review date dis…
Browse files Browse the repository at this point in the history
…play behavior (#250)

* feat: add config properties to customize reviewer and review date display behavior

* feat: define display policy types

* feat: create class and hook to access display configuration

* feat: add support for display options to score card table

* feat: respect display options when building reviewer link

* chore: update dev app-config with new options for better discoverability

* chore: generate changeset
  • Loading branch information
sbstnc committed Apr 12, 2024
1 parent b4fae31 commit 500e616
Show file tree
Hide file tree
Showing 10 changed files with 328 additions and 15 deletions.
5 changes: 5 additions & 0 deletions .changeset/silent-jars-complain.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@oriflame/backstage-plugin-score-card': minor
---

Added config options to customize reviewer and review date display behavior
12 changes: 12 additions & 0 deletions plugins/score-card/config.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,18 @@ export interface Config {
* @visibility frontend
*/
jsonDataUrl?: string;
display?: {
/**
* Whether to display the reviewer column in the score card table.
* @visibility frontend
*/
reviewer?: string;
/**
* Whether to display the review date column in the score card table.
* @visibility frontend
*/
reviewDate?: string;
};
/**
* The template for the link to the wiki, e.g. "https://TBD/XXX/_wiki/wikis/XXX.wiki/{id}"
* @visibility frontend
Expand Down
3 changes: 3 additions & 0 deletions plugins/score-card/dev/app-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,6 @@ backend:

scorecards:
jsonDataUrl: http://127.0.0.1:8090/sample-data/ #this needs to be served via http-server as we get http 200 with WDS server instead of http 404 for nonExistent entity
display:
reviewer: always
reviewDate: always
5 changes: 4 additions & 1 deletion plugins/score-card/src/components/ScoreCard/ScoreCard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import { scorePercentColumn } from './columns/scorePercentColumn';
import { titleColumn } from './columns/titleColumn';
import { getReviewerLink } from './sub-components/getReviewerLink';
import { scoringDataApiRef } from '../../api';
import { useDisplayConfig } from '../../config/DisplayConfig';

// lets prepare some styles
const useStyles = makeStyles(theme => ({
Expand Down Expand Up @@ -122,6 +123,8 @@ export const ScoreCard = ({

const allEntries = getScoreTableEntries(data);

const displayPolicies = useDisplayConfig().getDisplayPolicies();

return (
<InfoCard
title="Scoring"
Expand Down Expand Up @@ -174,7 +177,7 @@ export const ScoreCard = ({
}}
/>

{getReviewerLink(data)}
{getReviewerLink(data, displayPolicies)}
</Grid>
</div>
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,35 @@
import { EntityRefLink } from '@backstage/plugin-catalog-react';
import React from 'react';
import { EntityScoreExtended } from '../../../api/types';
import { DisplayPolicies, DisplayPolicy } from '../../../config/types';

export function getReviewerLink(
value: EntityScoreExtended,
displayPolicies: DisplayPolicies,
) {
const displayReviewer = displayPolicies.reviewer !== DisplayPolicy.Never;
const displayReviewDate = displayPolicies.reviewDate !== DisplayPolicy.Never;

if (!displayReviewer && !displayReviewDate) {
return null;
}

export function getReviewerLink(value: EntityScoreExtended) {
return (
<div style={{ textAlign: 'right', margin: '0.2rem' }}>
{value.reviewer ? (
<>
Review done by&nbsp;
<EntityRefLink entityRef={value.reviewer}>
{value.reviewer?.name}
</EntityRefLink>
&nbsp;at&nbsp;
{value.reviewDate ? value.reviewDate.toLocaleDateString() : 'unknown'}
Review done
{displayReviewer && ' by '}
{displayReviewer && (
<EntityRefLink entityRef={value.reviewer}>
{value.reviewer?.name}
</EntityRefLink>
)}
{displayReviewDate &&
` at
${(value.reviewDate
? value.reviewDate.toLocaleDateString()
: 'unknown')}`}
</>
) : (
<>Not yet reviewed.</>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@
import React from 'react';
import { act, render } from '@testing-library/react';
import { ScoreCardTable } from './ScoreCardTable';
import { TestApiProvider } from '@backstage/test-utils';
import { MockConfigApi, TestApiProvider } from '@backstage/test-utils';
import { ScoringDataApi, scoringDataApiRef } from '../../api';
import { Entity } from '@backstage/catalog-model';
import { EntityScoreExtended } from '../../api/types';
import { errorApiRef } from '@backstage/core-plugin-api';
import { configApiRef, errorApiRef } from '@backstage/core-plugin-api';
import { lightTheme } from '@backstage/theme';
import { ThemeProvider } from '@material-ui/core';
import { MemoryRouter as Router } from 'react-router-dom';
Expand Down Expand Up @@ -53,6 +53,7 @@ describe('ScoreBoardPage-EmptyData', () => {
apis={[
[errorApiRef, errorApi],
[scoringDataApiRef, mockClient],
[configApiRef, new MockConfigApi({})],
]}
>
<ScoreCardTable />
Expand All @@ -68,6 +69,68 @@ describe('ScoreBoardPage-EmptyData', () => {
await findByTestId('score-board-table');
jest.useRealTimers();
});

it.each([
['always', 1],
['if-data-present', 0],
['never', 0],
])('should apply reviewer column display policy', async (displayPolicy, isPresent) => {
const errorApi = { post: () => {} };
const displayPolicies = { reviewer: displayPolicy };
const configApi = new MockConfigApi({ scorecards: {display: displayPolicies} });
const { findByTestId, queryAllByText } = render(
<ThemeProvider theme={lightTheme}>
<TestApiProvider
apis={[
[errorApiRef, errorApi],
[scoringDataApiRef, mockClient],
[configApiRef, configApi],
]}
>
<Router>
<ScoreCardTable />
</Router>
</TestApiProvider>
</ThemeProvider>,
);

await findByTestId('score-board-table');

const reviewerColumn = await queryAllByText('Reviewer')

expect(reviewerColumn).toHaveLength(isPresent)
});

it.each([
['always', 1],
['if-data-present', 0],
['never', 0],
])('should apply review date column display policy', async (displayPolicy, isPresent) => {
const errorApi = { post: () => {} };
const displayPolicies = { reviewDate: displayPolicy };
const configApi = new MockConfigApi({ scorecards: {display: displayPolicies} });
const { findByTestId, queryAllByText } = render(
<ThemeProvider theme={lightTheme}>
<TestApiProvider
apis={[
[errorApiRef, errorApi],
[scoringDataApiRef, mockClient],
[configApiRef, configApi],
]}
>
<Router>
<ScoreCardTable />
</Router>
</TestApiProvider>
</ThemeProvider>,
);

await findByTestId('score-board-table');

const reviewerColumn = await queryAllByText('Date')

expect(reviewerColumn).toHaveLength(isPresent)
});
});

describe('ScoreCard-TestWithData', () => {
Expand Down Expand Up @@ -119,6 +182,7 @@ describe('ScoreCard-TestWithData', () => {
apis={[
[errorApiRef, errorApi],
[scoringDataApiRef, mockClient],
[configApiRef, new MockConfigApi({})],
]}
>
<Router>
Expand All @@ -135,4 +199,66 @@ describe('ScoreCard-TestWithData', () => {

expect(podcastRow).toHaveTextContent('podcastsystemAB+DFFC');
});

it.each([
['always', 1],
['if-data-present', 1],
['never', 0],
])('should apply reviewer column display policy', async (displayPolicy, isPresent) => {
const errorApi = { post: () => {} };
const displayPolicies = { reviewer: displayPolicy };
const configApi = new MockConfigApi({ scorecards: {display: displayPolicies} });
const { findByTestId, queryAllByText } = render(
<ThemeProvider theme={lightTheme}>
<TestApiProvider
apis={[
[errorApiRef, errorApi],
[scoringDataApiRef, mockClient],
[configApiRef, configApi],
]}
>
<Router>
<ScoreCardTable />
</Router>
</TestApiProvider>
</ThemeProvider>,
);

await findByTestId('score-board-table');

const reviewerColumn = await queryAllByText('Reviewer')

expect(reviewerColumn).toHaveLength(isPresent)
});

it.each([
['always', 1],
['if-data-present', 1],
['never', 0],
])('should apply review date column display policy', async (displayPolicy, isPresent) => {
const errorApi = { post: () => {} };
const displayPolicies = { reviewDate: displayPolicy };
const configApi = new MockConfigApi({ scorecards: {display: displayPolicies} });
const { findByTestId, queryAllByText } = render(
<ThemeProvider theme={lightTheme}>
<TestApiProvider
apis={[
[errorApiRef, errorApi],
[scoringDataApiRef, mockClient],
[configApiRef, configApi],
]}
>
<Router>
<ScoreCardTable />
</Router>
</TestApiProvider>
</ThemeProvider>,
);

await findByTestId('score-board-table');

const reviewerColumn = await queryAllByText('Date')

expect(reviewerColumn).toHaveLength(isPresent)
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import { scoringDataApiRef } from '../../api';
import { EntityScoreExtended } from '../../api/types';
import { EntityRefLink } from '@backstage/plugin-catalog-react';
import { DEFAULT_NAMESPACE } from '@backstage/catalog-model';
import { useDisplayConfig } from '../../config/DisplayConfig';
import { DisplayPolicy } from '../../config/types';

const useScoringAllDataLoader = (entityKindFilter?: string[]) => {
const errorApi = useApi(errorApiRef);
Expand All @@ -49,6 +51,8 @@ type ScoreTableProps = {
};

export const ScoreTable = ({ title, scores }: ScoreTableProps) => {
const displayPolicies = useDisplayConfig().getDisplayPolicies();

const columns: TableColumn<EntityScoreExtended>[] = [
{
title: 'Name',
Expand Down Expand Up @@ -85,7 +89,14 @@ export const ScoreTable = ({ title, scores }: ScoreTableProps) => {
</>
) : null,
},
{
];

if (
displayPolicies.reviewer === DisplayPolicy.Always ||
(displayPolicies.reviewer === DisplayPolicy.IfDataPresent &&
scores.some(s => !!s.scoringReviewer))
) {
columns.push({
title: 'Reviewer',
field: 'scoringReviewer',
render: entityScore =>
Expand All @@ -96,16 +107,24 @@ export const ScoreTable = ({ title, scores }: ScoreTableProps) => {
</EntityRefLink>
</>
) : null,
},
{
});
}

if (
displayPolicies.reviewDate === DisplayPolicy.Always ||
(displayPolicies.reviewDate === DisplayPolicy.IfDataPresent &&
scores.some(s => !!s.scoringReviewDate))
) {
columns.push({
title: 'Date',
field: 'scoringReviewDate',
render: entityScore =>
entityScore.reviewDate ? (
<>{entityScore.reviewDate.toLocaleDateString()}</>
) : null,
},
];
});
}

scores
.flatMap(s => {
return s.areaScores ?? [];
Expand Down
58 changes: 58 additions & 0 deletions plugins/score-card/src/config/DisplayConfig.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/*
* Copyright 2022 Oriflame
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import { MockConfigApi } from '@backstage/test-utils';
import { DisplayConfig } from './DisplayConfig';
import { DisplayPolicy } from './types';

describe('display config', () => {
it.each([
[
{ reviewer: 'always', reviewDate: 'always' },
{ reviewer: DisplayPolicy.Always, reviewDate: DisplayPolicy.Always },
],
[
{ reviewer: 'never', reviewDate: 'never' },
{ reviewer: DisplayPolicy.Never, reviewDate: DisplayPolicy.Never },
],
[
{ reviewer: 'if-data-present', reviewDate: 'if-data-present' },
{
reviewer: DisplayPolicy.IfDataPresent,
reviewDate: DisplayPolicy.IfDataPresent,
},
],
[
{ reviewDate: 'if-data-present' },
{
reviewer: DisplayPolicy.Always,
reviewDate: DisplayPolicy.IfDataPresent,
},
],
[
{ reviewer: 'never' },
{ reviewer: DisplayPolicy.Never, reviewDate: DisplayPolicy.Always },
],
[{}, { reviewer: DisplayPolicy.Always, reviewDate: DisplayPolicy.Always }],
])('gets expected display policies from config', (policies, expected) => {
const mockConfig = new MockConfigApi({ scorecards: { display: policies } });

const config = new DisplayConfig({ configApi: mockConfig });

const displayPolicies = config.getDisplayPolicies();
expect(displayPolicies).toEqual(expected);
});
});
Loading

0 comments on commit 500e616

Please sign in to comment.