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.

Sign up for GitHub

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

Jump to bottom

Fixes amount propery persistance scale handling #669

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from
Open

Fixes amount propery persistance scale handling #669

wants to merge 5 commits into from

Conversation

mkeckmkeck
Copy link
Contributor

@mkeckmkeck mkeckmkeck commented Dec 18, 2024 •
edited
Loading

Description

When applying user input or import input data to an Amount field of an entity, we must apply the scale setting that would be applied by the database. So we are able to properly compare values of the field and detect changes to it.

  • Drive by: Fix scale of default value in case of missing Numeric annotation

Out of scope:

  • precision, as this is less significant (if you have to low precision you are screwed anyway) and harder to check/ensure

Additional Notes

  • This PR fixes or works on following ticket(s): SIRI-1041

Checklist

  • Code change has been tested and works locally
  • Code was formatted via IntelliJ and follows SonarLint & best practices
  • Patch Tasks: Is local execution of Patch Tasks necessary? If so, please also mark the PR with the tag.

@mkeckmkeck
Fixes default value scaling in case of missing Numeric annotation
2c5f9e0 
- Using Numeric annotation is strongly advised and missing annotations
  get reported at startup. So no one should define an Amount field without it
- But when it is missing, AmountProperty#contributeToTable will default to (15, 0)
  and so we should also apply scale 0 as default to the sql default value

Fixes: SIRI-1041
@mkeckmkeck
Creates a failing testcase for amount scaling
538ef8e 
Fixes: SIRI-1041
@mkeckmkeck
Applies persistance scale on amounts
e12083c 
- when transforming values from user input or import input
- round by the amount property field defined scale so the current field value
  has the same scale as persisted and loaded data

Fixes: SIRI-1041
@mkeckmkeck mkeckmkeck added the 🐛 Bugfix Contains only a small fix for an existing bug label Dec 18, 2024
@mkeckmkeck mkeckmkeck added the 🖐 Keep open Should not be merged label Dec 18, 2024
@mkeckmkeck
Only apply amount property scaling in case a Numeric annotation exists
5ec3512 
- so mongo can keep on going with "unlimited" scale and not annotated fields
- and adds test case for scaling

Fixes: SIRI-1041
sabieber
sabieber approved these changes Dec 18, 2024
View reviewed changes
Copy link
Member

@sabieber sabieber left a comment

Choose a reason for hiding this comment

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

Thanks for the added mongo test 👌🏻

@sabieber sabieber removed the 🖐 Keep open Should not be merged label Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@scireum-mbo scireum-mbo scireum-mbo approved these changes

@mko-sci mko-sci mko-sci approved these changes

@idlira idlira idlira approved these changes

@jmuscireum jmuscireum jmuscireum approved these changes

@sabieber sabieber sabieber approved these changes

Assignees
No one assigned
Labels
🐛 Bugfix Contains only a small fix for an existing bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@mkeckmkeck @sabieber @jmuscireum @idlira @mko-sci @scireum-mbo