-
Notifications
You must be signed in to change notification settings - Fork 14
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
#3748 - Content - Student current year income dynamic year #3791
#3748 - Content - Student current year income dynamic year #3791
Conversation
"properties": {}, | ||
"customClass": "", | ||
"lockKey": true, | ||
"calculateValue": "value = data.programYear ? data.programYear: \"\";" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If data.programYear
already has the desired value, is there a need to create a second hidden field?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I have tried to separate data.programYear
with 'Current tax Tear', so I have defined new Current tax year hidden value. Just in case if we need know Current tax year is fetched.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
About the justification "Just in case if we need know Current tax year is fetched", when that would be a benefit since this is only used to be displayed in some labels and is derived from the program year?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case, 'data.programYear' gives first year of program year like '2024', which is the required value for Current tax year
. I was thinking to use separate variable name of Current tax year
rather than use same data.programYear
as variable name is not as per the required Current tax year
to follow standardization.
"input": true, | ||
"tableView": true, | ||
"key": "currentTaxYear", | ||
"label": "currentTaxYear", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Label updated
"label": "currentTaxYear", | ||
"protected": false, | ||
"unique": false, | ||
"persistent": true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please set the persistent
to false
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice changes, please take a look at the comments.
@@ -279,7 +298,7 @@ | |||
"value": "" | |||
} | |||
], | |||
"content": "If you have had, or anticipate having a significant decrease in income for {{data.calculatedTaxYear}}, you may request to be assessed using this year's estimated income.", | |||
"content": "If you have had, or anticipate having a significant decrease in income for {{data.currentTaxYear}}, you may request to be assessed using this year's estimated income.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use data.programYear, or please explain why this was not used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I have tried to separate data.programYear
with 'Current tax Tear', so I have defined new Current tax year hidden value. Just in case if we need know Current tax year is being fetched.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The data.programYear
in this form is not actually the "2024-2025" program year name?
If I am right the question from @guru-aot would not make sense and and reply from @bidyashish should probably mention it. Please let me know if I am missing something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes thats correct, data.programYear
gives '2024-2025' as Program year, I used the the similar logic as calculatedTaxYear
to get Current Tax Year.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work @bidyashish , please have a look on the comments.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving assuming the "Current Tax Year" hidden variable was created in all forms using the same name to avoid confusion in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, nice work @bidyashish
Below AC were Done previously and not needed any more as part of this PR.