[IMP] industry_real_estate: add meter invoicing - #2763
Conversation
|
|
99e0490 to
b045bde
Compare
- Improved related `x_property` views - Moved `x_distribution_keys` and related functionality from `condominium` to `property_assets_distribution` to support them in `industry_real_estate` - Added `x_meter_reading` demo data tasks-6318046
b045bde to
5118c54
Compare
vava-odoo
left a comment
vava-odoo
left a comment
There was a problem hiding this comment.
You should probably retarget to master (or 19.4 at least if really needed)
| <record id="automation_call_distribute_costs_on_update" model="base.automation"> | ||
| <field name="name">On Distribution input update</field> | ||
| <field name="model_id" ref="account.model_account_move"/> | ||
| <field name="action_server_ids" eval="[(6, 0, [ref('property_assets_distribution.ir_actions_server_distribute_costs')])]"/> |
There was a problem hiding this comment.
no need for self module ref
| <field name="action_server_ids" eval="[(6, 0, [ref('property_assets_distribution.ir_actions_server_distribute_costs')])]"/> | |
| <field name="action_server_ids" eval="[(6, 0, [ref('ir_actions_server_distribute_costs')])]"/> |
| <record id="ratios_multi_company" model="ir.rule"> | ||
| <field name="name">Ratios - Multi-Company</field> | ||
| <field name="model_id" ref="ratios_model"/> | ||
| <field name="model_id" ref="property_assets_distribution.ratios_model"/> |
There was a problem hiding this comment.
probably to move to common module as well, no?
There was a problem hiding this comment.
multicompany is a condominium only feature
| help="Used to distribute costs by meter; closest meter reading date used" | ||
| /> | ||
| <field name="x_period_end" | ||
| help="Used to distribute costs by meter; closest meter reading date used" |
There was a problem hiding this comment.
no it is optional, and we invoice based on latest reading
| <field name="model_id" ref="model_x_distribution_key"/> | ||
| <field name="name">x_based_on</field> | ||
| </record> | ||
| <record id="field_distribution_key_base_meter" model="ir.model.fields"> |
There was a problem hiding this comment.
since you restructured the file: maybe a pitty to put this one between a field and its field selection values
| # compute overlap end once | ||
| overlap_end = account.x_end_date if account.x_end_date else record.x_period_end | ||
| analytic_distribution[account.id] = ((overlap_end - overlap_start).days + 1) * ratio.x_ratio | ||
| elif account.partner_id.id == ratio.x_owner.id: analytic_distribution[account.id] = ratio.x_ratio |
There was a problem hiding this comment.
I don't remember what it was for exactly, but this case is not taken into account in your new action
There was a problem hiding this comment.
the if for this elif executes when both record.x_period_start and record.x_period_end exist. since record.x_period_start is required it always exists, meaning we enter the elif block iff record.x_period_end=False, if you look closely the entire block is inside an if block that checks for periods_valid which is true iff both dates exist.
In otherwords this elif is deadcode that never really runs so there is no reason to keep it.
| properties = record.invoice_line_ids.x_property_id or env['x_property'].search([('x_company_id', '=', dist_key.x_company_id.id)]) | ||
| for property in properties: |
| for property in properties: | ||
| readings = property.x_meter_reading_ids.filtered(lambda mr: mr.x_meter_id.id == dist_key.x_meter.id) | ||
| if len(readings) < 2: continue | ||
| for account in property.x_account_ids: |
There was a problem hiding this comment.
maybe for later but wouldn't it make sense to archive (or filter out) past accounts?
There was a problem hiding this comment.
filtering out would be a bad idea, we could invoice someone a short while after they leave when the meter bill comes. for archiving maybe we could add something for it but i think we should leave it for @Pierre-savp to decide.
| mrostart = findClosestMeterReading(readings, acc_start) | ||
| mroend = findClosestMeterReading(readings, acc_end) | ||
| if mrostart.x_date < mroend.x_date: | ||
| analytic_distribution[account.id] = abs(mroend.x_quantity - mrostart.x_quantity) |
There was a problem hiding this comment.
why not a += here as well?
| analytic_distribution[account.id] = abs(mroend.x_quantity - mrostart.x_quantity) | |
| analytic_distribution[account.id] = analytic_distribution.get(account.id, 0) + abs(mroend.x_quantity - mrostart.x_quantity) |
There was a problem hiding this comment.
because in python = inserts keys in the map if they dont exist but +=, -=, *=, ... dont so it will throw an error
| mrostart = findClosestMeterReading(account.x_start_date, meter_reading_ids) | ||
| mrostart_date = mrostart.x_date | ||
| billend_date = billend.x_date | ||
| if mrostart_date < billstart.x_date: mrostart = billstart |
There was a problem hiding this comment.
why did you remove this logic here?
There was a problem hiding this comment.
for each account we invoice from the closest meter reading to the start till the closes meter reading to the end, since all these account periods are within the record period any meter reading queries for any account will be within the meter reading taken for the account either way.
also according to @Pierre-savp it is the landlords responsibility to take readings between tenants switching so there should be no conflicts between accounts and bills
3 participants
tasks-6318046