Add plants:import for the ECHOcommunity plant migration - #117
Merged
Conversation
compare_and_sync read local state with record.attributes.slice(*source_attributes).
Mobility is configured with backend :container and without the attribute_methods
plugin, so translated attributes (description, uses, cultivation, ...) live in the
translations jsonb and never appear in #attributes. Slicing there returned {} for
them, so local always looked different from the stored snapshot.
Consequences, both verified by the added spec before the fix:
- an unchanged re-sync was scored locally_modified instead of synced
- a genuine upstream edit to a translated attribute was silently dropped -
applied stayed 0, no conflict was raised, and the value never changed
Translated fields therefore stopped syncing after record creation, silently.
The existing suite missed this because SOURCE_ATTRS is scientific_name and
family_names, both real columns, and the one spec passing %w[description]
exercises the invalid-payload path, which returns before compare_and_sync.
Read through the public reader instead, so column-backed and translated
attributes behave identically. Applied at both call sites.
Full suite: 2307 examples, 0 failures. Rubocop clean.
Imports plants exported from ECHOcommunity during the plant-data ownership
migration, consuming the same JSON shape as db/seeds/Plants.json so the export
tooling targets a format this application already understands.
Writes go through the model layer rather than SQL, so Mobility translations,
the visibility trio and PaperTrail behave normally. Idempotent: an existing
uuid is skipped, never overwritten - bringing a record up to date is
reconciliation, a separate concern with conflict handling.
Two things worth calling out:
- Absent ranges are written as NULL explicitly. These columns default to
values nobody measured (ph_range '[0.0,14.0]', optimal_temperature_range
'[0,61)', n_accumulation_range '[0,1)'), so omitting an absent range
records 'tolerates pH 0-14' rather than 'unknown'. That is how the 2020
seed left 174 of its 322 plants claiming exactly that.
- created_by_principal_id is NOT NULL and a 'real' organization such as ECHO
has no principal of its own, so it defaults to the service principal for
the owner address, matching how production attributes its seeded plants.
Verified against a copy of the 70 published plants currently missing from the
API: 70 created, all public, 177 common names, every range NULL where absent.
Full suite 2312 examples, 0 failures. Rubocop clean.
The export now carries each plant's ECHOcommunity status as a visibility, so a
draft is created as a draft rather than created public and hidden a moment
later. Anything unrecognised errs to draft: invisible is the safe direction.
Also fixes two spec bugs found while adding coverage. A bare hash in an
argument list is parsed as keyword arguments in Ruby 3, so record('k' => v)
passed zero positional args. And a memoised rake task cannot be invoked twice
in one example, so the task helper became a method that re-enables.
Full suite 2313 examples, 0 failures. Rubocop clean.
The previous commit added a Metrics/ClassLength section to .rubocop.yml to exclude the importer. That overrode the exclusions inherited from .rubocop_todo.yml and broke CI with six pre-existing offenses in files nobody had touched - source_synchronizer.rb, ownership_backfill.rb and four others. Local linting missed it because it only inspected lib and spec/tasks; CI inspects 734 files. Range parsing moves to lib/ec_range_parser.rb, which brings the importer under the limit honestly and puts a pure function somewhere reusable. Whole-repo rubocop: 730 files, no offenses. Suite 2313 examples, 0 failures.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Phase 1 of the plant-data ownership migration. Imports plants exported from
ECHOcommunity so the API can become the system of record for them.
Shape
Consumes the same JSON as
db/seeds/Plants.json, so the export tooling targetsa format this app already understands rather than a new one. Logic lives in
lib/ec_plant_importer.rbwith a thin rake wrapper, followingstaging_rehearsal.rake.ECHO_ORG_IDselects the owning organization — staging usesStagingRehearsalMapping::ECHO_ORG_ID.Writes go through the model layer, never SQL, so Mobility translations, the
visibility trio and PaperTrail all behave normally. Idempotent: an existing
uuid is skipped, never overwritten — bringing a record up to date is
reconciliation, which is a separate concern with conflict handling.
Two decisions worth reviewing
Absent ranges are written as NULL explicitly. These columns carry defaults
that assert facts nobody measured:
ph_range[0.0,14.0]optimal_temperature_range[0,61)n_accumulation_range[0,1)biomass_production_range[0.0,0.0]optimal_rainfall_range,optimal_altitude_range[0,)Omitting an absent range therefore records "tolerates pH 0–14" rather than
"unknown". That is how the 2020 seed left 174 of its 322 plants claiming
exactly that, and it is worth knowing those rows are still in production. There
is a spec pinning this behaviour.
created_by_principal_id. It is NOT NULL, and arealorganization such asECHO has no principal of its own — only
personalorgs do. Productionattributes its 322 seeded plants to the service principal for
echo@echonet.org, so that is the default here, overridable withECHO_PRINCIPAL_ID.Verification
Run against a copy of the 70 published ECHOcommunity plants currently absent
from the API: 70 created, all public and not deleted, 177 common names, and
every range NULL where the source had no value. Field mapping spot-checked
against source — the baobab's
resource.description(1,376 chars),plant.description(2,219) andplant.uses(2,167) land indescription,info_sheet_descriptionandusesrespectively.idempotency and the dry-run default
plant_import.rakeadded to theMetrics/BlockLengthexcludelist alongside
ownership.rake)Depends on nothing else; #116 is independent.