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

Allow zero for project allocation on hourly projects #1730

Merged
merged 2 commits into from
Mar 13, 2024

Conversation

neilmb
Copy link
Member

@neilmb neilmb commented Mar 13, 2024

Description

#1728 was tracked down to a situation where the timecard form might submit "0" for project allocation even for hourly billing projects. This changes the form cleaning code to allow that possibility without raising the error message from the bug.

We add a test for the situation that fails in our current code and passes with the fix.

Additional information

There are a couple unrelated changes where or None was removed where it wasn't doing anything.

Fixes #1728

@neilmb neilmb requested a review from cantsin March 13, 2024 15:54
Copy link
Member

@cantsin cantsin left a comment

Choose a reason for hiding this comment

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

one question but LGTM, we can test in staging

self.cleaned_data['project_allocation'] != ''):
if (
"project_allocation" in self.cleaned_data
and not self._is_empty_or_zero(
Copy link
Member

Choose a reason for hiding this comment

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

do we need the staticmethod decorator here? Shouldn't we call it as TimecardObjectForm._is_empty_or_zero to be clearer?

Copy link
Member Author

Choose a reason for hiding this comment

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

We don't strictly need to use @staticmethod, but I think it's good practice when we don't need to use any properties of the instance or the class. As far as calling the method, self works and is much less verbose than having to repeat the class name.

@neilmb neilmb merged commit 100a7fd into main Mar 13, 2024
2 checks passed
@neilmb neilmb deleted the nmb/1728-zero-allocation branch March 13, 2024 17:49
This pull request was closed.
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.

bug: You cannot submit weekly billing for a project under hourly billing
2 participants