-
Notifications
You must be signed in to change notification settings - Fork 2
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.
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
Added support for export of records with MSExcel #178
Changes from 1 commit
3f0eee4
9cb62f2
bb140f5
b3fbab1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -104,6 +104,7 @@ class RecordsController extends React.Component { | |
this.props.exportRecords(exportType, { | ||
...this.state.filters, | ||
sort: sortToParams(this.state.sort), | ||
page: this.state.pageNumber, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @kostobog I believe this is what this parameter is supposed to represent? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @LaChope |
||
}), | ||
"records", | ||
); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed this as described here: kbss-cvut/record-manager#52 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest to revert this change and keep the accept header.
Without it the backend will not receive the export type and it will always return JSON.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it returns the correct file type even without it (not sure how tbh, I think it it because of the file extension), but if I add the headers, the backend returns 406 (only for excel file types)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@LaChope
I just tested this PR. The when downloading excel the file has
xslx
extension but the content is json.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oohh ok I see, then I can put back the headers but I will get 406 error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kostobog I reverted
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@LaChope @blcham
Export of both json and excel in distribution works for small number of records. I tested it with 71 records.