Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add type casting to CSV fields #82

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Add type casting to CSV fields #82

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
f5220bb
Add type casting to CSV fields
Libertie Oct 7, 2021
6125efa
Correct fields prop default
Libertie Oct 8, 2021
524680c
Refactor to typeCast method + add error handling
Libertie Oct 12, 2021
4bc1a68
Document new invalidFieldType text
Libertie Oct 12, 2021
ba37f17
Additional error handling for typeCast method
Libertie Oct 15, 2021
7cf7ae2
Expand documentation for type casting
Libertie Oct 15, 2021
d628eff
Reorganize to eliminate redundancy in typeCast
Libertie Oct 15, 2021
2e728c8
Filter csv map to prevent typeCast errors
Libertie Oct 17, 2021
0c21a32
Reduce nesting in buildMappedCsv()
Libertie Oct 22, 2021
dfbc006
Bump cli-plugin-unit-jest version
Libertie Feb 15, 2022
e045a72
Update for republish on npmjs.com
Libertie Apr 8, 2023
ceb547b
Merge remote-tracking branch 'upstream/master'
Libertie Apr 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 32 additions & 3 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,8 @@ Primary wrapper component.
},
age: {
required: true,
label: 'Age'
label: 'Age',
type: Number
}
}"
>
Expand All @@ -101,16 +102,44 @@ Primary wrapper component.

| Prop | Default | Description |
| ------ | ------- | ----------- |
| fields | null | (required) The field names used to map the CSV. |
| fields | null | (required) The fields used to map the CSV - see below. |
| text | see below | (optional) Override the default text used in the component. |
| modelValue | N/A | (optional) Binds to the mapped CSV object. |

#### Default text
##### Fields Prop

The fields prop may be a simple array (e.g. `['name', 'age']`) or an object with the following properties:

| Prop | Default | Description |
| ------ | ------- | ----------- |
| required | true | (required) The field names used to map the CSV. |
| label | N/A | (required) Override the default text used in the component. |
| type | String | (optional) A primitive object used to cast the field value - see below |

The type property supports casting with primitive objects, including String, Number, BigInt, and Boolean.

- Number fields - should contain numeric values
- BigInt fields - should contain valid integers or strings (incl. hex, oct, or binary)
- Boolean fields - should include true, false, yes, no, on, off, 1, 0, null or an empty string

Field values that are incompatible with the specified type will be returned as strings and will generate an error.

A closure, may also be assigned to the type property for more complex casting:
```
discount: {
required: true,
label: 'Discount %',
type: (v) => Number((v / 100)) }
}
```

##### Default Text Prop

