Move widget position logic migration to the backend and remove some u…

…nused code.

Closes getredash#2218.

client/app/services/dashboard.js

@@ -2,32 +2,13 @@ import * as _ from 'underscore';
 
 function Dashboard($resource, $http, currentUser, Widget, dashboardGridOptions) {
   function prepareDashboardWidgets(widgets) {
-    if (_.isArray(widgets) && (widgets.length > 0) && _.isArray(widgets[0])) {
-      // Dashboard v1 processing
-      // v1 dashboard has two columns, and widget can occupy one of them or both;
-      // this means, that there can be at most two widgets per row.
-      // Here we will map gridster columns and rows to v1-style grid
-      const dashboardV1ColumnSize = Math.round(dashboardGridOptions.columns / 2);
-      widgets = _.map(
-        widgets,
-        (row, rowIndex) => _.map(row, (widget, widgetIndex) => {
-          widget.options = widget.options || {};
-          widget.options.position = _.extend({}, {
-            row: rowIndex,
-            col: widgetIndex * dashboardV1ColumnSize,
-            sizeX: dashboardV1ColumnSize * widget.width,
-            // do not set sizeY - let widget to use defaults for visualization
-          }, widget.options.position);
-          return widget;
-        }),
-      );
-    }
-
-    return _.map(_.flatten(widgets), widget => new Widget(widget));
+    return widgets.map(widget => new Widget(widget));
   }
 
   function transformSingle(dashboard) {
-    dashboard.widgets = prepareDashboardWidgets(dashboard.widgets);
+    if (dashboard.widgets) {
+      dashboard.widgets = prepareDashboardWidgets(dashboard.widgets);
+    }
     dashboard.publicAccessEnabled = dashboard.public_url !== undefined;
   }
 

client/app/visualizations/chart/index.js

@@ -270,7 +270,7 @@ export default function init(ngModule) {
       columnMapping: {},
       defaultColumns: 3,
       defaultRows: 8,
-      minColumns: 2,
+      minColumns: 1,
       minRows: 5,
     };
 

migrations/versions/969126bd800f_.py

@@ -0,0 +1,69 @@
+"""Update widget's position data based on dashboard layout.
+
+Revision ID: 969126bd800f
+Revises: 6b5be7e0a0ef
+Create Date: 2018-01-31 15:20:30.396533
+
+"""
+import json
+from alembic import op
+import sqlalchemy as sa
+
+from redash.models import Dashboard, Widget, db
+
+
+# revision identifiers, used by Alembic.
+revision = '969126bd800f'
+down_revision = '6b5be7e0a0ef'
+branch_labels = None
+depends_on = None
+
+
+def upgrade():
+    # Update widgets position data:
+    column_size = 3
+    print "Updating dashboards position data:"
+    for dashboard in Dashboard.query:
+        print "  Updating dashboard: {}".format(dashboard.id)
+        layout = json.loads(dashboard.layout)
+
+        print "    Building widgets map:"
+        widgets = {}
+        for w in dashboard.widgets:
+            print "    Widget: {}".format(w.id)
+            widgets[w.id] = w
+
+        print "    Iterating over layout:"
+        for row_index, row in enumerate(layout):
+            print "      Row: {} - {}".format(row_index, row)
+            if row is None:
+                continue
+
+            for column_index, widget_id in enumerate(row):
+                print "      Column: {} - {}".format(column_index, widget_id)
+                widget = widgets.get(widget_id)
+
+                if widget is None:
+                    continue
+
+                options = json.loads(widget.options) or {}
+                options['position'] = {
+                    "row": row_index,
+                    "col": column_index * column_size,
+                    "sizeX": column_size * widget.width
+                }
+
+                widget.options = json.dumps(options)
+
+                db.session.add(widget)
+
+    db.session.commit()
+
+    # Remove legacy columns no longer in use.
+    op.drop_column('widgets', 'type')
+    op.drop_column('widgets', 'query_id')
+
+
+def downgrade():
+    op.add_column('widgets', sa.Column('query_id', sa.INTEGER(), autoincrement=False, nullable=True))
+    op.add_column('widgets', sa.Column('type', sa.VARCHAR(length=100), autoincrement=False, nullable=True))

redash/handlers/widgets.py

@@ -21,9 +21,6 @@ def post(self):
         :<json number width: Width for widget display
 
         :>json object widget: The created widget
-        :>json array layout: The new layout of the dashboard this widget was added to
-        :>json boolean new_row: Whether this widget was added on a new row or not
-        :>json number version: The revision number of the dashboard
         """
         widget_properties = request.get_json(force=True)
         dashboard = models.Dashboard.get_by_id_and_org(widget_properties.pop('dashboard_id'), self.current_org)
@@ -46,25 +43,8 @@ def post(self):
         models.db.session.add(widget)
         models.db.session.commit()
 
-        layout = json.loads(widget.dashboard.layout)
-        new_row = True
-
-        if len(layout) == 0 or widget.width == 2:
-            layout.append([widget.id])
-        elif len(layout[-1]) == 1:
-            neighbour_widget = models.Widget.query.get(layout[-1][0])
-            if neighbour_widget.width == 1:
-                layout[-1].append(widget.id)
-                new_row = False
-            else:
-                layout.append([widget.id])
-        else:
-            layout.append([widget.id])
-
-        widget.dashboard.layout = json.dumps(layout)
-        models.db.session.add(widget.dashboard)
         models.db.session.commit()
-        return {'widget': widget.to_dict(), 'layout': layout, 'new_row': new_row, 'version': dashboard.version}
+        return {'widget': widget.to_dict()}
 
 
 class WidgetResource(BaseResource):
@@ -92,12 +72,8 @@ def delete(self, widget_id):
         Remove a widget from a dashboard.
 
         :param number widget_id: ID of widget to remove
-
-        :>json array layout: New layout of dashboard this widget was removed from
-        :>json number version: Revision number of dashboard
         """
         widget = models.Widget.get_by_id_and_org(widget_id, self.current_org)
         require_object_modify_permission(widget.dashboard, self.current_user)
-        widget.delete()
+        models.db.session.delete(widget)
         models.db.session.commit()
-        return {'layout': widget.dashboard.layout, 'version': widget.dashboard.version}

redash/models.py

@@ -1295,8 +1295,7 @@ class Dashboard(ChangeTrackingMixin, TimestampMixin, BelongsToOrgMixin, db.Model
     name = Column(db.String(100))
     user_id = Column(db.Integer, db.ForeignKey("users.id"))
     user = db.relationship(User)
-    # TODO: The layout should dynamically be built from position and size information on each widget.
-    # Will require update in the frontend code to support this.
+    # layout is no longer used, but kept so we know how to render old dashboards.
     layout = Column(db.Text)
     dashboard_filters_enabled = Column(db.Boolean, default=False)
     is_archived = Column(db.Boolean, default=False, index=True)
@@ -1311,39 +1310,22 @@ class Dashboard(ChangeTrackingMixin, TimestampMixin, BelongsToOrgMixin, db.Model
     def to_dict(self, with_widgets=False, user=None):
         layout = json.loads(self.layout)
 
-        if with_widgets:
-            widget_list = Widget.query.filter(Widget.dashboard == self)
-
-            widgets = {}
+        widgets = []
 
-            for w in widget_list:
+        if with_widgets:
+            for w in self.widgets:
+                pass
                 if w.visualization_id is None:
-                    widgets[w.id] = w.to_dict()
+                    widgets.append(w.to_dict())
                 elif user and has_access(w.visualization.query_rel.groups, user, view_only):
-                    widgets[w.id] = w.to_dict()
+                    widgets.append(w.to_dict())
                 else:
-                    widgets[w.id] = project(w.to_dict(),
-                                            ('id', 'width', 'dashboard_id', 'options', 'created_at', 'updated_at'))
-                    widgets[w.id]['restricted'] = True
-
-            # The following is a workaround for cases when the widget object gets deleted without the dashboard layout
-            # updated. This happens for users with old databases that didn't have a foreign key relationship between
-            # visualizations and widgets.
-            # It's temporary until better solution is implemented (we probably should move the position information
-            # to the widget).
-            widgets_layout = []
-            for row in layout:
-                if not row:
-                    continue
-                new_row = []
-                for widget_id in row:
-                    widget = widgets.get(widget_id, None)
-                    if widget:
-                        new_row.append(widget)
-
-                widgets_layout.append(new_row)
+                    widget = project(w.to_dict(),
+                                    ('id', 'width', 'dashboard_id', 'options', 'created_at', 'updated_at'))
+                    widget['restricted'] = True
+                    widgets.append(widget)
         else:
-            widgets_layout = None
+            widgets = None
 
         return {
             'id': self.id,
@@ -1352,7 +1334,7 @@ def to_dict(self, with_widgets=False, user=None):
             'user_id': self.user_id,
             'layout': layout,
             'dashboard_filters_enabled': self.dashboard_filters_enabled,
-            'widgets': widgets_layout,
+            'widgets': widgets,
             'is_archived': self.is_archived,
             'is_draft': self.is_draft,
             'updated_at': self.updated_at,
@@ -1464,10 +1446,6 @@ class Widget(TimestampMixin, db.Model):
     options = Column(db.Text)
     dashboard_id = Column(db.Integer, db.ForeignKey("dashboards.id"), index=True)
 
-    # unused; kept for backward compatability:
-    type = Column(db.String(100), nullable=True)
-    query_id = Column(db.Integer, nullable=True)
-
     __tablename__ = 'widgets'
 
     def to_dict(self):
@@ -1486,15 +1464,6 @@ def to_dict(self):
 
         return d
 
-    def delete(self):
-        layout = json.loads(self.dashboard.layout)
-        layout = map(lambda row: filter(lambda w: w != self.id, row), layout)
-        layout = filter(lambda row: len(row) > 0, layout)
-        self.dashboard.layout = json.dumps(layout)
-
-        db.session.add(self.dashboard)
-        db.session.delete(self)
-
     def __unicode__(self):
         return u"%s" % self.id
 

tests/factories.py

@@ -118,7 +118,6 @@ def __call__(self):
                                      options='{}')
 
 widget_factory = ModelFactory(redash.models.Widget,
-                              type='chart',
                               width=1,
                               options='{}',
                               dashboard=dashboard_factory.create,

tests/handlers/test_dashboards.py

@@ -38,8 +38,8 @@ def test_get_dashboard_filters_unauthorized_widgets(self):
 
         rv = self.make_request('get', '/api/dashboards/{0}'.format(dashboard.slug))
         self.assertEquals(rv.status_code, 200)
-        self.assertTrue(rv.json['widgets'][0][1]['restricted'])
-        self.assertNotIn('restricted', rv.json['widgets'][0][0])
+        self.assertTrue(rv.json['widgets'][0]['restricted'])
+        self.assertNotIn('restricted', rv.json['widgets'][1])
 
     def test_get_non_existing_dashboard(self):
         rv = self.make_request('get', '/api/dashboards/not_existing')

tests/handlers/test_widgets.py

@@ -22,28 +22,6 @@ def test_create_widget(self):
         rv = self.create_widget(dashboard, vis)
         self.assertEquals(rv.status_code, 200)
 
-        dashboard = models.Dashboard.query.get(dashboard.id)
-        self.assertEquals(unicode(rv.json['layout']), dashboard.layout)
-
-        self.assertEquals(dashboard.widgets.count(), 1)
-        self.assertEquals(rv.json['layout'], [[rv.json['widget']['id']]])
-        self.assertEquals(rv.json['new_row'], True)
-
-        rv2 = self.create_widget(dashboard, vis)
-        self.assertEquals(dashboard.widgets.count(), 2)
-        self.assertEquals(rv2.json['layout'],
-                          [[rv.json['widget']['id'], rv2.json['widget']['id']]])
-        self.assertEquals(rv2.json['new_row'], False)
-
-        rv3 = self.create_widget(dashboard, vis)
-        self.assertEquals(rv3.json['new_row'], True)
-        rv4 = self.create_widget(dashboard, vis, width=2)
-        self.assertEquals(rv4.json['layout'],
-                          [[rv.json['widget']['id'], rv2.json['widget']['id']],
-                           [rv3.json['widget']['id']],
-                           [rv4.json['widget']['id']]])
-        self.assertEquals(rv4.json['new_row'], True)
-
     def test_wont_create_widget_for_visualization_you_dont_have_access_to(self):
         dashboard = self.factory.create_dashboard()
         vis = self.factory.create_visualization()
@@ -86,8 +64,3 @@ def test_delete_widget(self):
         self.assertEquals(rv.status_code, 200)
         dashboard = models.Dashboard.get_by_slug_and_org(widget.dashboard.slug, widget.dashboard.org)
         self.assertEquals(dashboard.widgets.count(), 0)
-        self.assertEquals(dashboard.layout, '[]')
-
-        # TODO: test how it updates the layout
-
-

tests/test_models.py

@@ -476,26 +476,6 @@ def test_records_additional_properties(self):
         self.assertDictEqual(event.additional_properties, additional_properties)
 
 
-class TestWidgetDeleteInstance(BaseTestCase):
-    def test_delete_removes_from_layout(self):
-        widget = self.factory.create_widget()
-        widget2 = self.factory.create_widget(dashboard=widget.dashboard)
-        db.session.flush()
-        widget.dashboard.layout = json.dumps([[widget.id, widget2.id]])
-        widget.delete()
-        self.assertEquals(json.dumps([[widget2.id]]), widget.dashboard.layout)
-
-    def test_delete_removes_empty_rows(self):
-        widget = self.factory.create_widget()
-        widget2 = self.factory.create_widget(dashboard=widget.dashboard)
-        db.session.flush()
-        widget.dashboard.layout = json.dumps([[widget.id, widget2.id]])
-        db.session.flush()
-        widget.delete()
-        widget2.delete()
-        self.assertEquals("[]", widget.dashboard.layout)
-
-
 def _set_up_dashboard_test(d):
     d.g1 = d.factory.create_group(name='First', permissions=['create', 'view'])
     d.g2 = d.factory.create_group(name='Second',  permissions=['create', 'view'])