fix(farmer_demo): assign FARM group type to farm demo data (#1120) - #322
fix(farmer_demo): assign FARM group type to farm demo data (#1120)#322emjay0921 wants to merge 4 commits into
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 19.0 #322 +/- ##
==========================================
- Coverage 72.24% 67.13% -5.11%
==========================================
Files 419 100 -319
Lines 29813 9814 -19999
==========================================
- Hits 21539 6589 -14950
+ Misses 8274 3225 -5049
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
gonzalesedwin1123
left a comment
There was a problem hiding this comment.
Thanks — this one is in good shape. The fix is correct and well-scoped: _ensure_farm_group_type() mirrors the existing cooperative helper (ADR-016 get_or_create_local — signature checked against spp_vocabulary/models/vocabulary_code.py:468), cooperative typing already existed so leaving _create_cooperatives untouched is right, and the tests cover all four surfaces — story farms, the _create_farm helper, the seeded farms, and the container-vs-member split (nice that test_1120_cooperative_container_typed_cooperative walks the memberships and asserts members stay farm).
One blocking item, and it's convention rather than breakage:
1. Version bump
spp_farmer_registry_demo stays at 19.0.2.1.1, and the module is in the 2026.08 release tag. Since this is pure Python in a demo generator (no data files, views, or schema), nothing actually breaks without an upgrade — but the repo rule applies (same-version changes give upgraders no signal), and #441 will eventually enforce it mechanically. One-line fix.
2. Nits (non-blocking, take or leave)
_ensure_farm_group_type()is called inside_create_farmfor each farm — an uncachedget_or_create_localsearch × 8 story farms. The module already demonstrates both better patterns: hoisted once in_create_cooperatives(cooperative_type_id, line ~1280) and cached in the seeded generator (self._farm_group_type_id). Hoisting or caching would make the three sites consistent.- Existing demo databases keep their blank-typed farms — no backfill, which seems fine for demo data as long as QA regenerates rather than expecting the fix to repair old data. Worth a line on the ticket confirming that expectation.
- The broad
except Exception → warning → blank typemirrors the cooperative helper, so consistency wins here — and the new assertions would catch a systemic failure in CI, which makes the silent fallback acceptable.
With the bump added I'm happy to approve.
…er run Version bump with its changelog entry, per the convention that bumps land in the PR. Nothing malfunctions without an upgrade — this is Python in a demo generator — but a same-version change gives upgraders no signal. The group-type lookup ran once per story farm. It is now resolved once before the loop and passed down, with the per-farm lookup kept as the fallback for direct calls such as the tests. Caching it on the recordset the way the seeded generator does is not available here: that one is a plain utility class, while this is a TransientModel, and recordsets use __slots__ — the first attempt at this raised AttributeError across nine tests.
Version bump with its changelog entry, per the convention that bumps land in the PR. The module now has three PRs in flight, so the concrete number is a merge-time reconciliation: #412 carries .2, this takes .4 and #322 takes .3. USE_CASES.md is the document this blueprint exists to serve, and it had no EC1 row while its totals still read 21 blueprints and ~730 farms — both undercounts as soon as this lands. The row is added and the totals now read 22 and ~731.
|
Thanks — both done. Pushed as 1. Version bump — 2. The per-farm lookup — hoisted, and the attempt at your other suggestion is worth recording. I first cached it on the recordset the way 3. Existing demo databases — confirmed, no backfill: farms already created keep their blank type, and the expectation is that QA regenerates rather than upgrading in place. I have put that on OP#1120 so it is on the ticket rather than only here.
|
#412 landed the same module's 19.0.2.1.2, so the manifest version and the top of the changelog both conflicted. Resolved by keeping this branch's 19.0.2.1.3 and both changelog entries, newest first; README.rst and the description page are generated from readme/, so they were rebuilt rather than merged by hand.
Why is this change needed?
The farmer demo data generator created farm groups with a blank Group Type (OP#1120). Farm groups should default to Farm, and cooperative container groups to Cooperative. Neither generator set
group_type_id, and nofarmcode existed in theurn:openspp:vocab:group-typevocabulary (onlycooperativewas created at runtime).How was the change implemented?
_ensure_farm_group_type()(mirrors the existing_ensure_cooperative_group_type()) to create a localfarmgroup-type code viaget_or_create_local— in both the story-farm wizard and the seeded-farm utility.group_type_id = farmon every farm group created by both generators (story farms in_create_farm, and the ~730 seeded/volume farms).group_type_id = cooperative; member farms stay Farm (a farm is still a farm inside a cooperative — only the container is a Cooperative).New unit tests
test_1120_cooperative_container_typed_cooperative— the cooperative container is typedcooperativeand its member farms are typedfarm.group_type_id.code == "farm"assertions to the seeded-farm generator test, the story-farm group test, and the_create_farmhelper test.Unit tests executed by the author
spp_farmer_registry_demosuite: 0 failed, 0 error(s) of 133 tests (fresh DB,--stop-after-init).How to test manually
Related links
https://projects.acn.fr/work_packages/1120