Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change permissions used for finishing plugin setup #2242

Merged
merged 11 commits into from
Jun 26, 2023
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## Unreleased

### Changed

- Change permissions used during setup to better represent actions being taken by @mderynck ([#2242](https://github.com/grafana/oncall/pull/2242))

## v1.3.1 (2023-06-26)

### Fixed
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ describe('PluginConfigPage', () => {
const metaJsonDataOnCallApiUrl = 'onCallApiUrlFromMetaJsonData';

process.env.ONCALL_API_URL = processEnvOnCallApiUrl;

window.location.reload = jest.fn();
PluginState.checkIfPluginIsConnected = jest.fn().mockResolvedValueOnce(null);
mockSyncDataWithOnCall(License.OSS);

Expand All @@ -302,8 +302,6 @@ describe('PluginConfigPage', () => {
// click the confirm button within the modal, which actually triggers the callback
await userEvent.click(screen.getByText('Remove'));

await screen.findByTestId(successful ? PLUGIN_CONFIGURATION_FORM_DATA_ID : STATUS_MESSAGE_BLOCK_DATA_ID);

// assertions
expect(PluginState.checkIfPluginIsConnected).toHaveBeenCalledTimes(1);
expect(PluginState.checkIfPluginIsConnected).toHaveBeenCalledWith(metaJsonDataOnCallApiUrl);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
import React, { FC, useCallback, useEffect, useState } from 'react';

import { Button, Label, Legend, LoadingPlaceholder } from '@grafana/ui';
import { Button, HorizontalGroup, Label, Legend, LoadingPlaceholder } from '@grafana/ui';
import { useLocation } from 'react-router-dom';
import { OnCallPluginConfigPageProps } from 'types';

import logo from 'img/logo.svg';
import PluginState, { PluginStatusResponseBase } from 'state/plugin';
import { GRAFANA_LICENSE_OSS } from 'utils/consts';
import { FALLBACK_LICENSE, GRAFANA_LICENSE_OSS } from 'utils/consts';

import ConfigurationForm from './parts/ConfigurationForm';
import RemoveCurrentConfigurationButton from './parts/RemoveCurrentConfigurationButton';
Expand Down Expand Up @@ -75,13 +74,13 @@ const PluginConfigPage: FC<OnCallPluginConfigPageProps> = ({
const pluginMetaOnCallApiUrl = jsonData?.onCallApiUrl;
const processEnvOnCallApiUrl = process.env.ONCALL_API_URL; // don't destructure this, will break how webpack supplies this
const onCallApiUrl = pluginMetaOnCallApiUrl || processEnvOnCallApiUrl;
const licenseType = pluginIsConnected?.license;
const licenseType = pluginIsConnected?.license || FALLBACK_LICENSE;

const resetQueryParams = useCallback(() => removePluginConfiguredQueryParams(pluginIsEnabled), [pluginIsEnabled]);

const triggerDataSyncWithOnCall = useCallback(async () => {
resetMessages();
setSyncingPlugin(true);
setSyncError(null);

const syncDataResponse = await PluginState.syncDataWithOnCall(onCallApiUrl);

Expand Down Expand Up @@ -144,35 +143,25 @@ const PluginConfigPage: FC<OnCallPluginConfigPageProps> = ({
}
}, [pluginMetaOnCallApiUrl, processEnvOnCallApiUrl, onCallApiUrl, pluginConfiguredRedirect]);

const resetState = useCallback(() => {
const resetMessages = useCallback(() => {
setPluginResetError(null);
setPluginConnectionCheckError(null);
setPluginIsConnected(null);
setSyncError(null);
}, []);

const resetState = useCallback(() => {
resetMessages();
resetQueryParams();
}, [resetQueryParams]);

/**
* NOTE: there is a possible edge case when resetting the plugin, that would lead to an error message being shown
* (which could be fixed by just reloading the page)
* This would happen if the user removes the plugin configuration, leaves the page, then comes back to the plugin
* configuration.
*
* This is because the props being passed into this component wouldn't reflect the actual plugin
* provisioning state. The props would still have onCallApiUrl set in the plugin jsonData, so when we make the API
* call to check the plugin state w/ OnCall API the plugin-proxy would return a 502 Bad Gateway because the actual
* provisioned plugin doesn't know about the onCallApiUrl.
*
* This could be fixed by instead of passing in the plugin provisioning information as props always fetching it
* when this component renders (via a useEffect). We probably don't need to worry about this because it should happen
* very rarely, if ever
*/
const triggerPluginReset = useCallback(async () => {
setResettingPlugin(true);
resetState();

try {
await PluginState.resetPlugin();
window.location.reload();
} catch (e) {
// this should rarely, if ever happen, but we should handle the case nevertheless
setPluginResetError('There was an error resetting your plugin, try again.');
Expand All @@ -186,6 +175,15 @@ const PluginConfigPage: FC<OnCallPluginConfigPageProps> = ({
[resettingPlugin, triggerPluginReset]
);

const ReconfigurePluginButtons = () => (
<HorizontalGroup>
<Button variant="primary" onClick={triggerDataSyncWithOnCall} size="md">
Retry Sync
</Button>
{licenseType === GRAFANA_LICENSE_OSS ? <RemoveConfigButton /> : null}
</HorizontalGroup>
);

let content: React.ReactNode;

if (checkingIfPluginIsConnected) {
Expand All @@ -196,16 +194,14 @@ const PluginConfigPage: FC<OnCallPluginConfigPageProps> = ({
content = (
<>
<StatusMessageBlock text={pluginConnectionCheckError || pluginResetError} />
<RemoveConfigButton />
<ReconfigurePluginButtons />
</>
);
} else if (syncError) {
content = (
<>
<StatusMessageBlock text={syncError} />
<Button variant="primary" onClick={triggerDataSyncWithOnCall} size="md">
Retry Sync
</Button>
<ReconfigurePluginButtons />
</>
);
} else if (!pluginIsConnected) {
Expand All @@ -228,8 +224,8 @@ const PluginConfigPage: FC<OnCallPluginConfigPageProps> = ({
{pluginIsConnected ? (
<>
<p>
Plugin is connected! Continue to Grafana OnCall by clicking the{' '}
<img alt="Grafana OnCall Logo" src={logo} width={18} /> icon over there 👈
Plugin is connected! Continue to Grafana OnCall by clicking OnCall under Alerts & IRM in the navigation over
there 👈
mderynck marked this conversation as resolved.
Show resolved Hide resolved
</p>
<StatusMessageBlock
text={`Connected to OnCall (${pluginIsConnected.version}, ${pluginIsConnected.license})`}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,39 @@ exports[`PluginConfigPage If onCallApiUrl is not set in the plugin's meta jsonDa
ohhh nooo an error msg from self hosted install plugin
</span>
</pre>
<button
class="css-1ed0qk5-button"
type="button"
<div
class="css-ve64a7-horizontal-group"
style="width: 100%; height: 100%;"
>
<span
class="css-1mhnkuh"
<div
class="css-cvef6c-layoutChildrenWrapper"
>
Remove current configuration
</span>
</button>
<button
class="css-z53gi5-button"
type="button"
>
<span
class="css-1mhnkuh"
>
Retry Sync
</span>
</button>
</div>
<div
class="css-cvef6c-layoutChildrenWrapper"
>
<button
class="css-1ed0qk5-button"
type="button"
>
<span
class="css-1mhnkuh"
>
Remove current configuration
</span>
</button>
</div>
</div>
</div>
`;

Expand Down Expand Up @@ -152,16 +175,39 @@ exports[`PluginConfigPage If onCallApiUrl is set, and checkIfPluginIsConnected r
ohhh nooo a plugin connection error
</span>
</pre>
<button
class="css-1ed0qk5-button"
type="button"
<div
class="css-ve64a7-horizontal-group"
style="width: 100%; height: 100%;"
>
<span
class="css-1mhnkuh"
<div
class="css-cvef6c-layoutChildrenWrapper"
>
Remove current configuration
</span>
</button>
<button
class="css-z53gi5-button"
type="button"
>
<span
class="css-1mhnkuh"
>
Retry Sync
</span>
</button>
</div>
<div
class="css-cvef6c-layoutChildrenWrapper"
>
<button
class="css-1ed0qk5-button"
type="button"
>
<span
class="css-1mhnkuh"
>
Remove current configuration
</span>
</button>
</div>
</div>
</div>
`;

Expand All @@ -173,14 +219,7 @@ exports[`PluginConfigPage It doesn't make any network calls if the plugin config
Configure Grafana OnCall
</legend>
<p>
Plugin is connected! Continue to Grafana OnCall by clicking the

<img
alt="Grafana OnCall Logo"
src="[object Object]"
width="18"
/>
icon over there 👈
Plugin is connected! Continue to Grafana OnCall by clicking OnCall under Alerts & IRM in the navigation over there 👈
</p>
<pre
data-testid="status-message-block"
Expand Down Expand Up @@ -212,14 +251,7 @@ exports[`PluginConfigPage OnCallApiUrl is set, and syncDataWithOnCall does not r
Configure Grafana OnCall
</legend>
<p>
Plugin is connected! Continue to Grafana OnCall by clicking the

<img
alt="Grafana OnCall Logo"
src="[object Object]"
width="18"
/>
icon over there 👈
Plugin is connected! Continue to Grafana OnCall by clicking OnCall under Alerts & IRM in the navigation over there 👈
</p>
<pre
data-testid="status-message-block"
Expand Down Expand Up @@ -251,14 +283,7 @@ exports[`PluginConfigPage OnCallApiUrl is set, and syncDataWithOnCall does not r
Configure Grafana OnCall
</legend>
<p>
Plugin is connected! Continue to Grafana OnCall by clicking the

<img
alt="Grafana OnCall Logo"
src="[object Object]"
width="18"
/>
icon over there 👈
Plugin is connected! Continue to Grafana OnCall by clicking OnCall under Alerts & IRM in the navigation over there 👈
</p>
<pre
data-testid="status-message-block"
Expand Down Expand Up @@ -302,16 +327,39 @@ exports[`PluginConfigPage OnCallApiUrl is set, and syncDataWithOnCall returns an
ohhh noooo a sync issue
</span>
</pre>
<button
class="css-z53gi5-button"
type="button"
<div
class="css-ve64a7-horizontal-group"
style="width: 100%; height: 100%;"
>
<span
class="css-1mhnkuh"
<div
class="css-cvef6c-layoutChildrenWrapper"
>
Retry Sync
</span>
</button>
<button
class="css-z53gi5-button"
type="button"
>
<span
class="css-1mhnkuh"
>
Retry Sync
</span>
</button>
</div>
<div
class="css-cvef6c-layoutChildrenWrapper"
>
<button
class="css-1ed0qk5-button"
type="button"
>
<span
class="css-1mhnkuh"
>
Remove current configuration
</span>
</button>
</div>
</div>
</div>
`;

Expand All @@ -334,16 +382,39 @@ exports[`PluginConfigPage Plugin reset: successful - false 1`] = `
There was an error resetting your plugin, try again.
</span>
</pre>
<button
class="css-1ed0qk5-button"
type="button"
<div
class="css-ve64a7-horizontal-group"
style="width: 100%; height: 100%;"
>
<span
class="css-1mhnkuh"
<div
class="css-cvef6c-layoutChildrenWrapper"
>
Remove current configuration
</span>
</button>
<button
class="css-z53gi5-button"
type="button"
>
<span
class="css-1mhnkuh"
>
Retry Sync
</span>
</button>
</div>
<div
class="css-cvef6c-layoutChildrenWrapper"
>
<button
class="css-1ed0qk5-button"
type="button"
>
<span
class="css-1mhnkuh"
>
Remove current configuration
</span>
</button>
</div>
</div>
</div>
`;

Expand Down
Loading