Datasources: Fix deletion of data source if plugin is not found #40095

jackw · 2021-10-06T15:57:48Z

What this PR does / why we need it:
This PR makes it possible to delete a datasource if the plugin cannot be found / has been uninstalled. Most of the code already existed but an unhandled error prevented it from rendering. After handling the plugin loading error it revealed a second issue where the delete functionality either didn’t work (due to missing redux state) or allowed a user to delete the wrong datasource (due to out of sync redux state).

Thanks to @mckn & @leventebalogh for taking the time to help me understand the code and bounce ideas around on how to fix it. 🥇

Which issue(s) this PR fixes:

Fixes #37650

Special notes for your reviewer:

hugohaggmark

I did a shallow code review and the code looks great, left some comments for consideration.

public/app/features/datasources/state/actions.ts

public/app/features/datasources/state/reducers.ts

mckn

LGTM! Seems to work.

torkelo · 2021-10-07T13:20:42Z

Nice!

leventebalogh

👍

leventebalogh · 2021-10-08T06:12:11Z

public/app/features/datasources/settings/DataSourceSettingsPage.tsx

@@ -111,7 +111,7 @@ export class DataSourceSettingsPage extends PureComponent<Props> {
    appEvents.publish(
      new ShowConfirmModalEvent({
        title: 'Delete',
-        text: 'Are you sure you want to delete this data source?',
+        text: `Are you sure you want to delete the "${this.props.dataSource.name}" data source?`,


…ailures still render ui

…ore has info for deletion

…e and meta loading

…0095) * fix(pluginsettings): reject with error so datasource plugin loading failures still render ui * feat(pluginpage): handle plugin loading error * refactor(datasources): separate out datasource and meta loading so store has info for deletion * fix(datasourcesettings): introduce loading flag to wait for datasource and meta loading * test(datasourcesettings): fix failing test * test(datasources): assert loading status of datasource settings * test(datasources): update action tests for latest changes (cherry picked from commit 7cf3c84)

…0095) (#40209) * fix(pluginsettings): reject with error so datasource plugin loading failures still render ui * feat(pluginpage): handle plugin loading error * refactor(datasources): separate out datasource and meta loading so store has info for deletion * fix(datasourcesettings): introduce loading flag to wait for datasource and meta loading * test(datasourcesettings): fix failing test * test(datasources): assert loading status of datasource settings * test(datasources): update action tests for latest changes (cherry picked from commit 7cf3c84) Co-authored-by: Jack Westbrook <[email protected]>

…0095) * fix(pluginsettings): reject with error so datasource plugin loading failures still render ui * feat(pluginpage): handle plugin loading error * refactor(datasources): separate out datasource and meta loading so store has info for deletion * fix(datasourcesettings): introduce loading flag to wait for datasource and meta loading * test(datasourcesettings): fix failing test * test(datasources): assert loading status of datasource settings * test(datasources): update action tests for latest changes

@grafanabot

…backs (#39919) * add custom icons * use resourcePicker for marker icon * use regular shapes for specific icons * update type * update svgs and remove marker shape selection * update types and resourcePicker * add migration and update markers Co-authored-by: Ryan McKinley <[email protected]> * quick cleanup * update marker path and remove any * update migration test * use inline snapshot * remove unused * Docs: Add documentation for library elements API (#39829) * LibraryElements: Adds api documentation * Update docs/sources/http_api/library_element.md Co-authored-by: achatterjee-grafana <[email protected]> * Update docs/sources/http_api/library_element.md Co-authored-by: achatterjee-grafana <[email protected]> * Update docs/sources/http_api/library_element.md Co-authored-by: achatterjee-grafana <[email protected]> * Update docs/sources/http_api/library_element.md Co-authored-by: achatterjee-grafana <[email protected]> * Update docs/sources/http_api/library_element.md Co-authored-by: achatterjee-grafana <[email protected]> * Update docs/sources/http_api/library_element.md Co-authored-by: achatterjee-grafana <[email protected]> * Update docs/sources/http_api/library_element.md Co-authored-by: achatterjee-grafana <[email protected]> * Update docs/sources/http_api/library_element.md Co-authored-by: achatterjee-grafana <[email protected]> * Update docs/sources/http_api/library_element.md Co-authored-by: achatterjee-grafana <[email protected]> * Update docs/sources/http_api/library_element.md Co-authored-by: achatterjee-grafana <[email protected]> * Update docs/sources/http_api/library_element.md Co-authored-by: achatterjee-grafana <[email protected]> * Update docs/sources/http_api/library_element.md Co-authored-by: achatterjee-grafana <[email protected]> * Update docs/sources/http_api/library_element.md Co-authored-by: achatterjee-grafana <[email protected]> * Update docs/sources/http_api/library_element.md Co-authored-by: achatterjee-grafana <[email protected]> * Update docs/sources/http_api/library_element.md Co-authored-by: achatterjee-grafana <[email protected]> * Update docs/sources/http_api/library_element.md Co-authored-by: achatterjee-grafana <[email protected]> * Update docs/sources/http_api/library_element.md Co-authored-by: achatterjee-grafana <[email protected]> * Update docs/sources/http_api/library_element.md Co-authored-by: achatterjee-grafana <[email protected]> * Update docs/sources/http_api/library_element.md Co-authored-by: achatterjee-grafana <[email protected]> * Update docs/sources/http_api/library_element.md Co-authored-by: achatterjee-grafana <[email protected]> * Update docs/sources/http_api/library_element.md Co-authored-by: achatterjee-grafana <[email protected]> * Update docs/sources/http_api/library_element.md Co-authored-by: achatterjee-grafana <[email protected]> * Update docs/sources/http_api/library_element.md Co-authored-by: achatterjee-grafana <[email protected]> * Refactor: changes after PR comments * Apply suggestions from code review Co-authored-by: achatterjee-grafana <[email protected]> * Chore: updates after review Co-authored-by: achatterjee-grafana <[email protected]> * CodeEditor: making sure we trigger the latest onSave callback provided to the component (#39835) * Fix: prevent queryDisplyText in QueryRowHeader from overflowing (#40094) * Revert "Fix Query Editor Row horizontal overflow (#39419)" This reverts commit 42b1fa0. * fix: prevent queryDisplyText in QueryRowHeader from overflowing * Search: Fix local storage key (#40127) * Default to 'General' if no folder title is present * Add bottom padding * Live: remote write sampling (#40079) * Stat: recompute shared y range during streaming updates (#39485) * NavBar: Order App plugins alphabetically (#40078) * NavBar: Order App plugins alphabetically * Update pkg/api/index.go Co-authored-by: Will Browne <[email protected]> Co-authored-by: Will Browne <[email protected]> * Schema: use the generated graph.gen.ts (#40090) * actually generate graph.gen.ts * getting closer * keep file where it is * manual fixes * Update packages/grafana-schema/src/schema/graph.gen.ts Co-authored-by: sam boyer <[email protected]> * Docs: Whats new in 8.2 (#39945) * Added time range controls updates * Added plugins catalog update * Added enterprise images * Added community contributions highlights for 8.2 * accessibility statement * Update docs/sources/whatsnew/whats-new-in-v8-2.md Co-authored-by: Fiona Artiaga <[email protected]> * Update docs/sources/whatsnew/whats-new-in-v8-2.md Co-authored-by: Fiona Artiaga <[email protected]> * Update docs/sources/whatsnew/whats-new-in-v8-2.md Co-authored-by: Fiona Artiaga <[email protected]> * Update docs/sources/whatsnew/whats-new-in-v8-2.md Co-authored-by: Fiona Artiaga <[email protected]> * Update docs/sources/whatsnew/whats-new-in-v8-2.md Co-authored-by: achatterjee-grafana <[email protected]> * Update docs/sources/whatsnew/whats-new-in-v8-2.md Co-authored-by: achatterjee-grafana <[email protected]> * Update docs/sources/whatsnew/whats-new-in-v8-2.md Co-authored-by: Fiona Artiaga <[email protected]> * Update docs/sources/whatsnew/whats-new-in-v8-2.md Co-authored-by: achatterjee-grafana <[email protected]> * Update docs/sources/whatsnew/whats-new-in-v8-2.md Co-authored-by: achatterjee-grafana <[email protected]> * Apply suggestions from code review Co-authored-by: Fiona Artiaga <[email protected]> Co-authored-by: Fiona Artiaga <[email protected]> Co-authored-by: achatterjee-grafana <[email protected]> * Live: array for Processor, Outputter and Subscriber in channel rule top level (#39677) * ReleaseNotes: Updated changelog and release notes for 8.1.7 (#40081) * more * more Co-authored-by: sam boyer <[email protected]> Co-authored-by: Petros Kolyvas <[email protected]> Co-authored-by: Fiona Artiaga <[email protected]> Co-authored-by: achatterjee-grafana <[email protected]> Co-authored-by: Alexander Emelin <[email protected]> Co-authored-by: Grot (@grafanabot) <[email protected]> * Access Control: Add scope type prefix (#40076) * prefix runtime scopes with key type * Bump mocha from 7.0.1 to 9.1.2 (#39979) * Bump mocha from 7.0.1 to 9.1.2 Bumps [mocha](https://github.com/mochajs/mocha) from 7.0.1 to 9.1.2. - [Release notes](https://github.com/mochajs/mocha/releases) - [Changelog](https://github.com/mochajs/mocha/blob/master/CHANGELOG.md) - [Commits](mochajs/mocha@v7.0.1...v9.1.2) --- updated-dependencies: - dependency-name: mocha dependency-type: direct:development update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]> * kick drone * Remove mocha as it's not used by anything * kick drone Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Ashley Harrison <[email protected]> * ReleaseNotes: Updated changelog and release notes for 8.2.0 (#40141) * ReleaseNotes: Updated changelog and release notes for 8.2.0 * Add link & remove empty line in CHANGELOG * remove empty line Co-authored-by: Elfo404 <[email protected]> * Create search filters by interface (#39843) * Extract search users to a new service * Fix wire provider * Fix common_test and remove RouteRegister * Remove old endpoints * Fix test * Create search filters using interfaces * Move Enterprise filter, rename filter for filters and allow use filters with params * Each filter has unique key * Back activeLast30Days filter to OSS * Fix tests * Delete unusued param * Move filters to searchusers service and small refactor * Fix tests * Encryption: Refactor securejsondata.SecureJsonData to stop relying on global functions (#38865) * Encryption: Add support to encrypt/decrypt sjd * Add datasources.Service as a proxy to datasources db operations * Encrypt ds.SecureJsonData before calling SQLStore * Move ds cache code into ds service * Fix tlsmanager tests * Fix pluginproxy tests * Remove some securejsondata.GetEncryptedJsonData usages * Add pluginsettings.Service as a proxy for plugin settings db operations * Add AlertNotificationService as a proxy for alert notification db operations * Remove some securejsondata.GetEncryptedJsonData usages * Remove more securejsondata.GetEncryptedJsonData usages * Fix lint errors * Minor fixes * Remove encryption global functions usages from ngalert * Fix lint errors * Minor fixes * Minor fixes * Remove securejsondata.DecryptedValue usage * Refactor the refactor * Remove securejsondata.DecryptedValue usage * Move securejsondata to migrations package * Move securejsondata to migrations package * Minor fix * Fix integration test * Fix integration tests * Undo undesired changes * Fix tests * Add context.Context into encryption methods * Fix tests * Fix tests * Fix tests * Trigger CI * Fix test * Add names to params of encryption service interface * Remove bus from CacheServiceImpl * Add logging * Add keys to logger Co-authored-by: Emil Tullstedt <[email protected]> * Add missing key to logger Co-authored-by: Emil Tullstedt <[email protected]> * Undo changes in markdown files * Fix formatting * Add context to secrets service * Rename decryptSecureJsonData to decryptSecureJsonDataFn * Name args in GetDecryptedValueFn * Add template back to NewAlertmanagerNotifier * Copy GetDecryptedValueFn to ngalert * Add logging to pluginsettings * Fix pluginsettings test Co-authored-by: Tania B <[email protected]> Co-authored-by: Emil Tullstedt <[email protected]> * Admin: Enable extending filters (#39825) * Setup extensible filters * Fix test * Handle filter as array * Add className * Abstract getFilters * Make docs link external * Use underline for links in tooltips instead of link color Co-authored-by: Selene <[email protected]> * Chore: update latest.json to 8.2 (#40153) * Doc: Fixed issue 40017 (#40152) * Added content as suggested by Will * removed a few extra words. * Update docs/sources/administration/configuration.md Co-authored-by: Fiona Artiaga <[email protected]> * Update docs/sources/administration/configuration.md Co-authored-by: Fiona Artiaga <[email protected]> * Update docs/sources/administration/configuration.md Co-authored-by: Fiona Artiaga <[email protected]> Co-authored-by: Fiona Artiaga <[email protected]> * docs: Add keepCokkies cofiguration option in datasources (#39890) Signed-off-by: Vinayak Kadam <[email protected]> * Grammar issues (#40168) * Packaging: document systemd net bind capability rpm and deb installations (#40165) * add systemd net bind capability docs for rpm and deb Co-authored-by: achatterjee-grafana <[email protected]> * Alerting: declare constants for __dashboardUid__ and __panelId__ literals (#39976) * Babel: Remove unused plugin (#40172) * removed unused babel plugin * Update lock file * Chore(dependencies): Remove puppeteer since we don't use it anywhere (#40137) * Folders: Prevents deletion of General folder (#40192) * Datasources: Fix deletion of datasource if plugin cannot be found (#40095) * fix(pluginsettings): reject with error so datasource plugin loading failures still render ui * feat(pluginpage): handle plugin loading error * refactor(datasources): separate out datasource and meta loading so store has info for deletion * fix(datasourcesettings): introduce loading flag to wait for datasource and meta loading * test(datasourcesettings): fix failing test * test(datasources): assert loading status of datasource settings * test(datasources): update action tests for latest changes * Replace SAML library with fork (#40149) * Update saml library to latest * Use fork of crewjam/saml with fix for certificate chain bug * CloudMonitoring: Migrate to use backend plugin SDK contracts (#38650) * Use SDK contracts for cloudmonitoring * Get build running, tests passing and do some refactoring (#38754) * fix build+tests and refactor * remove alerting stuff * remove unused field * fix plugin fetch * end to end * resp rename * tidy annotations * reformatting * update refID * reformat imports * fix styling * clean up unmarshalling * uncomment + fix tests * appease linter * remove spaces * remove old cruft * add check for empty queries * update tests * remove pm as dep * adjust proxy route contract * fix service loading * use UNIX val * fix endpoint + resp * h@ckz for frontend * fix resp * fix interval * always set custom meta * remove unused param * fix labels fetch * fix linter * fix test + remove unused field * apply pr feedback * fix grafana-auto intervals * fix tests * resolve conflicts * fix bad merge * fix conflicts * remove bad logger import Co-authored-by: Will Browne <[email protected]> Co-authored-by: Will Browne <[email protected]> * Do codegen and check no-diff of all (non-blacklisted) CUE->TS codegen during CI (#39922) * Add file blacklist to `grafana-cli cue gen-ts` cmd * Add CI step checking all cuetsification is done * Add dummy command to make the next one fail * Generate drone bits * Check diff output failure * Echo list of untracked files, for failure locality * Move git cleanness checking into script * Blacklist of cue files is complete and correct * Remove news panel plugin from cuetsify blacklist * Dummy commit, check that untracked gen still fail * Tie off remaining errors * Re-add barchart to blacklist * Remove file left around by earlier pipeline * Commit generated news models.gen.ts * Include eslint as part of cuetsified output gen * Update pkg/cmd/grafana-cli/commands/cuetsify_command.go Co-authored-by: Ashley Harrison <[email protected]> * Update scripts/drone/steps/lib.star Co-authored-by: Maria Alexandra <[email protected]> * Update drone.yml * Last fix on .drone.yml Co-authored-by: Ashley Harrison <[email protected]> Co-authored-by: Maria Alexandra <[email protected]> * Alerting: add organziation ID to the ngAlert webhook payload (#40189) * Alerting: add organziation ID to the ngAlert webhook payload * remove systemcallfilters sections from systemd unit files (#40176) * Add Headers to http client Options (#40214) * Docs: Add required library for the image renderer (#40201) * update permissions scopes and description for role scopes (#40206) * Chore: Migrate yarn from v1 to v2 (#39082) * Chore: Migrate yarn from v1 to v2 Co-authored-by: Hugo Häggmark <[email protected]> * ReleaseNotes: Updated changelog and release notes for 8.2.0 (#40233) Co-authored-by: Ryan McKinley <[email protected]> Co-authored-by: Hugo Häggmark <[email protected]> Co-authored-by: achatterjee-grafana <[email protected]> Co-authored-by: Marcus Andersson <[email protected]> Co-authored-by: Giordano Ricci <[email protected]> Co-authored-by: Alex Khomenko <[email protected]> Co-authored-by: Alexander Emelin <[email protected]> Co-authored-by: Leon Sorokin <[email protected]> Co-authored-by: Ashley Harrison <[email protected]> Co-authored-by: Will Browne <[email protected]> Co-authored-by: Ryan McKinley <[email protected]> Co-authored-by: sam boyer <[email protected]> Co-authored-by: Petros Kolyvas <[email protected]> Co-authored-by: Fiona Artiaga <[email protected]> Co-authored-by: Grot (@grafanabot) <[email protected]> Co-authored-by: Karl Persson <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Selene <[email protected]> Co-authored-by: Joan López de la Franca Beltran <[email protected]> Co-authored-by: Tania B <[email protected]> Co-authored-by: Emil Tullstedt <[email protected]> Co-authored-by: Vinayak <[email protected]> Co-authored-by: Anne E. Ulrich <[email protected]> Co-authored-by: Kevin Minehart <[email protected]> Co-authored-by: Yuriy Tseretyan <[email protected]> Co-authored-by: Torkel Ödegaard <[email protected]> Co-authored-by: Jack Westbrook <[email protected]> Co-authored-by: Alexander Zobnin <[email protected]> Co-authored-by: idafurjes <[email protected]> Co-authored-by: Will Browne <[email protected]> Co-authored-by: Maria Alexandra <[email protected]> Co-authored-by: Jean-Philippe Quéméner <[email protected]> Co-authored-by: Dimitris Sotirakis <[email protected]> Co-authored-by: Agnès Toulet <[email protected]> Co-authored-by: kay delaney <[email protected]> Co-authored-by: Hugo Häggmark <[email protected]>

jackw added area/plugins area/datasource/frontend labels Oct 6, 2021

jackw requested review from a team October 6, 2021 15:57

jackw self-assigned this Oct 6, 2021

jackw requested review from axelavargas and natellium and removed request for a team October 6, 2021 15:57

grafanabot added the area/frontend label Oct 6, 2021

jackw requested review from hugohaggmark and removed request for natellium October 6, 2021 15:58

hugohaggmark reviewed Oct 7, 2021

View reviewed changes

public/app/features/datasources/state/actions.ts Outdated Show resolved Hide resolved

public/app/features/datasources/state/reducers.ts Show resolved Hide resolved

public/app/features/datasources/state/reducers.ts Show resolved Hide resolved

jackw added the type/bug label Oct 7, 2021

jackw marked this pull request as ready for review October 7, 2021 11:13

jackw requested a review from a team as a code owner October 7, 2021 11:13

jackw changed the title ~~[WIP] Datasources: Fix deletion of datasource if plugin cannot be found~~ Datasources: Fix deletion of datasource if plugin cannot be found Oct 7, 2021

jackw added this to the 8.2.1 milestone Oct 7, 2021

mckn approved these changes Oct 7, 2021

View reviewed changes

leventebalogh approved these changes Oct 8, 2021

View reviewed changes

jackw added add to changelog old backport v8.2.x Mark PR for automatic backport to v8.2.x labels Oct 8, 2021

jackw added 7 commits October 8, 2021 10:20

fix(pluginsettings): reject with error so datasource plugin loading f…

b39d056

…ailures still render ui

feat(pluginpage): handle plugin loading error

315d8b7

refactor(datasources): separate out datasource and meta loading so st…

fa5c09f

…ore has info for deletion

fix(datasourcesettings): introduce loading flag to wait for datasourc…

b16ebbc

…e and meta loading

test(datasourcesettings): fix failing test

c353e9e

test(datasources): assert loading status of datasource settings

e90dba0

test(datasources): update action tests for latest changes

5130a9e

jackw force-pushed the jackw/allow-delete-datasource branch from 4f8f31a to 5130a9e Compare October 8, 2021 08:20

jackw merged commit 7cf3c84 into main Oct 8, 2021

jackw deleted the jackw/allow-delete-datasource branch October 8, 2021 10:31

grafanabot mentioned this pull request Oct 8, 2021

[v8.2.x] Datasources: Fix deletion of datasource if plugin cannot be found #40209

Merged

ashharrison90 changed the title ~~Datasources: Fix deletion of datasource if plugin cannot be found~~ Datasources: Fix deletion of data source if plugin is not found Oct 11, 2021

jtorkkel mentioned this pull request Mar 16, 2022

Dashboard when saved in 8.4 does not work in 8.2/7.5, works in 8.3 #46528

Closed

natellium mentioned this pull request Apr 6, 2022

Button to delete a data source not shown if there's an error #32814

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Datasources: Fix deletion of data source if plugin is not found #40095

Datasources: Fix deletion of data source if plugin is not found #40095

jackw commented Oct 6, 2021 •

edited

Loading

hugohaggmark left a comment

mckn left a comment

torkelo commented Oct 7, 2021

leventebalogh left a comment

leventebalogh Oct 8, 2021

Datasources: Fix deletion of data source if plugin is not found #40095

Datasources: Fix deletion of data source if plugin is not found #40095

Conversation

jackw commented Oct 6, 2021 • edited Loading

hugohaggmark left a comment

Choose a reason for hiding this comment

mckn left a comment

Choose a reason for hiding this comment

torkelo commented Oct 7, 2021

leventebalogh left a comment

Choose a reason for hiding this comment

leventebalogh Oct 8, 2021

Choose a reason for hiding this comment

jackw commented Oct 6, 2021 •

edited

Loading