Many misc fixes and improvements

- Fix importing empty rows - Fix not importing first row (was being skipped too early) - Fix sample values including column row - Better type default (other instead of username) - Add new `matchPattern` to use for matching, make it stricter, and leave `pattern` for validation, looser.
2022-03-15 16:57:32 +00:00 · 2022-03-15 16:57:32 +00:00 · a4bbd20d6d
parent 7e732cba3a
commit a4bbd20d6d
4 changed files with 108 additions and 82 deletions
--- a/packages/app/src/elements/import-dialog.ts
+++ b/packages/app/src/elements/import-dialog.ts
@ -92,8 +92,11 @@ export class ImportDialog extends Dialog<File, void> {
                            .label=${$l("Name Column")}
                            .options=${this._itemColumns.map((itemColumn, itemColumnIndex) => ({
                                label: csvHasColumnsOnFirstRow
-                                    ? `${itemColumn.displayName} (${$l("Column {0}", itemColumnIndex.toString())})`
-                                    : $l("Column {0}", itemColumnIndex.toString()),
+                                    ? `${itemColumn.displayName} (${$l(
+                                          "Column {0}",
+                                          (itemColumnIndex + 1).toString()
+                                      )})`
+                                    : $l("Column {0}", (itemColumnIndex + 1).toString()),
                                value: itemColumnIndex,
                            }))}
                            .selectedIndex=${this._nameColumnSelect?.selectedIndex}
@ -120,7 +123,7 @@ export class ImportDialog extends Dialog<File, void> {
                                ...this._itemColumns.map((itemColumn, itemColumnIndex) => ({
                                    label: `${itemColumn.displayName} (${$l(
                                        "Column {0}",
-                                        itemColumnIndex.toString()
+                                        (itemColumnIndex + 1).toString()
                                    )})`,
                                    value: itemColumnIndex,
                                })),
@ -153,16 +156,16 @@ export class ImportDialog extends Dialog<File, void> {
                                        <div class="stretch">
                                            ${csvHasColumnsOnFirstRow
                                                ? itemColumn.name
-                                                : $l("Column {0}", itemColumnIndex.toString())}
+                                                : $l("Column {0}", (itemColumnIndex + 1).toString())}
                                        </div>
                                        <div class="subtle" ?hidden=${!csvHasColumnsOnFirstRow}>
-                                            ${$l("Column {0}", itemColumnIndex.toString())}
+                                            ${$l("Column {0}", (itemColumnIndex + 1).toString())}
                                        </div>
                                    </div>

                                    <div class="tiny horizontally-margined subtle mono ellipsis">
                                        ${itemColumn.values
-                                            .filter((v) => v !== "")
+                                            .filter((value) => value !== "")
                                            .slice(0, 20)
                                            .map((value) => (value.includes(",") ? `"${value}"` : value))
                                            .join(", ")}
@ -205,9 +208,9 @@ export class ImportDialog extends Dialog<File, void> {

                <pl-select
                    id="vaultSelect"
-                    .options=${app.vaults.map((v) => ({
-                        disabled: !app.isEditable(v),
-                        value: v,
+                    .options=${app.vaults.map((vault) => ({
+                        disabled: !app.isEditable(vault),
+                        value: vault,
                    }))}
                    .label=${$l("Target Vault")}
                ></pl-select>
@ -278,9 +281,9 @@ export class ImportDialog extends Dialog<File, void> {
                );
                this._items = result.items;
                this._itemColumns = result.itemColumns;
-                console.log("itemcolumns", this._itemColumns);
                await this.updateComplete;
                this._nameColumnSelect.selectedIndex = this._itemColumns.findIndex(({ type }) => type === "name");
+                // +1 because the first item is "none" for tags
                this._tagsColumnSelect.selectedIndex = this._itemColumns.findIndex(({ type }) => type === "tags") + 1;
                break;
            case imp.ONEPUX.value:
--- a/packages/app/src/lib/import.ts
+++ b/packages/app/src/lib/import.ts
@ -56,7 +56,7 @@ export function loadPapa(): Promise<any> {
 * Takes a data table (represented by a two-dimensional array) and converts it
 * into an array of items
 * @param  Array    data            Two-dimensional array containing tabular item data; The first 'row'
- *                                  should contain field names. All other rows represent items, containing
+ *                                  might contain field names. All other rows represent items, containing
 *                                  the item name, field values and optionally a list of tags.
 * @param  Array    columnTypes     Array containing the type of field per column.
 * @param  Boolean  columnsOnFirstRow  Boolean, representing if there are columnms on the first row.
@ -76,29 +76,38 @@ async function fromTable(
    const dataRows = columnsOnFirstRow ? data.slice(1) : data;

    // All subsequent rows should contain values
-    let items = dataRows.map(function (row) {
-        // Construct an array of field object from column names and values
-        let fields: Field[] = [];
-        for (let columnIndex = 0; columnIndex < row.length; ++columnIndex) {
-            // Skip name column, category column (if any) and empty fields
-            if (columnIndex !== nameColumnIndex && columnIndex !== tagsColumnIndex && row[columnIndex]) {
-                const name = columnTypes[columnIndex].displayName;
-                const value = row[columnIndex];
-                const type = columnTypes[columnIndex].type || undefined;
-                fields.push(
-                    new Field().fromRaw({
-                        name,
-                        value,
-                        type,
-                    })
-                );
+    const items = dataRows
+        .filter((row) => {
+            // Skip empty rows
+            if (row.length === 1 && row[0] === "") {
+                return false;
            }
-        }

-        const name = row[nameColumnIndex!];
-        const tags = row[tagsColumnIndex!];
-        return createVaultItem({ name, fields, tags: (tags && tags.split(",")) || [] });
-    });
+            return true;
+        })
+        .map((row) => {
+            // Construct an array of field object from column names and values
+            const fields: Field[] = [];
+            for (let columnIndex = 0; columnIndex < row.length; ++columnIndex) {
+                // Skip name column, category column (if any) and empty fields
+                if (columnIndex !== nameColumnIndex && columnIndex !== tagsColumnIndex && row[columnIndex]) {
+                    const name = columnTypes[columnIndex]?.displayName || "";
+                    const value = row[columnIndex];
+                    const type = columnTypes[columnIndex]?.type || undefined;
+                    fields.push(
+                        new Field().fromRaw({
+                            name,
+                            value,
+                            type,
+                        })
+                    );
+                }
+            }
+
+            const name = row[nameColumnIndex!];
+            const tags = row[tagsColumnIndex!];
+            return createVaultItem({ name, fields, tags: (tags && tags.split(",")) || [] });
+        });

    return Promise.all(items);
 }
@ -127,12 +136,7 @@ export async function asCSV(

    const columnNames = columnsOnFirstRow
        ? rows[0].map((column) => column.toLowerCase())
-        : rows[0].map((_, i) => $l("Column {0}", i.toString()));
-
-    // If first row is column names, discard it
-    if (columnsOnFirstRow) {
-        rows.shift();
-    }
+        : rows[0].map((_value, index) => $l("Column {0}", index.toString()));

    let hasNameColumn = false;
    let hasTagsColumn = false;
@ -141,13 +145,13 @@ export async function asCSV(
        mappedItemColumns.length > 0
            ? mappedItemColumns
            : columnNames.map((columnName, columnIndex) => {
-                  const values = rows.map((row) => row[columnIndex] || "");
+                  const values = (columnsOnFirstRow ? rows.slice(1) : rows).map((row) => row[columnIndex] || "");

                  // Guess field type based on first non-empty value
                  // TODO: Sample all values for more reliable results?
                  let type = guessFieldType({
                      name: columnsOnFirstRow ? columnName : "",
-                      value: values.find((v) => Boolean(v)),
+                      value: values.find((value) => Boolean(value)),
                  }) as ImportCSVColumn["type"];

                  // If we're not given field names by the first row, base the name on the type
--- a/packages/core/src/item.ts
+++ b/packages/core/src/item.ts
@ -32,8 +32,10 @@ export enum FieldType {
 export interface FieldDef {
    /** content type */
    type: FieldType;
-    /** regular expression describing pattern of field contents (not used for validation) */
+    /** regular expression describing pattern of field contents (used for validation) */
    pattern: RegExp;
+    /** regular expression describing pattern of field contents (used for matching) */
+    matchPattern: RegExp;
    /** whether the field should be masked when displayed */
    mask: boolean;
    /** whether the field value can have multiple lines */
@ -48,11 +50,12 @@ export interface FieldDef {
    transform?: (value: string) => Promise<string>;
 }

-/** Available field types and respective meta data */
+/** Available field types and respective meta data (order matters for pattern matching) */
 export const FIELD_DEFS: { [t in FieldType]: FieldDef } = {
    [FieldType.Username]: {
        type: FieldType.Username,
        pattern: /.*/,
+        matchPattern: /.*/,
        mask: false,
        multiline: false,
        icon: "user",
@ -63,6 +66,7 @@ export const FIELD_DEFS: { [t in FieldType]: FieldDef } = {
    [FieldType.Password]: {
        type: FieldType.Password,
        pattern: /.*/,
+        matchPattern: /.*/,
        mask: true,
        multiline: true,
        icon: "lock",
@ -73,19 +77,10 @@ export const FIELD_DEFS: { [t in FieldType]: FieldDef } = {
            return masked ? value.replace(/./g, "\u2022") : value;
        },
    },
-    [FieldType.Url]: {
-        type: FieldType.Url,
-        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",
-        get name() {
-            return $l("URL");
-        },
-    },
    [FieldType.Email]: {
        type: FieldType.Email,
-        pattern: /^[\w-\.]+@([\w-]+\.)+[\w-]{2,8}$/,
+        pattern: /(.*)@(.*)/,
+        matchPattern: /^[\w-\.]+@([\w-]+\.)+[\w-]{2,8}$/,
        mask: false,
        multiline: false,
        icon: "email",
@ -93,9 +88,21 @@ export const FIELD_DEFS: { [t in FieldType]: FieldDef } = {
            return $l("Email Address");
        },
    },
+    [FieldType.Url]: {
+        type: FieldType.Url,
+        pattern: /.*/,
+        matchPattern: /[(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",
+        get name() {
+            return $l("URL");
+        },
+    },
    [FieldType.Date]: {
        type: FieldType.Date,
        pattern: /^\d{4}-(0[1-9]|1[012])-(0[1-9]|[12][0-9]|3[0-1])$/,
+        matchPattern: /^\d{4}-(0[1-9]|1[012])-(0[1-9]|[12][0-9]|3[0-1])$/,
        mask: false,
        multiline: false,
        icon: "date",
@ -109,6 +116,7 @@ export const FIELD_DEFS: { [t in FieldType]: FieldDef } = {
    [FieldType.Month]: {
        type: FieldType.Month,
        pattern: /^\d{4}-(0[1-9]|1[012])$/,
+        matchPattern: /^\d{4}-(0[1-9]|1[012])$/,
        mask: false,
        multiline: false,
        icon: "month",
@ -118,7 +126,8 @@ export const FIELD_DEFS: { [t in FieldType]: FieldDef } = {
    },
    [FieldType.Credit]: {
        type: FieldType.Credit,
-        pattern: /^\d{16}/,
+        pattern: /.*/,
+        matchPattern: /^\d{16}/,
        mask: true,
        multiline: false,
        icon: "credit",
@ -136,32 +145,10 @@ export const FIELD_DEFS: { [t in FieldType]: FieldDef } = {
            return parts.join(" ");
        },
    },
-    [FieldType.Phone]: {
-        type: FieldType.Phone,
-        pattern: /\d+/,
-        mask: false,
-        multiline: false,
-        icon: "phone",
-        get name() {
-            return $l("Phone Number");
-        },
-    },
-    [FieldType.Pin]: {
-        type: FieldType.Pin,
-        pattern: /\d+/,
-        mask: true,
-        multiline: false,
-        icon: "lock",
-        get name() {
-            return $l("PIN");
-        },
-        format(value, masked) {
-            return masked ? value.replace(/./g, "\u2022") : value;
-        },
-    },
    [FieldType.Totp]: {
        type: FieldType.Totp,
-        pattern: /^([A-Z2-7=]{8})+$/,
+        pattern: /^([A-Z2-7=]{8})+$/i,
+        matchPattern: /^([A-Z2-7=]{8})+$/i,
        mask: false,
        multiline: false,
        icon: "totp",
@ -172,9 +159,35 @@ export const FIELD_DEFS: { [t in FieldType]: FieldDef } = {
            return await totp(base32ToBytes(value));
        },
    },
+    [FieldType.Phone]: {
+        type: FieldType.Phone,
+        pattern: /.*/,
+        matchPattern: /\d+/,
+        mask: false,
+        multiline: false,
+        icon: "phone",
+        get name() {
+            return $l("Phone Number");
+        },
+    },
+    [FieldType.Pin]: {
+        type: FieldType.Pin,
+        pattern: /.*/,
+        matchPattern: /\d+/,
+        mask: true,
+        multiline: false,
+        icon: "lock",
+        get name() {
+            return $l("PIN");
+        },
+        format(value, masked) {
+            return masked ? value.replace(/./g, "\u2022") : value;
+        },
+    },
    [FieldType.Note]: {
        type: FieldType.Note,
-        pattern: /.*(\n)?/,
+        pattern: /.*/,
+        matchPattern: /(.*)(\n)?(.*)/,
        mask: false,
        multiline: true,
        icon: "note",
@ -185,6 +198,7 @@ export const FIELD_DEFS: { [t in FieldType]: FieldDef } = {
    [FieldType.Text]: {
        type: FieldType.Text,
        pattern: /.*/,
+        matchPattern: /.*/,
        mask: false,
        multiline: false,
        icon: "text",
@ -318,9 +332,13 @@ export function guessFieldType({
        return matchedTypeByName;
    }

-    const matchedTypeByValue = Object.keys(FIELD_DEFS).filter((fieldType) =>
-        FIELD_DEFS[fieldType].pattern.test(value)
-    )[0] as FieldType;
+    // We skip some because they can match anything, and are only really valuable when matched by name
+    const fieldTypesToSkipByValue = [FieldType.Username, FieldType.Password];
+
+    const matchedTypeByValue = Object.keys(FIELD_DEFS)
+        // @ts-ignore this is a string, deal with it, TypeScript (can't `as` as well)
+        .filter((fieldType) => !fieldTypesToSkipByValue.includes(fieldType))
+        .filter((fieldType) => FIELD_DEFS[fieldType].matchPattern.test(value))[0] as FieldType;

    if (value !== "" && matchedTypeByValue) {
        return matchedTypeByValue;
--- a/packages/core/src/util.ts
+++ b/packages/core/src/util.ts
@ -193,6 +193,7 @@ export async function getIdFromEmail(email: string) {
 export function capitalize(string: string) {
    const words = string.split(" ");
    const phrase = words
+        .filter((word) => Boolean(word))
        .map((word) =>
            word.toLocaleLowerCase() === "url" ? "URL" : `${word[0].toLocaleUpperCase()}${word.slice(1) || ""}`
        )