(core) Raw renames

Summary:
A new way for renaming tables.

  - There is a new popup to rename section (where you can also rename the table)
  - Renaming/Deleting page doesn't modify/delete the table.
  - Renaming table can rename a page if the names match (and the page contains a section with that table).
  - User can rename table in Raw Data UI in two ways - either on the listing or by using the section name popup
  - As before, there is no way to change tableId - it is derived from a table name.
  - When the section name is empty the table name is shown instead.
  - White space for section name is allowed (to discuss) - so the user can just paste '   '.
  - Empty name for a page is not allowed (but white space is).
  - Some bugs related to deleting tables with attached summary tables (and with undoing this operation) were fixed (but not all of them yet).

Test Plan: Updated tests.

Reviewers: georgegevoian

Reviewed By: georgegevoian

Subscribers: georgegevoian

Differential Revision: https://phab.getgrist.com/D3360

sandbox/grist/test_useractions.py

@@ -454,14 +454,16 @@ class TestUserActions(test_engine.EngineTestCase):
       [ 4,      'Table1',     3],
     ])
 
-    # Update the names in a few views, and ensure that primary ones cause tables to get renamed.
+    # Update the names in a few views, and ensure that primary ones won't cause tables to
+    # get renamed.
     self.apply_user_action(['BulkUpdateRecord', '_grist_Views', [2,3,4],
                             {'name': ['A', 'B', 'C']}])
+
     self.assertTableData('_grist_Tables', cols="subset", data=[
       [ 'id', 'tableId',  'primaryViewId'  ],
       [ 1,    'Schools',                1],
       [ 2,    'GristSummary_7_Schools', 0],
-      [ 3,    'C',                      4],
+      [ 3,    'Table1',                      4],
     ])
     self.assertTableData('_grist_Views', cols="subset", data=[
       [ 'id',   'name',   'primaryViewTable'  ],
@@ -471,6 +473,97 @@ class TestUserActions(test_engine.EngineTestCase):
       [ 4,      'C',        3]
     ])
 
+    # Now rename a table (by raw view section) and make sure that a view with the same name
+    # was renamed
+    self.apply_user_action(['UpdateRecord', '_grist_Views_section', 2,
+                            {'title': 'Bars'}])
+
+    self.assertTableData('_grist_Tables', cols="subset", data=[
+      ['id', 'tableId'],
+      [1, 'Bars', 1],
+      [2, 'GristSummary_4_Bars', 0],
+      [3, 'Table1', 4],
+    ])
+    self.assertTableData('_grist_Views', cols="subset", data=[
+      ['id', 'name'],
+      [1, 'Bars'],
+      [2, 'A'],
+      [3, 'B'],
+      [4, 'C']
+    ])
+
+    # Now rename tables so that two tables will have same names, to test if only the view
+    # with a page will be renamed.
+    self.apply_user_action(['UpdateRecord', '_grist_Views_section', 2,
+                            {'title': 'A'}])
+
+    self.assertTableData('_grist_Tables', cols="subset", data=[
+      ['id', 'tableId'],
+      [1, 'A', 1],
+      [2, 'GristSummary_1_A', 0],
+      [3, 'Table1', 4],
+    ])
+    self.assertTableData('_grist_Views', cols="subset", data=[
+      ['id', 'name'],
+      [1, 'A'],
+      [2, 'A'],
+      [3, 'B'],
+      [4, 'C']
+    ])
+
+    self.apply_user_action(['UpdateRecord', '_grist_Views_section', 2,
+                            {'title': 'Z'}])
+
+    self.assertTableData('_grist_Tables', cols="subset", data=[
+      ['id', 'tableId'],
+      [1, 'Z', 1],
+      [2, 'GristSummary_1_Z', 0],
+      [3, 'Table1', 4],
+    ])
+    self.assertTableData('_grist_Views', cols="subset", data=[
+      ['id', 'name'],
+      [1, 'Z'],
+      [2, 'Z'],
+      [3, 'B'],
+      [4, 'C']
+    ])
+
+    # Add new table, with a view with the same name (Z) and make sure it won't be renamed
+    self.apply_user_action(['AddTable', 'Stations', [
+      {'id': 'city', 'type': 'Text'},
+    ]])
+
+    # Replacing only a page name (though primary)
+    self.apply_user_action(['UpdateRecord', '_grist_Views', 5, {'name': 'Z'}])
+    self.assertTableData('_grist_Views', cols="subset", data=[
+      ['id', 'name'],
+      [1, 'Z'],
+      [2, 'Z'],
+      [3, 'B'],
+      [4, 'C'],
+      [5, 'Z']
+    ])
+
+    # Rename table Z to Schools. Primary view for Stations (Z) should not be renamed.
+    self.apply_user_action(['UpdateRecord', '_grist_Views_section', 2,
+                            {'title': 'Schools'}])
+
+    self.assertTableData('_grist_Tables', cols="subset", data=[
+      ['id', 'tableId'],
+      [1, 'Schools'],
+      [2, 'GristSummary_7_Schools'],
+      [3, 'Table1'],
+      [4, 'Stations'],
+    ])
+    self.assertTableData('_grist_Views', cols="subset", data=[
+      ['id', 'name'],
+      [1, 'Schools'],
+      [2, 'Schools'],
+      [3, 'B'],
+      [4, 'C'],
+      [5, 'Z']
+    ])
+
   #----------------------------------------------------------------------
 
   def test_section_removes(self):