```json
{
errors: {
fileRequired: 'A file is required',
invalidFieldType: "Invalid data type in row ${row} column ${col}. ",
invalidMimeType: "Invalid file type"
},
toggleHeaders: 'File has headers',
Expand Down
44 changes: 41 additions & 3 deletions src/components/VueCsvImport.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,12 @@
import map from 'lodash/map';
import set from 'lodash/set';
import merge from 'lodash/merge';
import pickBy from 'lodash/pickBy';

const defaultLanguage = {
errors: {
fileRequired: 'A file is required',
invalidFieldType: 'Invalid data type in row ${row} column ${col}. ',
invalidMimeType: "Invalid file type"
},
toggleHeaders: 'File has headers',
Expand All @@ -46,6 +48,7 @@
key: key,
label: get(val, 'label', val),
required: get(val, 'required', true),
type: get(val, 'type', String),
};
});
}
Expand Down Expand Up @@ -84,10 +87,23 @@
const buildMappedCsv = function () {
let newCsv = VueCsvImportData.fileHasHeaders ? VueCsvImportData.rawCsv : drop(VueCsvImportData.rawCsv);

VueCsvImportData.value = map(newCsv, (row) => {
VueCsvImportData.errors = [];

VueCsvImportData.value = map(newCsv, (row, index) => {
let newRow = {};
forEach(VueCsvImportData.map, (column, field) => {
set(newRow, field, get(row, column));
forEach(pickBy(VueCsvImportData.map, (v) => Number.isInteger(v)), (column, field) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This nested logic is pretty hairy.
Am I correct in assuming you are trying to do this?

VueCsvImportData
  .filter((v) => Number.isInteger(v))
  .map(([column, field]) => { try... })

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, that's not quite it. Here is that code fully commented, with the bit you highlighted split out for clarity:

VueCsvImportData.value = map(newCsv, (row, index) => {
    // Start a new row
    let newRow = {};

    // We only want to include mapped fields, so we'll exclude anything
    // in VueCsvImportData.map that isn't mapped to a column number
    let currentlyMapped = pickBy(VueCsvImportData.map, (v) => Number.isInteger(v));

    // Loop through the mapped fields and build up a new row with each field
    forEach(currentlyMapped, (column, field) => {
        // Get the field value
        let fieldVal = get(row, column);

        // Try casting the field value
        try { 
            fieldVal = typeCast(field, fieldVal);

        // If that generates an error, add a message to the VueCsvImportData.errors array
        } catch(err) {
            VueCsvImportData.errors.push(
                defaultLanguage.errors.invalidFieldType
                    .replace(/\${row}/g, index + (VueCsvImportData.fileHasHeaders ? 1 : 2))
                    .replace(/\${col}/g, column + 1)
            );

        // Either way, add it to the row
        } finally {
            set(newRow, field, fieldVal);
        }
    });

    // Return the completed row!
    return newRow;
});

Filtering VueCsvImportData.map is necessary because if you selected a column for a field, but then later unselected it, the field remains in the map object but is assigned a null value. In order to loop through the map and build up the new row, we have to exclude these unmapped fields. Otherwise they will generate errors.

I'm happy to add some or all of the commenting above to my code. And, of course, if you seen opportunities to improve it please let me know!

Copy link
Contributor

@James-Burgess James-Burgess Oct 22, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok that makes sense now! Nice catch 🥇

Could you just update the method to moved the nested curreltyMapped into what it looks like in your comment above. ie:

Suggested change
forEach(pickBy(VueCsvImportData.map, (v) => Number.isInteger(v)), (column, field) => {
// We only want to include mapped fields because deselected fields remain in the map object
// so we'll exclude anything in VueCsvImportData.map that isn't mapped to a column number
const currentlyMapped = pickBy(VueCsvImportData.map, (v) => Number.isInteger(v));
forEach(currentlyMapped, (column, field) => {

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good. And it looks like we can actually get a slight performance boost by moving that variable assignment out of the map function. I'll push a commit.

let fieldVal = get(row, column);
try {
fieldVal = typeCast(field, fieldVal);
} catch(err) {
VueCsvImportData.errors.push(
defaultLanguage.errors.invalidFieldType
.replace(/\${row}/g, index + (VueCsvImportData.fileHasHeaders ? 1 : 2))
.replace(/\${col}/g, column + 1)
);
} finally {
set(newRow, field, fieldVal);
}
});

return newRow;
Expand All @@ -96,6 +112,28 @@
emit('update:modelValue', VueCsvImportData.value);
};

const typeMap = VueCsvImportData.fields.reduce((a, f) => set(a, f.key, f.type ?? String), {});

const typeCast = function(field, fieldVal) {
let castVal = typeMap[field](fieldVal);

// Handle Booleans
if (typeMap[field] === Boolean) {
switch (fieldVal.toLowerCase().trim()) {
case 'false': case 'no': case 'off': case '0': case 'null': case '': return false;
case 'true': case 'yes': case 'on': case '1': return true;
default: throw 'Not a boolean!';
}
}

// Catch non-numeric Numbers
if (Object.is(NaN, castVal)) {
throw 'Not a number!';
}

return castVal;
};

provide('VueCsvImportData', VueCsvImportData);
provide('buildMappedCsv', buildMappedCsv);

Expand Down