Remove attempt to check token when editor/viewers are accessing the p…

…lugin (#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)
grafana · Jun 29, 2023 · 5a91a3c · 5a91a3c
1 parent d2cdd66
commit 5a91a3c
Show file tree

Hide file tree

Showing 4 changed files with 38 additions and 39 deletions.
diff --git a/src/containers/PluginConfigPage/PluginConfigPage.test.tsx b/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(<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.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,15 +157,15 @@ describe('PluginConfigPage', () => {
     } as ReturnType<typeof useLocationOriginal>);
 
     PluginState.checkIfPluginIsConnected = jest.fn();
-    mockSyncDataWithOnCall();
+    mockCheckTokenAndSyncDataWithOncall();
 
     // test setup
     const component = render(<PluginConfigPage {...generateComponentProps(metaJsonDataOnCallApiUrl)} />);
     await screen.findByTestId(STATUS_MESSAGE_BLOCK_DATA_ID);
 
     // assertions
     expect(PluginState.checkIfPluginIsConnected).not.toHaveBeenCalled();
-    expect(PluginState.syncDataWithOnCall).not.toHaveBeenCalled();
+    expect(PluginState.checkTokenAndSyncDataWithOncall).not.toHaveBeenCalled();
     expect(component.container).toMatchSnapshot();
   });
 
@@ -174,15 +174,15 @@ 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()} />);
     await screen.findByTestId(PLUGIN_CONFIGURATION_FORM_DATA_ID);
 
     // 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,15 +238,15 @@ 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';
 
     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.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/src/containers/PluginConfigPage/PluginConfigPage.tsx b/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);

diff --git a/src/containers/PluginConfigPage/__snapshots__/PluginConfigPage.test.tsx.snap b/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"

diff --git a/src/state/plugin/index.ts b/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>> => {