Add validation for 'Verify win' stage of investment projects #286
Conversation
…-verify-win-validation
Codecov Report
@@ Coverage Diff @@
## develop #286 +/- ##
==========================================
- Coverage 92.9% 92.9% -0.01%
==========================================
Files 60 60
Lines 2214 2198 -16
Branches 200 193 -7
==========================================
- Hits 2057 2042 -15
Misses 133 133
+ Partials 24 23 -1
Continue to review full report at Codecov.
|
datahub/core/utils.py
Outdated
| @@ -11,6 +11,11 @@ | |||
| logger = getLogger(__name__) | |||
|
|
|||
|
|
|||
| def is_falsey(val): | |||
| """Returns True if bool(val) is False""" | |||
| return not val | |||
There was a problem hiding this comment.
RLY, why not a plain lambda? ;]
There was a problem hiding this comment.
Then maybe run everywhere s/if not (.+):/if is_falsey($1)/ ;]
There was a problem hiding this comment.
operator.not_() did the same thing, so it's gone now.
datahub/investment/validate.py
Outdated
| @@ -52,11 +52,11 @@ | |||
| 'non_fdi_type': | |||
| CondValRule('investment_type', InvestmentType.non_fdi.value.id, Phase.prospect.value), | |||
| 'total_investment': | |||
| CondValRule('client_cannot_provide_total_investment', is_falsey, Phase.assign_pm.value), | |||
| CondValRule('client_cannot_provide_total_investment', not_, Phase.assign_pm.value), | |||
| 'address_line_postcode': 'This field is required.', | ||
| 'average_salary': 'This field is required.', | ||
| 'client_cannot_provide_foreign_investment': 'This field is required.', | ||
| 'foreign_equity_investment': 'This field is required.', |
There was a problem hiding this comment.
Do we need to repeat This field is required. like that?
There was a problem hiding this comment.
Not sure what you mean – what is your suggestion?
There was a problem hiding this comment.
That way it doesn't immediately tells that the responses are identical. I would either use constant REQUIRED_MESSAGE or create a variable this_field_is_required and assert against it.
For example:
...
'address_line_postcode': REQUIRED_MESSAGE,
'average_salary': REQUIRED_MESSAGE,
...
There was a problem hiding this comment.
It doesn't make much difference to me, but I kinda like seeing the output as is in tests.
There was a problem hiding this comment.
To me it hints that maybe one of the responses is different than the others in a subtle way (for example missing dot or a typo), which you can't tell without looking more closely.
This also changes how the validation works so it's simple and more maintainable.