Skip to content

Feature enable export reports to csv file#326

Merged
kbeker merged 7 commits intomasterfrom
feature-enable-export-reports-to-csv-file
Jul 8, 2019
Merged

Feature enable export reports to csv file#326
kbeker merged 7 commits intomasterfrom
feature-enable-export-reports-to-csv-file

Conversation

@rwrzesien
Copy link
Contributor

Resolves #311

Copy link
Contributor

@dybi dybi left a comment

Choose a reason for hiding this comment

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

I tssems that CSV is created only from active sheet... I am not sure if this is intended or should we have it changed (#311 says nothing about it)

@rwrzesien
Copy link
Contributor Author

@dybi Yes, it is intended, as it is not technically possible for CSV file to have more than 1 sheet, so I have assumed that we want to export only single user report. If we want to export multi-sheet project report too, I would need a sample of output file structure. @Karrp do we want that and could you provide that sample?

@kbeker
Copy link
Contributor

kbeker commented Jun 24, 2019

@rwrzesien I think that @Karrp is now unreachable so I think that structure of this output may be as follows:

archive named by project_name

project_name_creation_date.zip
│
└─── name_firstLetterOfLastName1.csv
│
└─── name_firstLetterOfLastName2.csv
.
.
│
└─── name_firstLetterOfLastNameN-1.csv
│
└─── name_firstLetterOfLastNameN.csv

where N is a invisible number and is used only for example purposes.

I think that this may looks like this. What do you think guys @rwrzesien @dybi ?

@rwrzesien
Copy link
Contributor Author

@kbeker Nice idea, I like it! Will require some extra work but definitely doable.

@rwrzesien rwrzesien changed the title Feature enable export reports to csv file [WIP] Feature enable export reports to csv file Jun 24, 2019
@rwrzesien rwrzesien force-pushed the feature-enable-export-reports-to-csv-file branch from 692027d to 1a9e205 Compare June 26, 2019 22:22
@rwrzesien rwrzesien changed the title [WIP] Feature enable export reports to csv file Feature enable export reports to csv file Jun 26, 2019
@rwrzesien
Copy link
Contributor Author

rwrzesien commented Jun 26, 2019

@dybi @kbeker Updated and ready for review.

The code is a little bit low level and far from beautiful, so I am looking forward for good review from you guys.

Copy link
Contributor

@kbeker kbeker left a comment

Choose a reason for hiding this comment

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

In my opinion everything is OK, but please fix this in comments :)

next_row = []
for cell_number, cell in enumerate(row, start=1):
if isinstance(cell.value, str) and cell.value.lower().startswith("=timevalue"):
hours_as_string = cell.value[len('=timevalue("') : -len('")')]
Copy link
Contributor

Choose a reason for hiding this comment

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

Please fix:

=============================== LINT ===============================
[FLAKE8: sheetstorm]
./employees/common/exports.py:245:65: E203 whitespace before ':'

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 being set by black, so we can either disable it black config, or disable it in pylint config, or disable it only for this single line, what do you think would be best in this case?

Btw. where is black config file in our repo? I couldn't find it.

Copy link
Contributor

Choose a reason for hiding this comment

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

@rwrzesien Actually we don't have black config. We give parameters explicitly to black command. Can we by black config disable only one line in whole file?



def save_work_book_as_csv(writer: csv.DictWriter, work_book: Workbook) -> None:
sheet = work_book.get_active_sheet()
Copy link
Contributor

Choose a reason for hiding this comment

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

Please change it into:

sheet = work_book.active

This does the same thing, but get_active_sheet() method will be deleted in future versions and because of it we get warnings at the end of full_check

Copy link
Contributor Author

@rwrzesien rwrzesien Jun 29, 2019

Choose a reason for hiding this comment

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

👍 done

writer.writerow(next_row)


def save_work_book_as_zip_of_csv(work_book: Workbook) -> zipfile.ZipFile:
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't you return here BytesIO object?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, ZIP was returned in previous version of the code. 👍 done

{% with date=path|extract_year_and_month_from_url %}
<a href="{% url 'export-project-data-xlsx' pk=object.pk year=date.0 month=date.1 %}" class="btn btn-info">
Export Reports
Export Reports
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add here that this button gives XLSX export

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 done

@@ -100,6 +100,9 @@ <h4><font color="red">{{ error }}</font></h4>
<a href="{% url 'export-data-xlsx' pk=user.pk year=date.0 month=date.1 %}" class="btn btn-info">
Export {{ date.1 | convert_to_month_name }} {{ date.0 }} Reports
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add here that this gives XLSX export :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 done

@rwrzesien rwrzesien force-pushed the feature-enable-export-reports-to-csv-file branch from 1a9e205 to 6c709d9 Compare June 30, 2019 00:24
@kbeker kbeker force-pushed the feature-enable-export-reports-to-csv-file branch from 6c709d9 to 16f8f76 Compare July 8, 2019 10:02
@kbeker kbeker added this to the next_release milestone Jul 8, 2019
@kbeker kbeker merged commit 16f8f76 into master Jul 8, 2019
@kbeker kbeker deleted the feature-enable-export-reports-to-csv-file branch July 8, 2019 10:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature New feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enable export reports to CSV file

3 participants