Skip to content
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

Fix/import dashboard sources #4068

Merged
merged 22 commits into from
Aug 1, 2018
Merged

Conversation

ischolten
Copy link
Contributor

@ischolten ischolten commented Aug 1, 2018

Closes #3980

What was the problem?
If you have different sources than the dashboard you are importing from, there is no way to map the sources from your import to your current chronograf.

What was the solution?
Add a source mapping page as a part of the import dashboard process.

  • CHANGELOG.md updated with a link to the PR (not the Issue)
  • Rebased/mergeable
  • Tests pass

Copy link
Contributor

@ebb-tide ebb-tide left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Loooking gooood!

@@ -0,0 +1,103 @@
import React from 'react'
import {shallow} from 'enzyme'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

YAYY! thanks for tests!

onImportDashboard(dashboard)
onDismissOverlay()
const cells = _.get(dashboard, 'cells', [])
const importedSources = _.get(meta, 'sources', {})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

get deep on these two lines plzz?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

word. fyi these don't need to be getDeep since they're not nested. has our convention become that we prefer getDeep generally?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed - thanks

@jaredscheib
Copy link
Contributor

jaredscheib commented Aug 1, 2018

I ran make test locally and everything passed. We're experiencing unknown Circle problems right now, the fix for which is blocked: #4055. In the meantime, this PR's build is failing. I'm going to merge in service of the release, and in the case that master breaks, we'll have effectively escalated fixing Circle.

EDIT: We figured it out and are going to resolve Circle issues first so that other branches don't check out a failing master.

alexpaxton and others added 3 commits August 1, 2018 16:09
@jaredscheib jaredscheib merged commit 2b71e67 into master Aug 1, 2018
@jaredscheib jaredscheib deleted the fix/import-dashboard-sources branch August 1, 2018 23:56
jaredscheib added a commit that referenced this pull request Aug 2, 2018
Co-authored-by: Iris Scholten <ischolten.is@gmail.com>
Co-authored-by: Alex Paxton <thealexpaxton@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants