feat(ogc): filter ogc_* views to public records (BDMS-971 A1)#782
Open
ksmuczynski wants to merge 7 commits into
Open
feat(ogc): filter ogc_* views to public records (BDMS-971 A1)#782ksmuczynski wants to merge 7 commits into
ksmuczynski wants to merge 7 commits into
Conversation
test_get_project_area assumed its "Test Group Foo" fixture would always get database id 1, which only held because nothing else in the suite created a Group row first. Any earlier-running test that inserts a Group (e.g. a data-migration test) shifts the sequence and breaks this test with an unrelated 404. Use the fixture's actual group.id instead of the literal 1.
pyproject.toml's explicit setuptools packages list omitted data_migrations, so the installed `oco` CLI couldn't import it at all -- `oco data-migrations status/run` failed with ModuleNotFoundError for every migration, not just new ones. Running via `uv run pytest`/`python -c` masked this because pytest prepends the repo root to sys.path; the installed console-script entry point does not.
The upcoming ogc_* view filter excludes non-public records, and nine currently-passing tests in test_ogc.py assert that rows backed by the shared location/water_well_thing/group fixtures ARE present in ogc_* responses. Their "draft" default was an arbitrary safe value, not a deliberate test input -- confirmed by grepping the suite for anything that depends on it being specifically "draft". Flips the three fixtures plus one inline Group(...)construction in test_ogc.py that bypassed the fixture.
Every ogc_* view/materialized view selected release_status but never filtered on it, so the unauthenticated /ogcapi endpoints have been serving private and draft records alongside public ones (e.g. water_wells: 1,145 private + 140 draft rows next to 8,678 public). Adds "AND release_status = 'public'" to all 21 existing ogc_* relations and introduces ogc_locations (previously the locations collection pointed straight at the raw location table, which has no filter at all). ogc_actively_monitored_wells gets no predicate of its own -- it already inherits public-only rows transitively through its join to ogc_water_well_summary; adding a redundant filter on status_history.release_status would repeat a mistake already made and reverted once (see w1x2y3z4a5b6), since that column is never populated by the transfer scripts. Fully reversible: downgrade() rebuilds the same relations with the byte-identical unfiltered SQL that was in production before this migration (ogc_locations is the exception, since it didn't exist pre-migration -- downgrade drops it rather than recreating it unfiltered). Repoints core/pygeoapi-config.yml's locations provider at the new view in the same commit, since shipping the view without the config change (or vice versa) leaves the collection either unfiltered or broken. Layer visibility (which collections are published)is unchanged. That's separate, out-of-scope work.
ogc_project_areas is now filtered to public records, but all 56 current project_area-bearing group rows are release_status=draft (confirmed in docs/ogc-layer-audit.md), which would make the layer return zero rows until someone flips them. Uses the data_migrations framework (not Alembic, which is schema-only in this repo) so the change is tracked, skip-if-applied, and gated on this ticket's schema revision having landed first. Runs independently via `oco data-migrations run 20260714_0001_publish_project_areas` -- deliberately not swept in via run-all, which could also apply other unrelated pending migrations. No downgrade: this framework is forward-only, and a blanket revert couldn't tell rows this migration published apart from ones published independently afterward -- undoing it later means writing a new, deliberate migration instead. This is a genuine publication decision, not a mechanical schema change -- confirm the 56 rows are actually appropriate for public release before merging.
Covers the migration-application, downgrade-reversibility, already-consistent-layers, and project_areas scenarios in ogc-cleanup-sprint1.feature tagged @A1. The other ~10 tickets sharing that feature file have no steps yet and stay undefined, per plan. Tags @production only on these specific scenarios (the feature file has no feature-level tag since ~20 other tickets' scenarios share it) and @migration-mutates-schema on the three that call alembic downgrade, wiring a before/after_scenario hook in environment.py that unconditionally restores head so a downgrade left mid-scenario can't leak into later scenarios sharing this database.
ksmuczynski
requested review from
TylerAdamMartinez,
chasetmartin,
jacob-a-brown and
jirhiker
July 15, 2026 22:54
TylerAdamMartinez
approved these changes
Jul 16, 2026
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
2 participants
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.
Summary
Restricts every
ogc_*view/materialized view behind the public,unauthenticated
/ogcapimount torelease_status = 'public', and flips the 56project_areasrows (currently alldraft) topublic, so the newly-filteredogc_project_areasview isn't left returning zero rows.Why
This PR addresses the following problem / context:
None of the 22
ogc_*relations filtered onrelease_status. They selected it, but never gated on it. Confirmed against live data:water_wellsalone was serving 1,145 private + 140 draft rows alongside 8,678 public ones, andproject_areaswas 100% draft (56/56).Changes
Implementation summary - the following was changed / added / removed:
AND release_status = 'public'to all 21 existingogc_*relations, and introducesogc_locations(previously thelocationscollection pointed straight at the rawlocationtable with no filter at all).ogc_actively_monitored_wellsgets no predicate of its own — it already inherits public-only rows transitively through its join toogc_water_well_summary. Fully reversible:downgrade()restores the byte-identical unfiltered SQL that was in production (ogc_locationsis the one exception. I didn't exist pre-migration, so downgrade drops it instead).project_areaspublish migration: companion data migration (run viaoco data-migrations run, not Alembic) flips the 56project_areasgroup rows fromdrafttopublic. Forward-only by design, gated on this PR's schema revision having landed first.location/water_well_thing/groupfixtures fromdrafttopublic, since 9 existing tests assert those rows are visible throughogc_*responses.@A1-tagged cases inogc-cleanup-sprint1.feature(migration application, downgrade reversibility, already-consistent layers, project_areas count).data_migrationspackaging:pyproject.toml's packages list was missingdata_migrations, so the installedocoCLI couldn't import it at all. Pre-existing gap, unrelated to this ticket's logic, found while verifyingoco data-migrations run.test_get_project_areaassumed its fixture'sGroupwould always get database id1, which only held because nothing else in the suite created aGroupfirst. Surfaced by this PR's new data-migration test.Verification
pytestsuite: 716 passed, 0 failed.alembic downgrade -1/upgrade headcycle on a local test DB, withpsqlinspection confirming the filter predicate lands in the right place (including the innermost CTE for materialized views, not the outer query, per the DDL-ordering constraint forogc_actively_monitored_wells→ogc_water_well_summary).oco data-migrations run/statusround-trip for the project_areas migration.behave --tags=@A1— 5/5 scenarios passing./ogcapi/collections/*/items.Notes
Any special considerations, workarounds, or follow-up work to note?
project_areasrows this PR publishes are actually appropriate for public release — the data migration trusts that this review is where that confirmation happens.SELECT release_status, count(*) FROM "group" WHERE project_area IS NOT NULL GROUP BY release_statusreadsdraft: 56before andpublic: 56after, on the real target database./ogcapi-internalmount for staff who need unfiltered access) is a separate ticket/PR/branch that depends on this one merging first.