Fix erroneous purchase category definitions #500
Conversation
|
@madsnedergaard FYI I found two other issues when adding unit tests for this. :) |
|
FYI: We also need to migrate existing activities with incorrect identifier :) |
Yes, I believe this is covered in https://github.com/tmrowco/bloom/issues/292#issuecomment-748224152 |
| 'Basic Expenses': PURCHASE_CATEGORY_FOOD, | ||
| 'Basic Expenses': null, |
There was a problem hiding this comment.
Why remove this one? It should point to Food, which is in footprints.yml?
There was a problem hiding this comment.
The test didn’t find a carbon intensity. I’ll check. :)
There was a problem hiding this comment.
So the issue seems to be that https://github.com/tmrowco/bloom-contrib/blob/master/co2eq/purchase/index.js#L51 only adds children... It means that we end up with a "Food" entry without any intensityKilograms. 🤔
There was a problem hiding this comment.
I would expect getEntryByKey('Food') to return an entry with a intensityKilograms... Not sure if this is a regression from #488 or it is working as intended.
There was a problem hiding this comment.
Huh, so it wouldn't be able to run the model with a "parent" identifier?
There was a problem hiding this comment.
Works in production right now despite #488 being merged 🤔
There was a problem hiding this comment.
Hm.
It seems like the purchase model is doing a smarter lookup for the entry and carbon intensity than doing getEntryByKey('Food'). This would means, that the test here is insufficient.
There was a problem hiding this comment.
I've verified that the activity you linked to, has the Food as identifier. And the carbonModelName is purchase version 9.
There was a problem hiding this comment.
Turned out to be a Jest caching issue: yarn test --clearCache did the job. Thanks @madsnedergaard !
Ah, nice - I'll do that migration on prod-database now :) EDIT Done! |
This reverts commit 0952e23.
| @@ -94,12 +92,12 @@ const idToCategory = { | |||
| 248: PURCHASE_CATEGORY_STORE_PERSONAL_CARE, // Cosmetics | |||
| 235: PURCHASE_CATEGORY_STORE_PERSONAL_CARE, // Hairdresser | |||
|
|
|||
| 161: PURCHASE_CATEGORY_HOUSING, // - Home | |||
| 161: null, // - Home (not sure about the subcategory in PURCHASE_CATEGORY_HOUSING) | |||
There was a problem hiding this comment.
I have verified that this was not affected by my Jest caching issue. We do not have a carbon footprint for PURCHASE_CATEGORY_HOUSING (HOUSING, WATER, ELECTRICITY, GAS AND OTHER FUELS).
Description
The identified purchaseCategories correspond to identifiers that don't hold any emission factors. Therefore when bloom activities pointing to these purchaseCategories are created, they won't have any carbon footprint
Misc
For more context, see this issue