Skip to content

Commit

Permalink
feat: display removed context props in the UI (#6844)
Browse files Browse the repository at this point in the history
This PR shows context warnings in the UI if they exist.

That means that if you provide invalid properties on the top level,
you'll get the result back, but we'll also tell you that we didn't take
these properties into account.

Screenie:


![image](https://github.com/Unleash/unleash/assets/17786332/06c31b15-a52f-4a22-a1ac-4f326cc34173)
  • Loading branch information
thomasheartman committed Apr 12, 2024
1 parent 31bf782 commit 6f79688
Show file tree
Hide file tree
Showing 2 changed files with 205 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,148 @@ test('should display error on submit', async () => {

const user = userEvent.setup();
const submitButton = screen.getByText('Submit');
user.click(submitButton);
await user.click(submitButton);

await screen.findByText('some error about too many items');
});

describe('context warnings on successful evaluation', () => {
const warningSummaryText =
'Some context properties were not taken into account during evaluation';

test('should show context warnings if they exist in the response', async () => {
const response = {
features: [],
input: {
environments: [],
projects: [],
context: {},
},
warnings: {
invalidContextProperties: [
'empty array',
'true',
'false',
'number',
'null',
'accountId',
'object',
],
},
};
testServerRoute(
server,
'/api/admin/playground/advanced',
response,
'post',
200,
);

render(testEvaluateComponent);

const user = userEvent.setup();
const submitButton = screen.getByText('Submit');
await user.click(submitButton);

await screen.findByText(warningSummaryText, { exact: false });
for (const prop of response.warnings.invalidContextProperties) {
await screen.findByText(prop);
}
});

test('sorts context warnings alphabetically', async () => {
const response = {
features: [],
input: {
environments: [],
projects: [],
context: {},
},
warnings: {
invalidContextProperties: ['b', 'a', 'z'],
},
};
testServerRoute(
server,
'/api/admin/playground/advanced',
response,
'post',
200,
);

render(testEvaluateComponent);

const user = userEvent.setup();
const submitButton = screen.getByText('Submit');
await user.click(submitButton);

const warnings = screen.getAllByTestId('context-warning-list-element');

expect(warnings[0]).toHaveTextContent('a');
expect(warnings[1]).toHaveTextContent('b');
expect(warnings[2]).toHaveTextContent('z');
});

test('does not render context warnings if the list of properties is empty', async () => {
const response = {
features: [],
input: {
environments: [],
projects: [],
context: {},
},
warnings: {
invalidContextProperties: [],
},
};
testServerRoute(
server,
'/api/admin/playground/advanced',
response,
'post',
200,
);

render(testEvaluateComponent);

const user = userEvent.setup();
const submitButton = screen.getByText('Submit');
await user.click(submitButton);

const warningSummary = screen.queryByText(warningSummaryText, {
exact: false,
});

expect(warningSummary).toBeNull();
});

test("should not show context warnings if they don't exist in the response", async () => {
testServerRoute(
server,
'/api/admin/playground/advanced',
{
features: [],
input: {
environments: [],
projects: [],
context: {},
},
warnings: {},
},
'post',
200,
);

render(testEvaluateComponent);

const user = userEvent.setup();
const submitButton = screen.getByText('Submit');
await user.click(submitButton);

const warningSummary = screen.queryByText(warningSummaryText, {
exact: false,
});

expect(warningSummary).toBeNull();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,59 @@ const StyledAlert = styled(Alert)(({ theme }) => ({
marginBottom: theme.spacing(3),
}));

const GenerateWarningMessages: React.FC<{
response?: AdvancedPlaygroundResponseSchema;
}> = ({ response }) => {
const invalidContextProperties =
response?.warnings?.invalidContextProperties;

if (invalidContextProperties && invalidContextProperties.length > 0) {
invalidContextProperties.sort();
const summary =
'Some context properties were not taken into account during evaluation';

const StyledDetails = styled('details')(({ theme }) => ({
'* + *': { marginBlockStart: theme.spacing(1) },
}));

return (
<StyledAlert severity='warning'>
<StyledDetails>
<summary>{summary}</summary>
<p>
The context you provided for this query contained
top-level properties with invalid values. These
properties were not taken into consideration when
evaluating your query. The properties are:
</p>
<ul>
{invalidContextProperties.map((prop) => (
<li
key={prop}
data-testid='context-warning-list-element'
>
<code>{prop}</code>
</li>
))}
</ul>

<p>
Remember that context fields (with the exception of the{' '}
<code>properties</code> object) must be strings.
</p>
<p>
Because we didn't take these properties into account
during the feature flag evaluation, they will not appear
in the results table.
</p>
</StyledDetails>
</StyledAlert>
);
} else {
return null;
}
};

export const AdvancedPlayground: VFC<{
FormComponent?: typeof PlaygroundForm;
}> = ({ FormComponent = PlaygroundForm }) => {
Expand Down Expand Up @@ -304,11 +357,16 @@ export const AdvancedPlayground: VFC<{
Object.values(errors).length === 0
}
show={
<AdvancedPlaygroundResultsTable
loading={loading}
features={results?.features}
input={results?.input}
/>
<>
<GenerateWarningMessages
response={results}
/>
<AdvancedPlaygroundResultsTable
loading={loading}
features={results?.features}
input={results?.input}
/>
</>
}
/>
<ConditionallyRender
Expand Down

0 comments on commit 6f79688

Please sign in to comment.