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

Canonical visualization layer fix for missing data layer #12519

Merged

Conversation

juanignaciosl
Copy link
Contributor

@juanignaciosl juanignaciosl commented Jul 26, 2017

The linked issue contains the steps to reproduce.

Changes might impact everything related to layer sorting (needed on visualization creation and layer addition, among others) and table dependency registration (the logic that shows the modal when you try do delete a dataset with a visualization, warning about related maps).

The problem appeared with a user that has Google Maps as default basemap. That basemap doesn't have labels on top. There's an iteration through layers that reloads the map layers in order to be sure that the map contains the actual layers so we can compute the order. As a side effect, that reloading keeps map.layers up-to-date for a next saving. As a map without layers on top doesn't have a layer after the data layer, the data layer addition wasn't refreshed and wasn't saved. The fix refreshes map just after layer addition, keeping it up-to-date as early as possible.

@javitonino javitonino self-assigned this Jul 27, 2017
Copy link
Contributor

@javitonino javitonino left a comment

Choose a reason for hiding this comment

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

Change looks good. Let's make sure everything is alright in acceptance (I'm thinking most likely issues could be layer duplication and registering user table dependencies)

@javitonino javitonino assigned javitonino and unassigned javitonino Jul 27, 2017
@javitonino
Copy link
Contributor

Did some testing including:

  • Imports
  • Duplication
  • Adding/removing/moving layers
  • Checking table dependencies at each step

Everything seems to work alright. This 🚀 is ready for departure! 👏

@javitonino javitonino merged commit 045d9f1 into master Jul 27, 2017
@javitonino javitonino deleted the cbcg265-canonical_visualization_does_not_create_named_map branch July 27, 2017 08:05
@javitonino javitonino removed their assignment Jul 27, 2017
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.

2 participants