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

Update Money protobuf validation to use string value #44

Merged
merged 1 commit into from
Aug 23, 2022

Conversation

rpatel-figure
Copy link
Collaborator

@rpatel-figure rpatel-figure commented Aug 2, 2022

Context

Updating the contracts to the latest version of metadata-asset-model per the changes made in provenance-io/metadata-asset-model#15

Changes

Patch

  • Update metadata-asset-model from 0.1.11 to 0.1.12
  • Change tech.figure.util.v1beta1.Money validation to validate the new string field over the deprecated double field
  • Add forgotten validation of currency field
  • Refactor the corresponding validation method to provide more specific exceptions
  • Adjust signature and file location of similar checksum validation method

Notes

  • Currently, the only contracts here that could make use of the Money validation are ones which update the top-level fields of the ServicingData record. However, the loan scope and its contracts are still in beta and being influenced by initial use cases, so ServicingData.originalNoteAmount does not undergo any validation as of this version.

@rpatel-figure rpatel-figure added patch Bug fixes or other non-breaking configuration changes dependencies An update to project dependencies labels Aug 2, 2022
@github-actions
Copy link

github-actions bot commented Aug 2, 2022

Test Results

  12 files  ±0    12 suites  ±0   3m 13s ⏱️ +29s
  48 tests +2    42 ✔️ +2    6 💤 ±0  0 ±0 
120 runs  +3  109 ✔️ +3  11 💤 ±0  0 ±0 

Results for commit 933173d. ± Comparison against base commit bf16778.

This pull request removes 1 and adds 3 tests. Note that renamed tests count towards both.
io.provenance.scope.loan.utility.DataValidationUnitTest ‑ return true for any double
io.provenance.scope.loan.utility.DataValidationUnitTest ‑ return true for any double and 3-letter currency
io.provenance.scope.loan.utility.DataValidationUnitTest ‑ throw an appropriate exception for any currency not 3 characters long
io.provenance.scope.loan.utility.DataValidationUnitTest ‑ throw an appropriate exception for any currency not consisting solely of letters

♻️ This comment has been updated with latest results.

@codecov-commenter
Copy link

codecov-commenter commented Aug 2, 2022

Codecov Report

Merging #44 (933173d) into main (bf16778) will increase coverage by 0.31%.
The diff coverage is 84.61%.

@@            Coverage Diff             @@
##             main      #44      +/-   ##
==========================================
+ Coverage   73.38%   73.70%   +0.31%     
==========================================
  Files          16       16              
  Lines         496      502       +6     
  Branches      119      120       +1     
==========================================
+ Hits          364      370       +6     
  Misses         81       81              
  Partials       51       51              
Impacted Files Coverage Δ
...ance/scope/loan/utility/LoanContractValidations.kt 69.42% <ø> (-0.66%) ⬇️
...nce/scope/loan/utility/DataValidationExtensions.kt 66.66% <84.61%> (+14.28%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@rpatel-figure rpatel-figure force-pushed the ravipatel/update-to-string-money-value branch from b907382 to c292e3b Compare August 3, 2022 14:24
@rpatel-figure rpatel-figure force-pushed the ravipatel/update-to-string-money-value branch from c292e3b to 933173d Compare August 3, 2022 14:29
internal fun ContractEnforcementContext.moneyValidation(parentDescription: String = "Input's money", money: FigureTechMoney?) {
money.takeIf { it.isSet() }?.let { setMoney ->
requireThat(
setMoney.value.matches(Regex("^[-]?([0-9]+(?:[\\\\.][0-9]+)?|\\\\.[0-9]+)\$")) orError "$parentDescription must have a valid value",
Copy link
Collaborator

Choose a reason for hiding this comment

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

just curious: are values less than $1 supposed to come in as 0.15 or .15? In the latter case, the decimal wouldn't be picked up by this expression

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The former, with a leading 0. It just seems to be the choice made 🤷🏽

@rpatel-figure rpatel-figure merged commit f361d7c into main Aug 23, 2022
@rpatel-figure rpatel-figure deleted the ravipatel/update-to-string-money-value branch August 23, 2022 17:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies An update to project dependencies patch Bug fixes or other non-breaking configuration changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants