(core) Preserve configured choices when converting between Choice and ChoiceList types.

Summary:
For conversions between Choice and ChoiceList, it makes more sense to preserve
the list of choices than to re-parse it from data.

Reported by Anais. Creating Choices from parsing ChoiceList cell values was
particularly poor, resulting in choices like "L,Foo,Bar".

Test Plan: Added a test case

Reviewers: paulfitz

Reviewed By: paulfitz

Differential Revision: https://phab.getgrist.com/D2819
This commit is contained in:
Dmitry S 2021-05-18 01:01:53 -04:00
parent 5f182841b9
commit 8c2f0307e5

View File

@ -80,31 +80,42 @@ export async function prepTransformColInfo(docModel: DocModel, origCol: ColumnRe
formula: "", // Will be filled in at the end. formula: "", // Will be filled in at the end.
}; };
const prevOptions = origCol.widgetOptionsJson.peek() || {};
switch (toType) { switch (toType) {
case 'Choice': { case 'Choice': {
// Set suggested choices. Limit to 100, since too many choices is more likely to cause if (Array.isArray(prevOptions.choices)) {
// trouble than desired behavior. For many choices, recommend using a Ref to helper table. // Use previous choices if they are set, e.g. if converting from ChoiceList
const columnData = tableData.getDistinctValues(origCol.colId(), 100); widgetOptions = {choices: prevOptions.choices};
if (columnData) { } else {
columnData.delete(""); // Set suggested choices. Limit to 100, since too many choices is more likely to cause
widgetOptions = {choices: Array.from(columnData, String)}; // trouble than desired behavior. For many choices, recommend using a Ref to helper table.
const columnData = tableData.getDistinctValues(origCol.colId(), 100);
if (columnData) {
columnData.delete("");
widgetOptions = {choices: Array.from(columnData, String)};
}
} }
break; break;
} }
case 'ChoiceList': { case 'ChoiceList': {
// Set suggested choices. This happens before the conversion to ChoiceList, so we do some if (Array.isArray(prevOptions.choices)) {
// light guessing for likely choices to suggest. // Use previous choices if they are set, e.g. if converting from ChoiceList
const choices = new Set<string>(); widgetOptions = {choices: prevOptions.choices};
for (let value of tableData.getColValues(origCol.colId()) || []) { } else {
value = String(value).trim(); // Set suggested choices. This happens before the conversion to ChoiceList, so we do some
const tags: string[] = (value.startsWith('[') && gutil.safeJsonParse(value, null)) || value.split(","); // light guessing for likely choices to suggest.
for (const tag of tags) { const choices = new Set<string>();
choices.add(tag.trim()); for (let value of tableData.getColValues(origCol.colId()) || []) {
if (choices.size > 100) { break; } // Don't suggest excessively many choices. value = String(value).trim();
const tags: string[] = (value.startsWith('[') && gutil.safeJsonParse(value, null)) || value.split(",");
for (const tag of tags) {
choices.add(tag.trim());
if (choices.size > 100) { break; } // Don't suggest excessively many choices.
}
} }
choices.delete("");
widgetOptions = {choices: Array.from(choices)};
} }
choices.delete("");
widgetOptions = {choices: Array.from(choices)};
break; break;
} }
case 'Ref': { case 'Ref': {
@ -170,7 +181,7 @@ export function getDefaultFormula(
`$${colId}`; `$${colId}`;
if (origCol.type.peek() === 'ChoiceList') { if (origCol.type.peek() === 'ChoiceList') {
origValFormula = `grist.ChoiceList.toString($${colId})` origValFormula = `grist.ChoiceList.toString($${colId})`;
} }
const toTypePure: string = gristTypes.extractTypeFromColType(newType); const toTypePure: string = gristTypes.extractTypeFromColType(newType);