Remove attempt to check token when editor/viewers are accessing the plugin (#2410)

# What this PR does
Remove attempt to check token when editor/viewers are accessing the
plugin. Only check the token for reinstall during sync from the
PluginConfigPage since only Admins would have access to it.

## Which issue(s) this PR fixes

## Checklist

- [X] Unit, integration, and e2e (if applicable) tests updated
- [X] Documentation added (or `pr:no public docs` PR label added if not
required)
- [X] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not
required)

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)
 

grafana-plugin/src/containers/PluginConfigPage/PluginConfigPage.test.tsx

@@ -71,8 +71,8 @@ afterEach(() => {
   console.error = originalError;
 });
 
-const mockSyncDataWithOnCall = (license: License = License.OSS) => {
+const mockCheckTokenAndSyncDataWithOncall = (license: License = License.OSS) => {
-  PluginState.syncDataWithOnCall = jest.fn().mockResolvedValueOnce({
+  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(<PluginConfigPage {...generateComponentProps(metaJsonDataOnCallApiUrl, true)} />);
@@ -142,8 +142,8 @@ describe('PluginConfigPage', () => {
     expect(PluginState.checkIfPluginIsConnected).toHaveBeenCalledTimes(1);
     expect(PluginState.checkIfPluginIsConnected).toHaveBeenCalledWith(metaJsonDataOnCallApiUrl);
 
-    expect(PluginState.syncDataWithOnCall).toHaveBeenCalledTimes(1);
+    expect(PluginState.checkTokenAndSyncDataWithOncall).toHaveBeenCalledTimes(1);
-    expect(PluginState.syncDataWithOnCall).toHaveBeenCalledWith(metaJsonDataOnCallApiUrl);
+    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<typeof useLocationOriginal>);
 
     PluginState.checkIfPluginIsConnected = jest.fn();
-    mockSyncDataWithOnCall();
+    mockCheckTokenAndSyncDataWithOncall();
 
     // test setup
     const component = render(<PluginConfigPage {...generateComponentProps(metaJsonDataOnCallApiUrl)} />);
@@ -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(<PluginConfigPage {...generateComponentProps()} />);
@@ -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(<PluginConfigPage {...generateComponentProps()} />);
@@ -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(<PluginConfigPage {...generateComponentProps(metaJsonDataOnCallApiUrl)} />);
@@ -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(<PluginConfigPage {...generateComponentProps(metaJsonDataOnCallApiUrl)} />);
@@ -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.checkTokenAndSyncDataWithOncall).toHaveBeenCalledTimes(1);
-    expect(PluginState.syncDataWithOnCall).toHaveBeenCalledWith(metaJsonDataOnCallApiUrl);
+    expect(PluginState.checkTokenAndSyncDataWithOncall).toHaveBeenCalledWith(metaJsonDataOnCallApiUrl);
 
     expect(PluginState.resetPlugin).toHaveBeenCalledTimes(1);
     expect(PluginState.resetPlugin).toHaveBeenCalledWith();

grafana-plugin/src/containers/PluginConfigPage/PluginConfigPage.tsx

@@ -82,7 +82,7 @@ const PluginConfigPage: FC<OnCallPluginConfigPageProps> = ({
     resetMessages();
     setSyncingPlugin(true);
 
-    const syncDataResponse = await PluginState.syncDataWithOnCall(onCallApiUrl);
+    const syncDataResponse = await PluginState.checkTokenAndSyncDataWithOncall(onCallApiUrl);
 
     if (typeof syncDataResponse === 'string') {
       setSyncError(syncDataResponse);

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
 </div>
 `;
 
-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`] = `
 <div>
   <legend
     class="css-11wqcat"
@@ -275,7 +275,7 @@ exports[`PluginConfigPage OnCallApiUrl is set, and syncDataWithOnCall does not r
 </div>
 `;
 
-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`] = `
 <div>
   <legend
     class="css-11wqcat"
@@ -308,7 +308,7 @@ exports[`PluginConfigPage OnCallApiUrl is set, and syncDataWithOnCall does not r
 </div>
 `;
 
-exports[`PluginConfigPage OnCallApiUrl is set, and syncDataWithOnCall returns an error 1`] = `
+exports[`PluginConfigPage OnCallApiUrl is set, and checkApiTokenSyncData returns an error 1`] = `
 <div>
   <legend
     class="css-11wqcat"

grafana-plugin/src/state/plugin/index.ts

@@ -260,24 +260,6 @@ class PluginState {
     onCallApiUrlIsConfiguredThroughEnvVar = false
   ): Promise<PluginSyncStatusResponse | string> => {
     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<PluginSyncStatusResponse | string> => {
+    /**
+     * 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 <RT = CloudProvisioningConfigResponse>(
     selfHosted = false
   ): Promise<InstallPluginResponse<RT>> => {