Skip to content

Bugfix admin report form#263

Merged
kbeker merged 3 commits intomasterfrom
bugfix-admin-report-form
May 31, 2019
Merged

Bugfix admin report form#263
kbeker merged 3 commits intomasterfrom
bugfix-admin-report-form

Conversation

@kbeker
Copy link
Contributor

@kbeker kbeker commented May 31, 2019

Problem occurs when Admin or Manager edit employee report.

@kbeker kbeker added the bug Something isn't working label May 31, 2019
@kbeker kbeker added this to the next_release milestone May 31, 2019
@kbeker kbeker self-assigned this May 31, 2019
@rwrzesien
Copy link
Contributor

rwrzesien commented May 31, 2019 •
edited
Loading

@kbeker Could you please explain the problem a little more? Because from the pull request it looks like you are implementing handling for duration field in form, which, as far as I remember, works exactly like that out of the box.

@kbeker
Copy link
Contributor Author

kbeker commented May 31, 2019

@kbeker Could you please explain the problem a little more? Because from the pull request it looks like you are implementing handling for duration field in form, which as far as I remember works exactly like that out of the box.

@rwrzesien
Sure, I should do it in PR description earlier :P. When as a Manager or Admin we get to ProjectReportDetail or AdminReportView of any user without this bugfix we get work_hours in format HH:MM:SS and if manager or admin edit this report and send work_hours as a 08:00 it will convert it into 00:08:00 so instead of 8 hours it will be just 8 minutes. This PR fixes it and in this views manager or admin always get work_hours in format HH:MM and if he edit report and send work_hours as a 08:00 it will be converted correctly :)

rwrzesien
rwrzesien approved these changes May 31, 2019
View reviewed changes
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.

All right makes sense, thanks for explanation.

@kbeker kbeker force-pushed the bugfix-admin-report-form branch from ca22956 to 8a2cb0b Compare May 31, 2019 11:34
@kbeker kbeker merged commit 8a2cb0b into master May 31, 2019
@kbeker kbeker deleted the bugfix-admin-report-form branch May 31, 2019 11:37
dybi
dybi approved these changes Jun 3, 2019
View reviewed changes
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.

LGTM
approve_proud

employees/tests/test_unit_custom_forms.py
queryset = Project.objects.all()
form = ProjectJoinForm(queryset)
choices = form.fields["projects"].choices
self.assertIsNotNone(choices)
Copy link
Contributor

@dybi dybi May 31, 2019

Choose a reason for hiding this comment

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

it is redundant: below asserion makes sure we won't have None ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@maciejSamerdak maciejSamerdak Awaiting requested review from maciejSamerdak

@MartynaAnnaGottschling MartynaAnnaGottschling Awaiting requested review from MartynaAnnaGottschling

@Szymiks Szymiks Awaiting requested review from Szymiks

2 more reviewers

@rwrzesien rwrzesien rwrzesien approved these changes

@dybi dybi dybi approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@kbeker kbeker

Labels

bug Something isn't working

Projects

None yet

Milestone

v0.5.0

Development

Successfully merging this pull request may close these issues.

3 participants

@kbeker @rwrzesien @dybi