From 6aadc6ec1373d5a6bb46c1485f744ac806ea9729 Mon Sep 17 00:00:00 2001
From: Maxime Beauchemin <maximebeauchemin@gmail.com>
Date: Fri, 9 Sep 2016 16:39:11 -0700
Subject: [PATCH] Simplifying the flow to add a table (#1068)

When specifying a table reference that can not be found, the system used
to still create the object, which would result in confusion and bad
error messages down the line. Now it will fail and not create the
object.

I also removed fields that are not necessary to worry about when
initially creating the table.
---
 caravel/models.py | 13 +++++++------
 caravel/views.py  | 35 ++++++++++++++++++++++-------------
 2 files changed, 29 insertions(+), 19 deletions(-)

diff --git a/caravel/models.py b/caravel/models.py
index 3048c7288b99f..3b130017dfc49 100644
--- a/caravel/models.py
+++ b/caravel/models.py
@@ -889,16 +889,17 @@ def query(  # sqla
         return QueryResult(
             df=df, duration=datetime.now() - qry_start_dttm, query=sql)
 
+    def get_sqla_table_object(self):
+        return self.database.get_table(self.table_name, schema=self.schema)
+
     def fetch_metadata(self):
         """Fetches the metadata for the table and merges it in"""
         try:
-            table = self.database.get_table(self.table_name, schema=self.schema)
-        except Exception as e:
-            flasher(str(e))
-            flasher(
+            table = self.get_sqla_table_object()
+        except Exception:
+            raise Exception(
                 "Table doesn't seem to exist in the specified database, "
-                "couldn't fetch column information", "danger")
-            return
+                "couldn't fetch column information")
 
         TC = TableColumn  # noqa shortcut to class
         M = SqlMetric  # noqa
diff --git a/caravel/views.py b/caravel/views.py
index 81d1ddfb3e9a5..eb010a4aa190b 100755
--- a/caravel/views.py
+++ b/caravel/views.py
@@ -526,9 +526,7 @@ class TableModelView(CaravelModelView, DeleteMixin):  # noqa
         'changed_by_', 'changed_on_']
     order_columns = [
         'table_link', 'database', 'is_featured', 'changed_on_']
-    add_columns = [
-        'table_name', 'database', 'schema',
-        'default_endpoint', 'offset', 'cache_timeout']
+    add_columns = ['table_name', 'database', 'schema']
     edit_columns = [
         'table_name', 'sql', 'is_featured', 'database', 'schema',
         'description', 'owner',
@@ -536,14 +534,16 @@ class TableModelView(CaravelModelView, DeleteMixin):  # noqa
     related_views = [TableColumnInlineView, SqlMetricInlineView]
     base_order = ('changed_on', 'desc')
     description_columns = {
-        'offset': "Timezone offset (in hours) for this datasource",
-        'schema': (
+        'offset': _("Timezone offset (in hours) for this datasource"),
+        'table_name': _(
+            "Name of the table that exists in the source database"),
+        'schema': _(
             "Schema, as used only in some databases like Postgres, Redshift "
             "and DB2"),
         'description': Markup(
             "Supports <a href='https://daringfireball.net/projects/markdown/'>"
             "markdown</a>"),
-        'sql': (
+        'sql': _(
             "This fields acts a Caravel view, meaning that Caravel will "
             "run a query against this string as a subquery."
         ),
@@ -561,17 +561,26 @@ class TableModelView(CaravelModelView, DeleteMixin):  # noqa
         'cache_timeout': _("Cache Timeout"),
     }
 
-    def post_add(self, table):
-        table_name = table.table_name
+    def pre_add(self, table):
+        # Fail before adding if the table can't be found
         try:
-            table.fetch_metadata()
+            table.get_sqla_table_object()
         except Exception as e:
             logging.exception(e)
-            flash(
-                "Table [{}] doesn't seem to exist, "
-                "couldn't fetch metadata".format(table_name),
-                "danger")
+            raise Exception(
+                "Table [{}] could not be found, "
+                "please double check your "
+                "database connection, schema, and "
+                "table name".format(table.table_name))
+
+    def post_add(self, table):
+        table.fetch_metadata()
         utils.merge_perm(sm, 'datasource_access', table.perm)
+        flash(_(
+            "The table was created. As part of this two phase configuration "
+            "process, you should now click the edit button by "
+            "the new table to configure it."),
+            "info")
 
     def post_update(self, table):
         self.post_add(table)