diff --git a/CHANGELOG.md b/CHANGELOG.md index 2963a516..2e28d0c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Change alerts order for `/alert` public api endpoint [#1031](https://github.com/grafana/oncall/issues/1031) - Change resolution notes order for `/resolution_notes` public api endpoint to show notes for the newest alert group on top ([#2404](https://github.com/grafana/oncall/pull/2404)) +- Remove attempt to check token when editor/viewers are accessing the plugin @mderynck ([#2410](https://github.com/grafana/oncall/pull/2410)) ## v1.3.2 (2023-06-29) diff --git a/grafana-plugin/src/containers/PluginConfigPage/PluginConfigPage.test.tsx b/grafana-plugin/src/containers/PluginConfigPage/PluginConfigPage.test.tsx index ceba2ed9..fac91c5c 100644 --- a/grafana-plugin/src/containers/PluginConfigPage/PluginConfigPage.test.tsx +++ b/grafana-plugin/src/containers/PluginConfigPage/PluginConfigPage.test.tsx @@ -71,8 +71,8 @@ afterEach(() => { console.error = originalError; }); -const mockSyncDataWithOnCall = (license: License = License.OSS) => { - PluginState.syncDataWithOnCall = jest.fn().mockResolvedValueOnce({ +const mockCheckTokenAndSyncDataWithOncall = (license: License = License.OSS) => { + PluginState.checkTokenAndSyncDataWithOncall = jest.fn().mockResolvedValueOnce({ token_ok: true, license, version: 'v1.2.3', @@ -130,7 +130,7 @@ describe('PluginConfigPage', () => { // mocks const metaJsonDataOnCallApiUrl = 'onCallApiUrlFromMetaJsonData'; PluginState.checkIfPluginIsConnected = jest.fn(); - mockSyncDataWithOnCall(); + mockCheckTokenAndSyncDataWithOncall(); // test setup render(); @@ -142,8 +142,8 @@ describe('PluginConfigPage', () => { expect(PluginState.checkIfPluginIsConnected).toHaveBeenCalledTimes(1); expect(PluginState.checkIfPluginIsConnected).toHaveBeenCalledWith(metaJsonDataOnCallApiUrl); - expect(PluginState.syncDataWithOnCall).toHaveBeenCalledTimes(1); - expect(PluginState.syncDataWithOnCall).toHaveBeenCalledWith(metaJsonDataOnCallApiUrl); + expect(PluginState.checkTokenAndSyncDataWithOncall).toHaveBeenCalledTimes(1); + expect(PluginState.checkTokenAndSyncDataWithOncall).toHaveBeenCalledWith(metaJsonDataOnCallApiUrl); }); test("It doesn't make any network calls if the plugin configured query params are provided", async () => { @@ -157,7 +157,7 @@ describe('PluginConfigPage', () => { } as ReturnType); PluginState.checkIfPluginIsConnected = jest.fn(); - mockSyncDataWithOnCall(); + mockCheckTokenAndSyncDataWithOncall(); // test setup const component = render(); @@ -165,7 +165,7 @@ describe('PluginConfigPage', () => { // assertions expect(PluginState.checkIfPluginIsConnected).not.toHaveBeenCalled(); - expect(PluginState.syncDataWithOnCall).not.toHaveBeenCalled(); + expect(PluginState.checkTokenAndSyncDataWithOncall).not.toHaveBeenCalled(); expect(component.container).toMatchSnapshot(); }); @@ -174,7 +174,7 @@ describe('PluginConfigPage', () => { delete process.env.ONCALL_API_URL; PluginState.checkIfPluginIsConnected = jest.fn(); - PluginState.syncDataWithOnCall = jest.fn(); + PluginState.checkTokenAndSyncDataWithOncall = jest.fn(); // test setup const component = render(); @@ -182,7 +182,7 @@ describe('PluginConfigPage', () => { // assertions expect(PluginState.checkIfPluginIsConnected).not.toHaveBeenCalled(); - expect(PluginState.syncDataWithOnCall).not.toHaveBeenCalled(); + expect(PluginState.checkTokenAndSyncDataWithOncall).not.toHaveBeenCalled(); expect(component.container).toMatchSnapshot(); }); @@ -192,7 +192,7 @@ describe('PluginConfigPage', () => { process.env.ONCALL_API_URL = processEnvOnCallApiUrl; PluginState.selfHostedInstallPlugin = jest.fn(); - mockSyncDataWithOnCall(); + mockCheckTokenAndSyncDataWithOncall(); // test setup render(); @@ -238,7 +238,7 @@ describe('PluginConfigPage', () => { expect(component.container).toMatchSnapshot(); }); - test('OnCallApiUrl is set, and syncDataWithOnCall returns an error', async () => { + test('OnCallApiUrl is set, and checkApiTokenSyncData returns an error', async () => { // mocks const processEnvOnCallApiUrl = 'onCallApiUrlFromProcessEnv'; const metaJsonDataOnCallApiUrl = 'onCallApiUrlFromMetaJsonData'; @@ -246,7 +246,7 @@ describe('PluginConfigPage', () => { process.env.ONCALL_API_URL = processEnvOnCallApiUrl; PluginState.checkIfPluginIsConnected = jest.fn().mockResolvedValueOnce(null); - PluginState.syncDataWithOnCall = jest.fn().mockResolvedValueOnce(SNYC_DATA_WITH_ONCALL_ERROR_MESSAGE); + PluginState.checkTokenAndSyncDataWithOncall = jest.fn().mockResolvedValueOnce(SNYC_DATA_WITH_ONCALL_ERROR_MESSAGE); // test setup const component = render(); @@ -259,7 +259,7 @@ describe('PluginConfigPage', () => { }); test.each([License.CLOUD, License.OSS])( - 'OnCallApiUrl is set, and syncDataWithOnCall does not return an error. It displays properly the plugin connected items based on the license - License: %s', + 'OnCallApiUrl is set, and checkApiTokenSyncData does not return an error. It displays properly the plugin connected items based on the license - License: %s', async (license) => { // mocks const processEnvOnCallApiUrl = 'onCallApiUrlFromProcessEnv'; @@ -268,7 +268,7 @@ describe('PluginConfigPage', () => { process.env.ONCALL_API_URL = processEnvOnCallApiUrl; PluginState.checkIfPluginIsConnected = jest.fn().mockResolvedValueOnce(null); - mockSyncDataWithOnCall(license); + mockCheckTokenAndSyncDataWithOncall(license); // test setup const component = render(); @@ -289,7 +289,7 @@ describe('PluginConfigPage', () => { process.env.ONCALL_API_URL = processEnvOnCallApiUrl; window.location.reload = jest.fn(); PluginState.checkIfPluginIsConnected = jest.fn().mockResolvedValueOnce(null); - mockSyncDataWithOnCall(License.OSS); + mockCheckTokenAndSyncDataWithOncall(License.OSS); if (successful) { PluginState.resetPlugin = jest.fn().mockResolvedValueOnce(null); @@ -310,8 +310,8 @@ describe('PluginConfigPage', () => { expect(PluginState.checkIfPluginIsConnected).toHaveBeenCalledTimes(1); expect(PluginState.checkIfPluginIsConnected).toHaveBeenCalledWith(metaJsonDataOnCallApiUrl); - expect(PluginState.syncDataWithOnCall).toHaveBeenCalledTimes(1); - expect(PluginState.syncDataWithOnCall).toHaveBeenCalledWith(metaJsonDataOnCallApiUrl); + expect(PluginState.checkTokenAndSyncDataWithOncall).toHaveBeenCalledTimes(1); + expect(PluginState.checkTokenAndSyncDataWithOncall).toHaveBeenCalledWith(metaJsonDataOnCallApiUrl); expect(PluginState.resetPlugin).toHaveBeenCalledTimes(1); expect(PluginState.resetPlugin).toHaveBeenCalledWith(); diff --git a/grafana-plugin/src/containers/PluginConfigPage/PluginConfigPage.tsx b/grafana-plugin/src/containers/PluginConfigPage/PluginConfigPage.tsx index 6b25db87..2919ad32 100644 --- a/grafana-plugin/src/containers/PluginConfigPage/PluginConfigPage.tsx +++ b/grafana-plugin/src/containers/PluginConfigPage/PluginConfigPage.tsx @@ -82,7 +82,7 @@ const PluginConfigPage: FC = ({ resetMessages(); setSyncingPlugin(true); - const syncDataResponse = await PluginState.syncDataWithOnCall(onCallApiUrl); + const syncDataResponse = await PluginState.checkTokenAndSyncDataWithOncall(onCallApiUrl); if (typeof syncDataResponse === 'string') { setSyncError(syncDataResponse); diff --git a/grafana-plugin/src/containers/PluginConfigPage/__snapshots__/PluginConfigPage.test.tsx.snap b/grafana-plugin/src/containers/PluginConfigPage/__snapshots__/PluginConfigPage.test.tsx.snap index cd7e024f..32d6cc10 100644 --- a/grafana-plugin/src/containers/PluginConfigPage/__snapshots__/PluginConfigPage.test.tsx.snap +++ b/grafana-plugin/src/containers/PluginConfigPage/__snapshots__/PluginConfigPage.test.tsx.snap @@ -243,7 +243,7 @@ exports[`PluginConfigPage It doesn't make any network calls if the plugin config `; -exports[`PluginConfigPage OnCallApiUrl is set, and syncDataWithOnCall does not return an error. It displays properly the plugin connected items based on the license - License: OpenSource 1`] = ` +exports[`PluginConfigPage OnCallApiUrl is set, and checkApiTokenSyncData does not return an error. It displays properly the plugin connected items based on the license - License: OpenSource 1`] = `
`; -exports[`PluginConfigPage OnCallApiUrl is set, and syncDataWithOnCall does not return an error. It displays properly the plugin connected items based on the license - License: some-other-license 1`] = ` +exports[`PluginConfigPage OnCallApiUrl is set, and checkApiTokenSyncData does not return an error. It displays properly the plugin connected items based on the license - License: some-other-license 1`] = `
`; -exports[`PluginConfigPage OnCallApiUrl is set, and syncDataWithOnCall returns an error 1`] = ` +exports[`PluginConfigPage OnCallApiUrl is set, and checkApiTokenSyncData returns an error 1`] = `
=> { try { - /** - * Allows the plugin config page to repair settings like the app initialization screen if a user deletes - * an API key on accident but leaves the plugin settings intact. - */ - const existingKey = await this.getGrafanaToken(); - if (!existingKey) { - try { - await this.installPlugin(); - } catch (e) { - return this.getHumanReadableErrorFromOnCallError( - e, - onCallApiUrl, - 'install', - onCallApiUrlIsConfiguredThroughEnvVar - ); - } - } - const startSyncResponse = await makeRequest(`${this.ONCALL_BASE_URL}/sync`, { method: 'POST' }); if (typeof startSyncResponse === 'string') { // an error occurred trying to initiate the sync @@ -294,6 +276,23 @@ class PluginState { } }; + static checkTokenAndSyncDataWithOncall = async (onCallApiUrl: string): Promise => { + /** + * Allows the plugin config page to repair settings like the app initialization screen if a user deletes + * an API key on accident but leaves the plugin settings intact. + */ + const existingKey = await PluginState.getGrafanaToken(); + if (!existingKey) { + try { + await PluginState.installPlugin(); + } catch (e) { + return PluginState.getHumanReadableErrorFromOnCallError(e, onCallApiUrl, 'install', false); + } + } + + return await PluginState.syncDataWithOnCall(onCallApiUrl); + }; + static installPlugin = async ( selfHosted = false ): Promise> => {