Skip to content

Bugfix total hours in csv in project reports#440

Merged
dybi merged 3 commits intomasterfrom
bugfix-hours-sum-not-working
Jul 23, 2019
Merged

Bugfix total hours in csv in project reports#440
dybi merged 3 commits intomasterfrom
bugfix-hours-sum-not-working

Conversation

@Szymiks
Copy link
Contributor

@Szymiks Szymiks commented Jul 17, 2019

Resolves: #436
The problem was only in csv in project reports.

@Szymiks Szymiks added bug Something isn't working priority medium Tasks with medium priority labels Jul 17, 2019
@Szymiks Szymiks added this to the next_release milestone Jul 17, 2019
@Szymiks Szymiks self-assigned this Jul 17, 2019
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.

Please see comment before merging.



def save_work_book_as_csv(writer: _writer, work_book: Workbook) -> None:
def save_work_book_as_csv(writer: _writer, work_book: Workbook, zip_version: bool = False) -> None:
Copy link
Contributor

Choose a reason for hiding this comment

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

save_work_book_as_csv has no knowledge about zip files. zip_version boolean is used to choose between SINGLE_USER constants and USER_IN_PROJECT constants. Is should be changed to sth like is_report_for_single_user.

Or even better - extract logic for choosing column to function call and pass hours_column_name instead.

Copy link
Contributor Author

@Szymiks Szymiks Jul 17, 2019

Choose a reason for hiding this comment

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

I was thinking about second option but I wasn't sure so I left it like that to listen your opinion, so I will change it as you said and pass hours_column_name but I have a question @dybi, should I pass this long version for instance: constants.HEADERS_TO_COLUMNS_SETTINGS_FOR_USER_IN_PROJECT.value[constants.HOURS_HEADER_STR.value].position or assign it to variable before passing?

Copy link
Contributor

@rwrzesien rwrzesien left a comment

Choose a reason for hiding this comment

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

Looks good, but please apply @dybi comments.

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.

I agree same as @rwrzesien and @dybi but please apply what @dybi asked for ;)

@Szymiks Szymiks force-pushed the bugfix-hours-sum-not-working branch from f269fd0 to c7485e8 Compare July 18, 2019 12:40
@dybi dybi force-pushed the bugfix-hours-sum-not-working branch from c7485e8 to 7342b35 Compare July 23, 2019 11:23
@dybi dybi merged commit 5931653 into master Jul 23, 2019
@dybi dybi deleted the bugfix-hours-sum-not-working branch July 23, 2019 11:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working priority medium Tasks with medium priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Hour sum not work in CSV

4 participants