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

Enable Export with Excel Format #214

Merged
merged 12 commits into from
May 21, 2024
Merged

Enable Export with Excel Format #214

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
4d9a28f
Add Excel Export Capability
positiveVibes4all May 14, 2024
f0e5c73
Bump
positiveVibes4all May 14, 2024
f091073
Cleanup
positiveVibes4all May 14, 2024
ce54bf9
Cleanup
positiveVibes4all May 14, 2024
b596032
Addressed PR Feedback
positiveVibes4all May 17, 2024
be79412
Update Test Per Feedback
positiveVibes4all May 17, 2024
1f57edb
PR Feedback
positiveVibes4all May 20, 2024
9f106e5
Fixes
positiveVibes4all May 20, 2024
c66b8e1
Smart Column Width
positiveVibes4all May 20, 2024
9c6fa9b
Add Index Column BG Color
positiveVibes4all May 20, 2024
842095b
Update Tests
positiveVibes4all May 20, 2024
60a9584
Merge branch 'main' into feature/ADD-EXCEL-EXPORT-ABILITY
positiveVibes4all May 21, 2024
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
1 change: 1 addition & 0 deletions packages/export/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
"futil": "^1.76.4",
"lodash": "^4.17.21",
"minimal-csv-formatter": "^1.0.15",
"read-excel-file": "^5.8.1",
positiveVibes4all marked this conversation as resolved.
Show resolved Hide resolved
"write-excel-file": "^2.0.1"
},
"packageManager": "[email protected]"
Expand Down
3 changes: 2 additions & 1 deletion packages/export/src/csv.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ export default ({
// and key of the record.
// [{ key: string(supports lodash dot notation), label: string, display: function(value, {key, record, transform})}...]
transform,
headers = null, // array of strings to use as headers, array or arrays for multi-line headers
//TODO: support multi-line headers in excel and csv when implemented
headers = null, // array of strings to use as headers
onWrite = _.noop, // function to intercept writing a page of records
}) => {
stream.write(csv(headers || transformLabels(transform)))
Expand Down
13 changes: 9 additions & 4 deletions packages/export/src/excel.js
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

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

Can you write tests for this file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What type of test would you suggest? the data is binary, so we really cannot build one like is done for CSV's, I thought about it but it felt like there was little value add for doing this based on this constraint.

Copy link
Member

Choose a reason for hiding this comment

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

You can read the stream via https://www.npmjs.com/package/read-excel-file, which is nice as you don't need to write a file in the test, keeping it pure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This seems ok, I am not in love with it because we are using another library to test this one in essence but seems like a start.

Copy link
Member

Choose a reason for hiding this comment

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

It's by the same author so I think that reduces the risk somewhat

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, it just feels like we are testing the library itself instead of anything novel about how it is used, I will add it though as it is at least something. Thanks for the feedback on this.

Copy link
Member

Choose a reason for hiding this comment

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

My thinking is that there is some code that can benefit from testing which is not library related. Another approach is to take a writeData callback that looks like

      const readStream = await writeXlsxFile(excelData, { columns })
      for await (const chunk of readStream) {
        stream.write(chunk)
      }

That way you don't have to read the xlsx file but can still test this function.

Copy link
Member

Choose a reason for hiding this comment

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

Reminder that this still needs to be done.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is in the current PR and the tests is using it as such.

Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,12 @@ const convertToExcelCell = (value) => {
}
}

export const writeStreamData = async (stream, readStream) => {
for await (const chunk of readStream) {
stream.write(chunk)
}
}

let transformLabels = _.map(_.get('label'))

export default ({
Expand All @@ -20,7 +26,8 @@ export default ({
// and key of the record.
// [{ key: string(supports lodash dot notation), label: string, display: function(value, {key, record, transform})}...]
transform,
headers = null, // array of strings to use as headers, array or arrays for multi-line headers
//TODO: support multi-line headers in excel and csv when implemented
headers = null, // array of strings to use as headers
onWrite = _.noop, // function to intercept writing a page of records
}) => {
const excelData = [
Expand Down Expand Up @@ -60,9 +67,7 @@ export default ({
excelData[1]
)
const readStream = await writeXlsxFile(excelData, { columns })
for await (const chunk of readStream) {
stream.write(chunk)
}
await writeStreamData(stream, readStream)
stellarhoof marked this conversation as resolved.
Show resolved Hide resolved
await onWrite({ recordsWritten: recordsWritten, isStreamDone: true })
positiveVibes4all marked this conversation as resolved.
Show resolved Hide resolved
await stream.end()
})(),
Expand Down
66 changes: 66 additions & 0 deletions packages/export/src/excel.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
import _ from 'lodash/fp.js'

Check failure on line 1 in packages/export/src/excel.test.js

View workflow job for this annotation

GitHub Actions / ESLint

packages/export/src/excel.test.js#L1 

'_' is defined but never used (no-unused-vars)
import { PassThrough } from 'stream'
import writeXlsxFile from 'write-excel-file/node'
import readXlsxFile from 'read-excel-file/node'
import { writeStreamData } from './excel.js'

const testDate = new Date()

const HEADER_ROW = [
{
value: 'Name',
fontWeight: 'bold',
},
{
value: 'Date of Birth',
fontWeight: 'bold',
},
{
value: 'Cost',
fontWeight: 'bold',
},
{
value: 'Paid',
fontWeight: 'bold',
},
]

const DATA_ROW_1 = [
{
type: String,
value: 'John Smith',
},
{
type: Date,
value: testDate,
format: 'mm/dd/yyyy',
},
{
type: Number,
value: 1800,
},
{
type: Boolean,
value: true,
},
]

const expectedFileContents = [
['Name', 'Date of Birth', 'Cost', 'Paid'],
['John Smith', testDate, 1800, true],
]

const excelData = [HEADER_ROW, DATA_ROW_1]

// These are skipped on purpose as they actual write CSVs
describe('Excel Stream Test', () => {
it('Export to Excel and Validate', async () => {
const writeStream = new PassThrough()
const readStream = await writeXlsxFile(excelData)
await writeStreamData(writeStream, readStream)
const rows = await readXlsxFile(writeStream)
console.log(rows)
console.log(expectedFileContents)
expect(rows).toStrictEqual(expectedFileContents)
})
})
Loading
Loading