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.

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

#104 Add export functionality #115

Merged
merged 27 commits into from
Sep 3, 2016
Merged

Conversation

JosefDvorak
Copy link
Collaborator

@JosefDvorak JosefDvorak commented Sep 1, 2016

This pull request adds a new UI to allow selecting and running modules in bulk. The results are saved in xlsx, docx or xml file.

It is possible to implement additional export modules by implementing IExportModule.

exportcollapsed
exportexpanded


// Send stream
var response = Request.CreateResponse(HttpStatusCode.OK);
response.Content = new ByteArrayContent(memoryStream.ToArray());
Copy link
Contributor

Choose a reason for hiding this comment

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

why not to use StreamContent to prevent copying byte arrays back and forth...?

@petrsvihlik
Copy link
Contributor

discovered while testing:

  • when I export to word the first page is blank (while it should contain "Kinspector report"
  • can you make it a little bit more user friendly?
    • use "Excel/Word" instead of xlsx/docx in the dropdown
    • is it necessary to have the module enabled in the "Setup" section?
    • module selection - is it wise to select all modules by default? uneducated user may be surprised why it takes so long to export... I'd probably invert it and implement a "(de)select all" functionality in another PR

code review:

  • the performance related comments in the code (they apply to all export modules)
  • all public members have to be commented using xmldoc (especially things like interfaces, extension methods) + go through the export implementations and add comments where it would make sense

otherwise it looks and works reeealy great! good job @JosefDvorak !

@JosefDvorak
Copy link
Collaborator Author

  • I've fixed the docx template. The NPOI library seems to have problem with placing text in a text section.
  • I've updated the naming to Word/Excel.
  • I was thinking about hiding the Export button in setup module as well, but then I figured there is no real harm in having it there. I won't break anything, just export results of the setup scripts. I'd leave it up to the user if they wants to use the export functionality with these modules.
  • I pre-selected the modules because I thought that would be the more user friendly way. I'm ok with having them un-selected by default, so I removed the pre-selection.
  • I've implemented the lock-able MemoryStream workaround
  • I've tried to use the StreamContent before, but kept running into issues where the file size was 0. I've found the issue and made it work.
  • Last thing to finish are the comments.

@petrsvihlik petrsvihlik merged commit 7b9ea0f into Kentico:master Sep 3, 2016
@petrsvihlik
Copy link
Contributor

good job, @JosefDvorak! thank you :)

@petrsvihlik petrsvihlik added this to the 3.4 milestone Sep 3, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants