diff --git a/sandbox/grist/column.py b/sandbox/grist/column.py index d9625dff..d116ffd0 100644 --- a/sandbox/grist/column.py +++ b/sandbox/grist/column.py @@ -524,11 +524,24 @@ class ReferenceListColumn(BaseReferenceColumn): Accessing them yields RecordSets. """ def set(self, row_id, value): - if isinstance(value, six.string_types) and value.startswith(u'['): + if isinstance(value, six.string_types): + # This is second part of a "hack" we have to do when we rename tables. During + # the rename, we briefly change all Ref columns to Int columns (to lose the table + # part), and then back to Ref columns. The values during this change are stored + # as serialized strings, which we expect to understand when the column is back to + # being a Ref column. We can either end up with a list of ints, or a RecordList + # serialized as a string. + # TODO: explain why we need to do this and why we have chosen the Int column try: - value = json.loads(value) + # If it's a string that looks like JSON, try to parse it as such. + if value.startswith('['): + value = json.loads(value) + else: + # Else try to parse it as a RecordList + value = objtypes.RecordList.from_repr(value) except Exception: pass + super(ReferenceListColumn, self).set(row_id, value) def _update_references(self, row_id, old_list, new_list): diff --git a/sandbox/grist/objtypes.py b/sandbox/grist/objtypes.py index d19894e5..1c298f6f 100644 --- a/sandbox/grist/objtypes.py +++ b/sandbox/grist/objtypes.py @@ -361,6 +361,20 @@ class CellError(Exception): class RecordList(list): + """ + A static method to recreate a RecordList from the output of __repr__. + It only restores the row_ids. The group_by and sort_by attributes are not restored. + """ + @staticmethod + def from_repr(repr_str): + if not repr_str.startswith('RecordList(['): + raise ValueError("Invalid RecordList representation") + # This is a string representation of a RecordList, which we can parse. + # > RecordList([1,2,3], group_by=%r, sort_by=%r) + # Match only rows, as group_by and sort_by are not used and can be stale. + numbers = repr_str.split('[')[1].split(']')[0].split(',') + return RecordList([int(v) for v in numbers]) + """ Just like list but allows setting custom attributes, which we use for remembering _group_by and _sort_by attributes when storing RecordSet as usertypes.ReferenceList type. @@ -375,7 +389,6 @@ class RecordList(list): list.__repr__(self), self._group_by, self._sort_by) - # We don't currently have a good way to convert an incoming marshalled record to a proper Record # object for an appropriate table. We don't expect incoming marshalled records at all, but if such # a thing happens, we'll construct this RecordStub. diff --git a/sandbox/grist/usertypes.py b/sandbox/grist/usertypes.py index 28f60a56..d617b927 100644 --- a/sandbox/grist/usertypes.py +++ b/sandbox/grist/usertypes.py @@ -431,12 +431,22 @@ class ReferenceList(BaseColumnType): def do_convert(self, value): if isinstance(value, six.string_types): - # If it's a string that looks like JSON, try to parse it as such. - if value.startswith('['): - try: + # This is second part of a "hack" we have to do when we rename tables. During + # the rename, we briefly change all Ref columns to Int columns (to lose the table + # part), and then back to Ref columns. The values during this change are stored + # as serialized strings, which we expect to understand when the column is back to + # being a Ref column. We can either end up with a list of ints, or a RecordList + # serialized as a string. + # TODO: explain why we need to do this and why we have chosen the Int column + try: + # If it's a string that looks like JSON, try to parse it as such. + if value.startswith('['): value = json.loads(value) - except Exception: - pass + else: + # Else try to parse it as a RecordList + value = objtypes.RecordList.from_repr(value) + except Exception: + pass if isinstance(value, RecordSet): assert value._table.table_id == self.table_id diff --git a/test/nbrowser/ReferenceList.ts b/test/nbrowser/ReferenceList.ts index 16e1f2f1..7757b8e8 100644 --- a/test/nbrowser/ReferenceList.ts +++ b/test/nbrowser/ReferenceList.ts @@ -13,7 +13,40 @@ describe('ReferenceList', function() { }); describe('other', function() { - it('allows to delete document with self reference', async function() { + it('fix: doesnt break when table is renamed', async function() { + // There was a bug in this scenario: + // 1. Create a Ref column that targets itself + // 2. Fill it up + // 3. Change it to RefList + // 4. Rename the table + // Previously, this would cause an error, cells were displaying Errors instead of values. + + // Create a table with a Ref column that targets itself. + await session.tempNewDoc(cleanup); + await gu.sendActions([ + ['ModifyColumn', 'Table1', 'B', {type: 'Ref:Table1'}], + ['AddRecord', 'Table1', null, {A: 'a', B: 1}], + ['AddRecord', 'Table1', null, {A: 'b', B: 2}], + ['AddRecord', 'Table1', null, {A: 'c', B: 3}], + ]); + await gu.openColumnPanel(); + await gu.getCell('B', 1).doClick(); + await gu.setRefShowColumn('A'); + + // Now convert it to RefList. + await gu.setType('Reference List', {apply: true}); + + // Make sure we see the values. + assert.deepEqual(await gu.getVisibleGridCells('B', [1, 2, 3]), ['a', 'b', 'c']); + + // Rename the table using widget in the section. + await gu.renameTable('Table1', 'Table2'); + + // Make sure we still see the values. + assert.deepEqual(await gu.getVisibleGridCells('B', [1, 2, 3]), ['a', 'b', 'c']); + }); + + it('fix: allows to delete document with self reference', async function() { const docId = await session.tempNewDoc(cleanup); await gu.sendActions([ ['AddEmptyTable', 'Table2'], @@ -39,7 +72,7 @@ describe('ReferenceList', function() { before(async function() { await session.tempDoc(cleanup, 'Favorite_Films.grist'); - await gu.toggleSidePanel('right'); + await gu.toggleSidePanel('right', 'open'); await driver.find(".test-right-tab-pagewidget").click(); await driver.find('.test-config-data').click(); });