-
Notifications
You must be signed in to change notification settings - Fork 300
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
Conversation
…roken and can be repaired or unable to communicate with backend to get maintenance status
…es to be less confusing when not self hosting, add fallback for if plugin should treat as cloud configured when unable to connect to backend, fix tests
@@ -302,7 +302,7 @@ 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); | |||
// await screen.findByTestId(successful ? PLUGIN_CONFIGURATION_FORM_DATA_ID : STATUS_MESSAGE_BLOCK_DATA_ID); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need some help on fixing this test properly now that the page is reloading on reset.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm, this type of assertion may no longer be possible for the successful
scenario. maybe we just replace this assertion by mocking window.location.reload
and assert that it was called in this scenario?
We're expecting the configuration form to be present here, but that is reliant on a full page/state reload to arrive at this state. It might be beyond what @testing-library/react
and @testing-library/user-event
are capable of? Curious to hear what someone from @grafana/grafana-oncall-frontend thinks here?
grafana-plugin/src/containers/PluginConfigPage/PluginConfigPage.tsx
Outdated
Show resolved
Hide resolved
grafana-plugin/src/containers/PluginConfigPage/PluginConfigPage.tsx
Outdated
Show resolved
Hide resolved
grafana-plugin/src/containers/PluginConfigPage/PluginConfigPage.tsx
Outdated
Show resolved
Hide resolved
Fixes issue where user not having `plugins:install`permission were unable to complete setup of OnCall. - Check multiple Grafana permissions to complete OnCall setup instead of `plugins:install` since the plugin is already installed at this point - Use the following permissions - `plugins:write` - Plugin setup will write to plugin config - `users:read` - Grafana API key being granted to OnCall will be used to read users from Grafana - `teams:read` - Grafana API key being granted to OnCall will be used to read teams from Grafana - `apikeys:create` - If Grafana API key does not exist it will be created - `apikeys:delete` - If existing Grafana API key does not work it will be deleted and recreated Closes grafana/oncall-private#1925 TODO: - [x] Fix tests
Fixes issue where user not having
plugins:install
permission were unable to complete setup of OnCall.plugins:install
since the plugin is already installed at this pointplugins:write
- Plugin setup will write to plugin configusers:read
- Grafana API key being granted to OnCall will be used to read users from Grafanateams:read
- Grafana API key being granted to OnCall will be used to read teams from Grafanaapikeys:create
- If Grafana API key does not exist it will be createdapikeys:delete
- If existing Grafana API key does not work it will be deleted and recreatedCloses https://github.com/grafana/oncall-private/issues/1925
TODO: