TP2000-143: 500 on Measure Detail view #497
Conversation
There was a problem hiding this comment.
Is it possible to do this in a general way and apply it anywhere else we have a SID? Also, are there any implications for this being a validation and not a business rule e.g. will it be possible to import duplicate SIDs and fix them later, or will this prevent that?
|
I think a 500 error would block import, so maybe a business rule is better. Might make generalising it for other components a bit easier too |
…as only identifying field
…b.com/uktrade/tamato into TP2000-143-500-on-Measure-Detail-view
|
I added a test for measures just because that was what originally motivated the PR, but I think tests in common/tests/test_business_rules should already have us covered. |
|
This is affecting github actions: aws/aws-sam-cli#3661 |
Codecov Report
@@ Coverage Diff @@
## master #497 +/- ##
==========================================
+ Coverage 91.99% 92.00% +0.01%
==========================================
Files 294 295 +1
Lines 18151 18224 +73
Branches 1450 1453 +3
==========================================
+ Hits 16698 16767 +69
- Misses 1186 1189 +3
- Partials 267 268 +1
Continue to review full report at Codecov.
|
commodities/models/orm.py
Outdated
| @@ -176,6 +177,7 @@ def footnote_application_codes(self) -> Set[ApplicationCode]: | |||
| business_rules.NIG31, | |||
| business_rules.NIG34, | |||
| business_rules.NIG35, | |||
| UniqueIdentifyingFields, | |||
There was a problem hiding this comment.
Ah I think in this case NIG1 actually has a slightly different requirement.
There was a problem hiding this comment.
Ah right, so they're allowed to have the same sid, as long as their validity periods don't overlap
* add custom sid validation in Measure.clean and call full_clean in Measure.save * add vanilla UniqueIdentifyingFields business rule to models with sid as only identifying field * specify markupsafe version * remove uniqueidentifyingfields where rule already exists
* add custom sid validation in Measure.clean and call full_clean in Measure.save * add vanilla UniqueIdentifyingFields business rule to models with sid as only identifying field * specify markupsafe version * remove uniqueidentifyingfields where rule already exists
Why
It shouldn't be possible to create a measure with the same
sidas another measure from a differentversion_group. There's still a separate issue being explored in this ticket ( https://uktrade.atlassian.net/browse/TP2000-42 ) around synchronising measure sids so that data engineers can make changes alongside those of Tariff Managers on the frontend, but this should add a bit of extra security in the meantime.What