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

Transparent datasource properties suggestions #4104

Merged
merged 5 commits into from
May 28, 2019

Conversation

philippotto
Copy link
Member

@philippotto philippotto commented May 21, 2019

@fm3 Can you double-check my small back end change please? A datastore update is needed for this, right?

  • tell user that the server suggests a new datasource json. also allow to see the old version + diff
  • removes the default "please review... use the advanced mode if you want to see the json" alert, since it felt a bit unnecessary to me (also, it's weird to have multiple alerts). the "simple" vs. "advanced" switch should be self-explanatory.

URL of deployed dev instance (used for testing):

Steps to test:

  • edit a datasource properties json (e.g., remove a resolution)
  • click on "refresh" on the datasets page and edit the dataset
  • there should be an alert telling you that wk has inferred additional information
  • clicking the links should render the old datasource-properties.json and the diff between new and old (the latter is more of a "pro feature", but I don't know if we can/should communicate this clearer)

Issues:


@philippotto philippotto self-assigned this May 21, 2019
@fm3
Copy link
Member

fm3 commented May 21, 2019

backend change LGTM :) Yes, this comes with a datastore upgrade

Copy link
Member

@daniel-wer daniel-wer left a comment

Choose a reason for hiding this comment

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

Awesome, works very well and will be quite helpful! 👍

Currently it's not possible to see in the diff whether properties were added or removed, I think, and changes to objects in arrays are hard to match, because the array index is not known, BUT I think we can always optimize this later if we find that this poses problems :)

@philippotto philippotto merged commit 0b6d8f4 into master May 28, 2019
@normanrz normanrz deleted the show-datasource-changes branch August 12, 2019 09:46
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.

3 participants