From f2f06e6890ca4c4471b54ae3adb835a3ee7d77cb Mon Sep 17 00:00:00 2001
From: Jan Pieter Waagmeester <jieter@jieter.nl>
Date: Tue, 13 Jun 2017 21:25:45 +0200
Subject: [PATCH] Defer reordering columns until TableBase.__init__ to
 correctly expand extra_columns too. (fixes #450)

---
 django_tables2/tables.py                      | 26 ++++++++-----------
 django_tables2/utils.py                       | 10 ++++++-
 .../test_dynamically_add_show_hide_columns.py | 19 ++++++++++++++
 3 files changed, 39 insertions(+), 16 deletions(-)

diff --git a/django_tables2/tables.py b/django_tables2/tables.py
index 6c48b9af..4617bbd0 100644
--- a/django_tables2/tables.py
+++ b/django_tables2/tables.py
@@ -306,15 +306,6 @@ def __new__(mcs, name, bases, attrs):
             if attr_name in base_columns:
                 base_columns.pop(attr_name)
 
-        # Reorder the columns based on explicit sequence
-        if opts.sequence:
-            opts.sequence.expand(base_columns.keys())
-            # Table's sequence defaults to sequence declared in Meta, if the
-            # column is not excluded
-            base_columns = OrderedDict((
-                (x, base_columns[x]) for x in opts.sequence if x in base_columns
-            ))
-
         # Set localize on columns
         for col_name in base_columns.keys():
             localize_column = None
@@ -484,16 +475,21 @@ def __init__(self, data, order_by=None, orderable=None, empty_text=None,
         # 2. sequence declared in ``Meta``
         # 3. sequence defaults to '...'
         if sequence is not None:
-            self._sequence = Sequence(sequence)
-            self._sequence.expand(base_columns.keys())
+            sequence = Sequence(sequence)
         elif self._meta.sequence:
-            self._sequence = self._meta.sequence
+            sequence = self._meta.sequence
         else:
             if self._meta.fields is not None:
-                self._sequence = Sequence(tuple(self._meta.fields) + ('...', ))
+                sequence = Sequence(tuple(self._meta.fields) + ('...', ))
             else:
-                self._sequence = Sequence(('...', ))
-            self._sequence.expand(base_columns.keys())
+                sequence = Sequence(('...', ))
+        self._sequence = sequence.expand(base_columns.keys())
+
+        # reorder columns based on sequence.
+        base_columns = OrderedDict((
+            (x, base_columns[x]) for x in sequence if x in base_columns
+        ))
+
         self.columns = columns.BoundColumns(self, base_columns)
         # `None` value for order_by means no order is specified. This means we
         # `shouldn't touch our data's ordering in any way. *However*
diff --git a/django_tables2/utils.py b/django_tables2/utils.py
index ed444773..8067900d 100644
--- a/django_tables2/utils.py
+++ b/django_tables2/utils.py
@@ -26,7 +26,13 @@ def expand(self, columns):
         Expands the ``'...'`` item in the sequence into the appropriate column
         names that should be placed there.
 
-        :raises: `ValueError` if the sequence is invalid for the columns.
+        arguments:
+            columns (list): list of column names.
+        returns:
+            The current instance.
+
+        raises:
+            `ValueError` if the sequence is invalid for the columns.
         '''
         ellipses = self.count("...")
         if ellipses > 1:
@@ -49,6 +55,8 @@ def expand(self, columns):
                 columns.pop(columns.index(name))
         self[:] = chain(head, columns, tail)
 
+        return self
+
 
 class OrderBy(str):
     '''
diff --git a/tests/test_dynamically_add_show_hide_columns.py b/tests/test_dynamically_add_show_hide_columns.py
index b3f3f50b..7200ed85 100644
--- a/tests/test_dynamically_add_show_hide_columns.py
+++ b/tests/test_dynamically_add_show_hide_columns.py
@@ -41,6 +41,25 @@ class MyTable(tables.Table):
     assert list(MyTable(data).columns.columns.keys()) == ['name']
 
 
+def test_dynamically_add_column_with_sequence():
+    class MyTable(tables.Table):
+        name = tables.Column()
+
+        class Meta:
+            sequence = ('...', 'name')
+
+    assert list(MyTable(data, extra_columns=[
+        ('country', tables.Column())
+    ]).columns.columns.keys()) == ['country', 'name']
+
+    # override sequence with an argument.
+    assert list(MyTable(
+        data,
+        extra_columns=[('country', tables.Column())],
+        sequence=('name', '...')
+    ).columns.columns.keys()) == ['name', 'country']
+
+
 @pytest.mark.django_db
 def test_dynamically_hide_columns():
     class MyTable(tables.Table):