@@ -531,7 +624,8 @@ class TestUserActions(test_engine.EngineTestCase):
     self.assertEqual(count_calls[0], 0)
 
     # Do a schema action to ensure it gets called: this causes a table rename.
-    self.apply_user_action(['UpdateRecord', '_grist_Views', 4, {'name': 'C'}])
+    # 7 is id of raw view section for the Tabl1 table
+    self.apply_user_action(['UpdateRecord', '_grist_Views_section', 7, {'title': 'C'}])
     self.assertEqual(count_calls[0], 1)
 
     self.assertTableData('_grist_Tables', cols="subset", data=[

sandbox/grist/useractions.py

@@ -681,22 +681,33 @@ class UserActions(object):
     make_acl_updates()
 
 
-  @override_action('BulkUpdateRecord', '_grist_Views')
-  def _updateViewRecords(self, table_id, row_ids, col_values):
-    # If we change a view's name, and that view is a primary view, change
-    # its table's tableId as well.
-    if 'name' in col_values:
+  @override_action('BulkUpdateRecord', '_grist_Views_section')
+  def _updateViewSections(self, table_id, row_ids, col_values):
+    # If we change a raw section name, rename also the table. Table name is a title of the RAW
+    # section. TableId is derived from the tableName (or is autogenerated if the tableName is blank)
+    if 'title' in col_values:
       rename_table_recs = []
       rename_names = []
-      rename_section_recs = []
       for i, rec, values in self._bulk_action_iter(table_id, row_ids, col_values):
-        table = rec.primaryViewTable
-        if table:
-          rename_table_recs.append(table)
-          rename_section_recs.append(table.rawViewSectionRef)
-          rename_names.append(values['name'])
+        if rec.isRaw:
+          rename_table_recs.append(rec.tableRef)
+          rename_names.append(values['title'])
+
+          # Renaming a table may sometimes rename pages: For any pages whose name matches
+          # the table name, rename those page to match (provided it contains a section with this
+          # table).
+
+          # Get all sections with this table
+          sections = self._docmodel.view_sections.lookupRecords(tableRef=rec.tableRef)
+          # Get the views of those sections
+          views = {s.parentId for s in sections if s.parentId is not None and s.parentId.id != 0}
+          # Filter them by the old table name (which may be empty - than by tableId)
+          related_views = [v for v in views if v.name == (rec.title or rec.tableRef.tableId)]
+          # Update the views immediately
+          if related_views:
+            self._docmodel.update(related_views, name=[values['title']] * len(related_views))
+
       self._docmodel.update(rename_table_recs, tableId=rename_names)
-      self._docmodel.update(rename_section_recs, title=rename_names)
 
     self.doBulkUpdateRecord(table_id, row_ids, col_values)
 
@@ -972,7 +983,7 @@ class UserActions(object):
     remove_table_recs.extend(st for t in remove_table_recs for st in t.summaryTables)
 
     # If other tables have columns referring to this table, remove them.
-    self._docmodel.remove(self._collect_back_references(remove_table_recs))
+    self.doRemoveColumns(self._collect_back_references(remove_table_recs))
 
     # Remove all view sections and fields for all tables being removed.
     # Bypass the check for raw data view sections.
@@ -1014,6 +1025,9 @@ class UserActions(object):
     if any(c.summarySourceCol for c in col_recs):
       raise ValueError("RemoveColumn: cannot remove a group-by column from a summary table")
 
+    self.doRemoveColumns(col_recs)
+
+  def doRemoveColumns(self, col_recs):
     # We need to remove group-by columns based on the columns being removed. To ensure we don't end
     # up with multiple summary tables with the same breakdown, we'll implement this by using
     # UpdateSummaryViewSection() on all the affected sections.
@@ -1030,7 +1044,7 @@ class UserActions(object):
 
     # Remove this column from any sort specs to which it belongs.
     parent_sections = {section for c in col_recs for section in c.parentId.viewSections}
-    removed_col_refs = set(row_ids)
+    removed_col_refs = set((c.id for c in col_recs))
     re_sort_sections = []
     re_sort_specs = []
     for section in parent_sections:
@@ -1081,7 +1095,7 @@ class UserActions(object):
 
     # Remove metadata records, but prepare schema actions before the metadata is cleared.
     removals = [actions.RemoveColumn(c.parentId.tableId, c.colId) for c in all_removals]
-    self.doBulkRemoveRecord(table_id, [int(c) for c in all_removals])
+    self.doBulkRemoveRecord('_grist_Tables_column', [int(c) for c in all_removals])
 
     # Finally do the schema actions to remove the columns.
     for action in removals: