Improve and use `guessFieldType`.

Enforce restrictions on `itemColumns`. Tweak UX for the dropdowns and added checkbox for no columns row. Still has the two modal bugs, referenced in the code comments.
2022-03-09 21:00:42 +00:00 · 2022-03-09 21:00:42 +00:00 · 9235543838
parent 0b03fd0990
commit 9235543838
3 changed files with 146 additions and 96 deletions
--- a/packages/app/src/elements/import-dialog.ts
+++ b/packages/app/src/elements/import-dialog.ts
@ -29,8 +29,8 @@ export class ImportDialog extends Dialog<File, void> {
    @state()
    private _itemColumns: imp.ImportCSVColumn[] = [];

-    @state()
-    private _csvHasDataOnFirstRow: boolean = false;
+    @query("#csvHasDataOnFirstRowCheckbox")
+    private _csvHasDataOnFirstRowCheckbox: HTMLInputElement;

    @query("#formatSelect")
    private _formatSelect: Select<string>;
@ -59,7 +59,10 @@ export class ImportDialog extends Dialog<File, void> {
        `,
    ];

+    // TODO: Fix dialog reset after closed (when re-opened)
+
    renderContent() {
+        // TODO: Fix wrong first selected value for selects
        if (this._formatSelect?.value === imp.CSV.value) {
            if (
                this._nameColumnSelect &&
@ -94,64 +97,8 @@ export class ImportDialog extends Dialog<File, void> {
                ></pl-select>

                <div class="small padded" ?hidden=${this._formatSelect && this._formatSelect.value !== imp.CSV.value}>
-                    ${$l("Choose the field name and type for each column below. If you are having trouble,")}
-                    <a href="#" @click=${this._downloadCSVSampleFile}> ${$l("Download Sample File")} </a>
-                </div>
-
-                <div
-                    class="vertical spacing layout"
-                    ?hidden=${this._formatSelect && this._formatSelect.value !== imp.CSV.value}
-                >
-                    <pl-select
-                        id=${"nameColumnSelect"}
-                        .label=${$l("Name Column")}
-                        .options=${this._itemColumns.map((itemColumn, itemColumnIndex) => ({
-                            label: `${itemColumn.displayName} (${$l("Column {0}", itemColumnIndex.toString())})`,
-                            value: itemColumnIndex,
-                        }))}
-                        .selectedIndex=${this._nameColumnSelect?.selectedIndex}
-                        @change=${() => {
-                            const currentNameColumnIndex = this._itemColumns.findIndex(
-                                (itemColumn) => itemColumn.type === "name"
-                            );
-                            const nameColumnIndex = this._nameColumnSelect?.value || 0;
-                            this._itemColumns[nameColumnIndex].type = "name";
-
-                            if (currentNameColumnIndex !== -1) {
-                                this._itemColumns[currentNameColumnIndex].type = FieldType.Text;
-                            }
-
-                            this._parseData();
-                        }}
-                    ></pl-select>
-
-                    <pl-select
-                        id=${"tagsColumnSelect"}
-                        .label=${$l("Tags Column")}
-                        .options=${[
-                            { label: $l("None"), value: -1 },
-                            ...this._itemColumns.map((itemColumn, itemColumnIndex) => ({
-                                label: `${itemColumn.displayName} (${$l("Column {0}", itemColumnIndex.toString())})`,
-                                value: itemColumnIndex,
-                            })),
-                        ]}
-                        .selectedIndex=${this._tagsColumnSelect?.selectedIndex}
-                        @change=${() => {
-                            const currentTagsColumnIndex = this._itemColumns.findIndex(
-                                (itemColumn) => itemColumn.type === "tags"
-                            );
-                            const tagsColumnIndex = this._tagsColumnSelect?.value || 0;
-                            this._itemColumns[tagsColumnIndex].type = "tags";
-
-                            if (currentTagsColumnIndex !== -1) {
-                                this._itemColumns[currentTagsColumnIndex].type = FieldType.Text;
-                            }
-
-                            this._parseData();
-                        }}
-                    ></pl-select>
-
-                    <!-- TODO: Add checkbox/toggle for this._csvHasDataOnFirstRow -->
+                    ${$l("Choose the correct column names and types for each column below. If you are having trouble,")}
+                    <a href="#" @click=${this._downloadCSVSampleFile}> ${$l("Download the Sample File")} </a>.
                </div>

                <pl-scroller
@ -159,6 +106,67 @@ export class ImportDialog extends Dialog<File, void> {
                    ?hidden=${this._formatSelect && this._formatSelect.value !== imp.CSV.value}
                >
                    <ul class="vertical spacing layout">
+                        <div class="padded">
+                            <label>
+                                <input type="checkbox" id="csvHasDataOnFirstRowCheckbox" />
+                                ${$l("Check if the CSV file has data on the first row (instead of the column names)")}
+                            </label>
+                        </div>
+
+                        <pl-select
+                            id=${"nameColumnSelect"}
+                            .label=${$l("Name Column")}
+                            .options=${this._itemColumns.map((itemColumn, itemColumnIndex) => ({
+                                label: `${itemColumn.displayName} (${$l("Column {0}", itemColumnIndex.toString())})`,
+                                value: itemColumnIndex,
+                            }))}
+                            .selectedIndex=${this._nameColumnSelect?.selectedIndex}
+                            @change=${() => {
+                                const currentNameColumnIndex = this._itemColumns.findIndex(
+                                    (itemColumn) => itemColumn.type === "name"
+                                );
+                                const nameColumnIndex = this._nameColumnSelect?.value || 0;
+                                this._itemColumns[nameColumnIndex].type = "name";
+
+                                if (currentNameColumnIndex !== -1) {
+                                    this._itemColumns[currentNameColumnIndex].type = FieldType.Text;
+                                }
+
+                                this._parseData();
+                            }}
+                        ></pl-select>
+
+                        <pl-select
+                            id=${"tagsColumnSelect"}
+                            .label=${$l("Tags Column")}
+                            .options=${[
+                                { label: $l("None"), value: -1 },
+                                ...this._itemColumns.map((itemColumn, itemColumnIndex) => ({
+                                    label: `${itemColumn.displayName} (${$l(
+                                        "Column {0}",
+                                        itemColumnIndex.toString()
+                                    )})`,
+                                    value: itemColumnIndex,
+                                })),
+                            ]}
+                            .selectedIndex=${this._tagsColumnSelect?.selectedIndex}
+                            @change=${() => {
+                                const currentTagsColumnIndex = this._itemColumns.findIndex(
+                                    (itemColumn) => itemColumn.type === "tags"
+                                );
+                                const tagsColumnIndex = this._tagsColumnSelect?.value || 0;
+                                this._itemColumns[tagsColumnIndex].type = "tags";
+
+                                if (currentTagsColumnIndex !== -1) {
+                                    this._itemColumns[currentTagsColumnIndex].type = FieldType.Text;
+                                }
+
+                                this._parseData();
+                            }}
+                        ></pl-select>
+
+                        <div class="spacer"></div>
+
                        ${this._itemColumns.map(
                            (itemColumn, itemColumnIndex) => html`
                                <li
@ -280,7 +288,7 @@ Github,"work,coding",https://github.com,john.doe@gmail.com,129lskdf93`)
                this._items = await imp.asLastPass(file);
                break;
            case imp.CSV.value:
-                const result = await imp.asCSV(file, this._itemColumns, this._csvHasDataOnFirstRow);
+                const result = await imp.asCSV(file, this._itemColumns, this._csvHasDataOnFirstRowCheckbox.checked);
                this._items = result.items;
                this._itemColumns = result.itemColumns;
                break;
--- a/packages/app/src/lib/import.ts
+++ b/packages/app/src/lib/import.ts
@ -1,7 +1,7 @@
 import { unmarshal, bytesToString } from "@padloc/core/src/encoding";
 import { PBES2Container } from "@padloc/core/src/container";
 import { validateLegacyContainer, parseLegacyContainer } from "@padloc/core/src/legacy";
-import { VaultItem, Field, createVaultItem, FieldType, FIELD_DEFS } from "@padloc/core/src/item";
+import { VaultItem, Field, createVaultItem, FieldType, guessFieldType } from "@padloc/core/src/item";
 import { Err, ErrorCode } from "@padloc/core/src/error";
 import { uuid, capitalize } from "@padloc/core/src/util";
 import { translate as $l } from "@padloc/locale/src/translate";
@ -122,15 +122,20 @@ export async function asCSV(
    }
    const rows = parsed.data as string[][];

-    const columnNames = rows.length > 0 ? rows[0].map((column) => column.toLowerCase()) : [];
+    if (rows.length === 0) {
+        throw new Err(ErrorCode.INVALID_CSV, "No rows found in .csv file.");
+    }
+
+    const columnNames = rows[0].map((column) => column.toLowerCase());

    const itemColumns =
        mappedItemColumns.length > 0
            ? mappedItemColumns
            : columnNames.map((columnName, columnIndex) => {
-                  // TODO: Use guessFieldType instead, after improving it
-                  let type = (Object.keys(FIELD_DEFS).filter((fieldType) => columnName.includes(fieldType))[0] ||
-                      FieldType.Text) as ImportCSVColumn["type"];
+                  let type = guessFieldType({
+                      name: columnName,
+                      value: rows[1] ? rows[1][columnIndex] : "",
+                  }) as ImportCSVColumn["type"];

                  const lowerCaseColumnName = columnName.toLocaleLowerCase();

@ -160,7 +165,29 @@ export async function asCSV(
                  };
              });

-    // TODO: Prevent itemColumns from having more than one "name" or "tags" (force those to "text")
+    // Prevent itemColumns from having more than one "name" or "tags"
+    let hasNameColumn = false;
+    let hasTagsColumn = false;
+    itemColumns.forEach((itemColumn) => {
+        if (itemColumn.type === "name") {
+            if (!hasNameColumn) {
+                hasNameColumn = true;
+            } else {
+                itemColumn.type = FieldType.Text;
+            }
+        } else if (itemColumn.type === "tags") {
+            if (!hasTagsColumn) {
+                hasTagsColumn = true;
+            } else {
+                itemColumn.type = FieldType.Text;
+            }
+        }
+    });
+
+    // Ensure there's at least one nameColumn
+    if (!hasNameColumn) {
+        itemColumns[0].type = "name";
+    }

    const items = await fromTable(rows, itemColumns, dataOnFirstRow);

--- a/packages/core/src/item.ts
+++ b/packages/core/src/item.ts
@ -32,8 +32,8 @@ export enum FieldType {
 export interface FieldDef {
    /** content type */
    type: FieldType;
-    /** regular expression describing pattern of field contents */
-    pattern: string;
+    /** regular expression describing pattern of field contents (not used for validation) */
+    pattern: RegExp;
    /** whether the field should be masked when displayed */
    mask: boolean;
    /** whether the field value can have multiple lines */
@ -52,7 +52,7 @@ export interface FieldDef {
 export const FIELD_DEFS: { [t in FieldType]: FieldDef } = {
    [FieldType.Username]: {
        type: FieldType.Username,
-        pattern: ".*",
+        pattern: /.*/,
        mask: false,
        multiline: false,
        icon: "user",
@ -62,7 +62,7 @@ export const FIELD_DEFS: { [t in FieldType]: FieldDef } = {
    },
    [FieldType.Password]: {
        type: FieldType.Password,
-        pattern: ".*",
+        pattern: /.*/,
        mask: true,
        multiline: true,
        icon: "lock",
@ -75,7 +75,7 @@ export const FIELD_DEFS: { [t in FieldType]: FieldDef } = {
    },
    [FieldType.Url]: {
        type: FieldType.Url,
-        pattern: ".*",
+        pattern: /[(http(s)?):\/\/(www\.)?a-zA-Z0-9@:%._\+~#=]{2,256}\.[a-z]{2,8}\b([-a-zA-Z0-9@:%_\+.~#?&//=]*)/i,
        mask: false,
        multiline: false,
        icon: "web",
@ -85,7 +85,7 @@ export const FIELD_DEFS: { [t in FieldType]: FieldDef } = {
    },
    [FieldType.Email]: {
        type: FieldType.Email,
-        pattern: ".*",
+        pattern: /^[\w-\.]+@([\w-]+\.)+[\w-]{2,8}$/,
        mask: false,
        multiline: false,
        icon: "email",
@ -95,7 +95,7 @@ export const FIELD_DEFS: { [t in FieldType]: FieldDef } = {
    },
    [FieldType.Date]: {
        type: FieldType.Date,
-        pattern: "\\d\\d\\d\\d-\\d\\d-\\d\\d",
+        pattern: /^\d{4}-(0[1-9]|1[012])-(0[1-9]|[12][0-9]|3[0-1])$/,
        mask: false,
        multiline: false,
        icon: "date",
@ -108,7 +108,7 @@ export const FIELD_DEFS: { [t in FieldType]: FieldDef } = {
    },
    [FieldType.Month]: {
        type: FieldType.Month,
-        pattern: "\\d\\d\\d\\d-\\d\\d",
+        pattern: /^\d{4}-(0[1-9]|1[012])$/,
        mask: false,
        multiline: false,
        icon: "month",
@ -118,7 +118,7 @@ export const FIELD_DEFS: { [t in FieldType]: FieldDef } = {
    },
    [FieldType.Credit]: {
        type: FieldType.Credit,
-        pattern: "\\d*",
+        pattern: /^\d{16}/,
        mask: true,
        multiline: false,
        icon: "credit",
@ -138,7 +138,7 @@ export const FIELD_DEFS: { [t in FieldType]: FieldDef } = {
    },
    [FieldType.Phone]: {
        type: FieldType.Phone,
-        pattern: ".*",
+        pattern: /\d/,
        mask: false,
        multiline: false,
        icon: "phone",
@ -148,7 +148,7 @@ export const FIELD_DEFS: { [t in FieldType]: FieldDef } = {
    },
    [FieldType.Pin]: {
        type: FieldType.Pin,
-        pattern: "\\d*",
+        pattern: /\d/,
        mask: true,
        multiline: false,
        icon: "lock",
@ -161,7 +161,7 @@ export const FIELD_DEFS: { [t in FieldType]: FieldDef } = {
    },
    [FieldType.Totp]: {
        type: FieldType.Totp,
-        pattern: ".*",
+        pattern: /.*/,
        mask: false,
        multiline: false,
        icon: "totp",
@ -174,7 +174,7 @@ export const FIELD_DEFS: { [t in FieldType]: FieldDef } = {
    },
    [FieldType.Note]: {
        type: FieldType.Note,
-        pattern: ".*",
+        pattern: /\n/,
        mask: false,
        multiline: true,
        icon: "note",
@ -184,7 +184,7 @@ export const FIELD_DEFS: { [t in FieldType]: FieldDef } = {
    },
    [FieldType.Text]: {
        type: FieldType.Text,
-        pattern: ".*",
+        pattern: /.*/,
        mask: false,
        multiline: false,
        icon: "text",
@ -296,22 +296,37 @@ export async function createVaultItem({
    });
 }

-const matchUsername = /username/i;
-const matchPassword = /password/i;
-const matchUrl = /url/i;
-const matchNote = /\n/;
-
 /** Guesses the most appropriate field type based on field name and value */
-export function guessFieldType({ name = "", value = "", masked }: any): FieldType {
-    return masked || name.match(matchPassword)
-        ? FieldType.Password
-        : name.match(matchUsername)
-        ? FieldType.Username
-        : name.match(matchUrl)
-        ? FieldType.Url
-        : value.match(matchNote)
-        ? FieldType.Note
-        : FieldType.Text;
+export function guessFieldType({
+    name,
+    value = "",
+    masked = false,
+}: {
+    name: string;
+    value?: string;
+    masked?: boolean;
+}): FieldType {
+    if (masked) {
+        return FieldType.Password;
+    }
+
+    let matchedTypeByName = Object.keys(FIELD_DEFS).filter((fieldType) =>
+        new RegExp(fieldType, "i").test(name)
+    )[0] as FieldType;
+
+    if (matchedTypeByName) {
+        return matchedTypeByName;
+    }
+
+    let matchedTypeByValue = Object.keys(FIELD_DEFS).filter((fieldType) =>
+        new RegExp(FIELD_DEFS[fieldType].pattern, "i").test(value)
+    )[0] as FieldType;
+
+    if (value !== "" && matchedTypeByValue) {
+        return matchedTypeByValue;
+    }
+
+    return FieldType.Text;
 }

 export interface ItemTemplate {