OFS-212: changes in calculation module related to contract module #8
Conversation
| cd_params = json.loads(cd_params) | ||
| if isinstance(phi_params, str): | ||
| phi_params = json.loads(phi_params) | ||
| if "rate" in cp_params: |
There was a problem hiding this comment.
I think it'd be good to put calculation parameter variables under distinct json_ext section. E.g. instead of
json_ext = {'rate': x, 'income': y} use json_ext = {'calculation_rule': {'rate': x, 'income': y}. This way we can avoid overlapping of the names of keys that are shared between modules. Let me know what do you think, we can discuss it with Patrick and adjust later
There was a problem hiding this comment.
I think your proposition is good and if Patrick is back we can discuss this with him, It is not a huge change in that moment!
There was a problem hiding this comment.
@delcroip I think Damian idea is good. We can discuss about that later.
There was a problem hiding this comment.
Actually it is how I imagined it but I didn't specify it .... when I realized it FE and BE where develop; @rstencel is it long to change the FE to use a distinct json section ?
There was a problem hiding this comment.
@delcroip no, it should take a few hours. This distinct section will have a static name (key to be precise) calculation_rule?
There was a problem hiding this comment.
yes always the same name for the calculation, Then we need to create 2 tickets one for BE and one for FE as Medium priority
There was a problem hiding this comment.
@delcroip I have created a ticket for changes on frontend: https://openimis.atlassian.net/browse/OFS-243
There was a problem hiding this comment.
@delcroip I also created https://openimis.atlassian.net/browse/OFS-244 for a backend. I will update my Code.
| @@ -68,11 +71,34 @@ def check_calculation(cls, instance): | |||
| @classmethod | |||
| def calculate(cls, instance, *args): | |||
| if instance.__class__.__name__ == "ContractContributionPlanDetails": | |||
There was a problem hiding this comment.
I'm not sure it it's best approach for checking types. I'm aware that we cannot directly compare types as it'd require importing ContractContributionPlanDetails class, but can we use something like
if 'contribution_plan' in sys.modules and isinstance(instance, sys.modules['contribution_plan'].ContractContributionPlanDetails) ?
There was a problem hiding this comment.
https://openimis.atlassian.net/wiki/spaces/OP/pages/2250473529/FS+calculation+rule+1+contribution+valuation - on wiki page - in "calculation" specification we have such condition defined by @delcroip
if instance.__class__ == ContractContributionPlanDetails.
rate = instance.contribution_plan.json_ext.rate
income = instance.contract_details.json_ext.income
CCPD.value = income * rate
return CCPD.value
else Return False
That is why I used this approach. But I can define this as you wrote - check module and instance!
There was a problem hiding this comment.
Also I think that "ContractContributionPlanDetails" is related to the contract and we should check a Contract module instead of contribution_plan.
if 'contract' in sys.modules and isinstance(instance, sys.modules['contract'].ContractContributionPlanDetails) ?
TICKET - OFS-212 - https://openimis.atlassian.net/browse/OFS-212
In that PR I want to add some changes in calculate (on rule level - change with processing json_ext params and some processing of exceptions in case of lack of some params and in case of json_data (string)) and run_calculation_rule (on service level) with returning calculated result instead of list results.
https://openimis.atlassian.net/wiki/spaces/OP/pages/1730740234/Calculations+Module
runCalculationRules(instance, context)
for all calculation rules (send signal):
runCalculationRules(instance, context)
no return