cartodb-org / cartodb

Location Intelligence & Data Visualization tool
http://cartodb.com
BSD 3-Clause "New" or "Revised" License
3 stars 2 forks source link

Enforce map legends flag is true by default #279

Closed tylerparsons closed 6 years ago

tylerparsons commented 6 years ago

Problem

When saving a copy of a sample map or opening a dataset in a map from the library, the legends of datasets on the map are hidden by default. This is not the intended or current production behavior, rather maps should always display legends, if any, by default.

This issue is caused by the legends property of the map models being incorrectly initialized. Instead of being true by default, which is the default value of the column in the database, the property is set to nil and interpreted as falsey by the UI.

Solution

When initializing any map model, namely Map and Carto::Map, the legends property should be set to true if it is nil. This ensures consistent behavior with the current blp_dev and with the database default.

Caveat

It appears that in the carto upstream, the legends property has been deprecated in favor of a flag in the options hash. Unfortunately, this change was not fully merged onto our branch and thus we still depend on this legends property in some places.

I've tested these changes with all map creation workflows, and they seem to be working. I've also identified usages of legends determined that it's largely unused in the editor. While ultimately we should migrate away from this flag as in the upstream, this issue does require a hotfix.

@gfiorav what are your thoughts? I notice you were one of the last to edit the Carto::Map file and made the same change for options[:legends].

acarrera42 commented 6 years ago

LGTM

tylerparsons commented 6 years ago

Thanks for taking a look