fix(i18n): align access-grants locale keys with en source#547
Closed
will-yuponce-db wants to merge 14 commits into
Closed
fix(i18n): align access-grants locale keys with en source#547will-yuponce-db wants to merge 14 commits into
will-yuponce-db wants to merge 14 commits into
Conversation
Add a merge_group trigger to the Test Coverage workflow so the five required status checks (Backend Tests, Frontend Tests, TypeScript Type Check, Requirements Lockfile Check, Frontend Build Check) report on the gh-readonly-queue branch GitHub builds for each queued batch. Without it, PRs entering a merge queue on main/develop would hang waiting on checks that never run. Also extend the lockfile drift gate to hard-fail under merge_group: the auto-commit fixup is PR-only (it cannot push to the read-only queue branch), so the queue must fail on drift rather than skip the check. Co-authored-by: Isaac Co-authored-by: Will Yuponce <will.yuponce+data@databricks.com>
The collection picker was disabled whenever a default collection prop was passed, which the business terms view always does. As a result users could never switch away from the first editable collection. Disable the picker only when there is at most one editable collection, treating the prop as a default seed value.
A data contract created via POST /api/data-contracts was saved with no owner: create_contract_with_relations never set draft_owner_id and only set owner_team_id when an owner was supplied. Under the PRD #442 role-aware visibility filter a family is "elevated" only for its draft_owner or owner_team members, so an owner-less draft was invisible to its own creator (and the create dialog showed no row in the list). Additionally GET /api/data-contracts had no `status` query parameter, so `?status=draft` was silently ignored by FastAPI. Changes: - Stamp the creator as draft_owner_id when a draft is created without an owning team, mirroring the existing personal-draft clone path. - Clear draft_owner_id on the draft->proposed transition so a contract under steward review is promoted from creator-only to org visibility. - Add a real `status` filter to GET /api/data-contracts, applied after the visibility filter so it can only narrow what a caller may see. - Unit tests for the status filter and personal-draft owner visibility. Fixes regression ONT-CUJ-006. Co-authored-by: Will Yuponce <will.yuponce+data@databricks.com>
Quality rules authored on a schema column (the schema editor's per-column
"Quality" tab) were accepted in the dialog but silently dropped on save:
GET /api/data-contracts/{id} came back with schema.quality=[] and the
column carried no quality field, so ODCS export omitted the rules.
Root cause: column rules arrive nested under properties[].quality, but
_create_schema_objects (used by both create and update) only ever
persisted the property row itself and never read its quality list. The
only quality persistence path was the separate object-level qualityRules
field. ColumnProperty also didn't declare `quality`, relying on
extra="allow" to keep it.
Changes:
- Declare `quality: List[QualityRule]` on the ColumnProperty API model.
- Extract the rule->DataQualityCheckDb mapping into a shared
_build_quality_check_db helper that sets property_id for column rules.
- Persist properties[].quality in _create_schema_objects, bound to both
object_id and property_id.
- On update, scope the object-level qualityRules replacement to
property_id IS NULL so it no longer deletes the column rules persisted
by the schema recreation in the same save.
- Unit tests for column-rule persistence and object/property coexistence.
Fixes regression ONT-CUJ-008.
Co-authored-by: Will Yuponce <will.yuponce+data@databricks.com>
…iew stub (#529) request_steward_review constructed an in-memory ReviewedAsset, logged "Created asset review record", and never persisted it — misleading dead code that made it look like a steward review record was created when none was. A proposed contract was therefore only discoverable by direct URL. Proposed contracts actually reach stewards through two real paths that already exist: - GET /api/approvals/queue lists contracts in proposed/under_review status, where the steward approves/rejects them. - The ON_REQUEST_REVIEW workflow trigger runs configured review workflows. The /data-asset-reviews surface targets Unity Catalog assets and requires an explicit reviewer; routing contracts there needs a steward-assignment policy that does not exist yet. Changes: - Remove the unsaved ReviewedAsset stub and its false success log (and the now-unused AssetType/ReviewedAssetStatus import). - Document the two real surfacing paths inline. - Add tests: requesting review transitions draft->proposed and the contract then appears in the approvals queue; review from a non-draft status is rejected. Addresses regression ONT-CUJ-012 (with contract-list discoverability also improved in the draft-owner-visibility fix). Co-authored-by: Will Yuponce <will.yuponce+data@databricks.com>
POST /api/data-contracts/{id}/approve returned HTTP 500 ("Failed to
approve contract") for a proposed contract, leaving it stuck in
"proposed". The approve endpoint is documented as accepting
PROPOSED/UNDER_REVIEW -> APPROVED and pre-checks that source status, but
DATA_CONTRACT_TRANSITIONS only listed ["draft", "under_review",
"deprecated"] for "proposed" — "approved" was missing. transition_status
therefore raised ValueError("Invalid status transition"), which the route
caught with a bare `except Exception` and reported as a 500.
Changes:
- Add "approved" to the allowed transitions from "proposed" so a steward
can approve directly (under_review stays an optional intermediate step).
- In the approve and reject routes, catch ValueError and return 409 with
the message instead of an opaque 500, so any invalid transition is a
clean client error.
- Tests: lifecycle map allows proposed->approved; transition_status
proposed->approved succeeds; a genuinely invalid jump still raises.
Fixes regression ONT-CUJ-013.
Co-authored-by: Will Yuponce <will.yuponce+data@databricks.com>
…ins (#531) Adding a deliverable to a data product failed with PUT 403 for both the Producer who created it and an Admin, because: 1. create_product never assigned an owner. A new product had no project_id, owner_team_id, or draft_owner_id, so the update authorization cascade failed for everyone except a workspace admin — the creator couldn't edit their own product, and it could never be assigned an owner since that also requires PUT. 2. The update ownership cascade only short-circuited on workspace-admin (group based), ignoring the caller's feature-level data-products permission. An in-app role override granting data-products=Admin was therefore denied when the caller wasn't the object owner. 3. GET /api/delivery-methods required the delivery-methods feature, which a Producer lacks, so the Add Deliverable delivery-method dropdown was empty (403). Changes: - create_product stamps the creator as draft_owner_id when no project, team, or draft owner is supplied (mirrors the personal-draft clone). - update_product_with_auth accepts is_feature_admin and short-circuits on it; the update route resolves the caller's effective data-products permission (honoring in-app role overrides) and passes it in. - Gate the delivery-methods read endpoints on data-products:READ_ONLY (reading the catalog is a prerequisite for authoring deliverables); writes stay on delivery-methods:READ_WRITE. - Tests: creator becomes draft owner; explicit team owner not overwritten; feature-admin can edit orphan/others' products; non-admin still fail-closed. Fixes regression ONT-CUJ-015. Co-authored-by: Will Yuponce <will.yuponce+data@databricks.com>
…tion (#532) POST /api/data-products/{id}/request-certify returned 202 Accepted for a product with zero deliverables (no output ports), letting an empty product into the certification workflow. The documented "at least one deliverable required" guard was never enforced. Changes: - Block request-certify with 409 when the product has no output ports. - Tests: request-certify without a deliverable returns 409; with a deliverable returns 202. Fixes regression ONT-CUJ-019 and ONT-NEG-008. Note: the request-certify endpoint intentionally uses a request-based model (it records a certification request and fires the on_request_certify workflow) rather than directly transitioning Draft->Proposed. The separate /submit-certification endpoint performs the draft->proposed transition. This change only adds the missing deliverable precondition; it does not alter the request-vs-transition model. Co-authored-by: Will Yuponce <will.yuponce+data@databricks.com>
…in (#533) Creating a data contract whose name duplicated an existing contract in the same domain succeeded silently (POST 200, duplicate persisted) — no 409, no inline error. create_contract_with_relations always creates a *new* version family (new versions go through create_version), so any existing contract sharing the same name+domain is a genuine duplicate rather than another version of the same family. Add a case-insensitive name + domain check that raises ConflictError (surfaced as HTTP 409 by the create route, matching the existing duplicate-id behavior). Same name in a different domain is still allowed. Tests: duplicate name in the same domain raises ConflictError (incl. case-insensitive); a distinct name still creates successfully. Fixes regression ONT-NEG-002. Co-authored-by: Will Yuponce <will.yuponce+data@databricks.com>
…1) (#535) GET /api/data-products/{id} returned the full product to any caller with data-products:READ_ONLY, with no status or ownership check. A Data Consumer could therefore read a draft (uncertified) product directly by id, even though the marketplace listing correctly hides drafts. Add a read gate to the get-by-id route: - Published products (active/deprecated) remain readable by anyone with READ_ONLY — the catalog/marketplace contract. - Unpublished products (draft/proposed/under_review/approved/...) are readable only by data-products admins (incl. via in-app role override) and owners (draft_owner / owning team / project member), reusing the same ownership scope as the listing. - A denied read returns 404 (not 403) so the draft's existence isn't disclosed. Tests cover the gate directly: published readable by all; draft denied for a non-owner consumer; draft allowed for a feature admin and for an owner in the accessible set. Fixes regression ONT-NEG-011. Co-authored-by: Will Yuponce <will.yuponce+data@databricks.com>
…2F%2F path collapse (#536) * fix(concepts): add /by-iri query-param routes to survive %2F%2F path collapse The Databricks Apps proxy decodes and collapses `%2F%2F` to a single `/` in path segments, mangling IRIs like `http://ontos.example.org/...` into `http:/ontos.example.org/...` before they reach FastAPI. This broke every concept detail page (`/concepts/browser/{iri}`) with "Concept not found" regardless of which concept was clicked. Query-string values survive that transformation unchanged. Existing sibling endpoints (`/concepts/hierarchy?iri=`, `/neighbors?iri=`) already work via the query-param shape — this PR aligns the rest of the `{concept_iri:path}` route family on the same pattern. Approach: - Add new `/concepts/by-iri?iri=<urlencoded>` (semantic-models) and `/knowledge/concepts/by-iri?iri=<urlencoded>` (knowledge) route families. - Keep the existing path-form routes as deprecated aliases for one migration window so external bookmarks don't 404. Both shapes share module-level `_*_payload` helpers — no duplicated logic. - Frontend (`concept-detail.tsx`, `node-links-panel.tsx`, `concepts-search.tsx`) switched to the query-param form. - Backend integration tests cover both shapes for the read path and at least one mutating path. - Drive-by fix: `get_concept_hierarchy` referenced an undefined name `concept_iri` in its exception logger (parameter is named `iri`). Routes declared in `by-iri`-first order — FastAPI matches in declaration order and the path-form parameter is greedy, so the reverse order would shadow the new routes. Co-authored-by: Isaac * test(concepts): correct lifecycle smoke test for /by-iri routes Direct draft → published transition is rejected by the manager (must go through under_review first), so swap the lifecycle smoke test to use the submit-review action — which is the right shape for proving the /by-iri/<action> routes are wired without re-testing the state machine. Co-authored-by: Isaac
POST /api/compliance/policies was sharing the same Pydantic CompliancePolicy model with PUT, which marks id as required. Pydantic rejected every UI-driven create with 422 "field required" before the route handler could even generate a UUID. This commit splits the create contract from the read/update model: - New CompliancePolicyCreate model with only the fields a client must supply (name, description, rule, failure_message, is_active, severity, category). - POST /api/compliance/policies now accepts CompliancePolicyCreate. - ComplianceManager.create_policy generates the UUID server-side via uuid4(). - Defensive secondary fix per #235: the read-side CompliancePolicy.compliance field gets default=0.0 so stale rows and migrations cannot 500 on read. - PUT contract is unchanged on purpose; the bug was create-only. No DB schema change is required — compliance_policies.id is already a string primary key with no DB-level default, and the manager continues to supply the UUID explicitly. Unit + integration tests updated to match the new contract. Closes #235 Co-authored-by: Isaac
Internal/computed named graphs (urn:app-entities, urn:demo, urn:semantic-links, urn:meta:sources, the rdflib default graph) were leaking into the RDF Sources list once the in-memory graph contained triples (e.g. after the first ontology upload with a Data Contract present). These pseudo-rows had no backing file, so their Preview action 404'd with "Failed to load content." Filter internal contexts at the /api/semantic-models boundary and derive the frontend table filter from the canonical SYSTEM_RDF_NAMESPACE_KEYS map so the two stay in sync. Caching and Knowledge Graph stats are intentionally left untouched.
Audited all locale namespaces against the en reference. Fixes the
correctness defects in access-grants (es/fr/it/ja):
- panel.revokeConfirm dropped the {{user}} interpolation present in en,
so the revoke-confirmation dialog could not show the affected user in
those languages. Restored {{user}} in each translation.
- Removed orphan keys not present in the en source and unreferenced
anywhere in the codebase: panel.columns.* (user/permission/granted/
expires/actions), request.cancel, request.error. These keys are
unreachable (i18next resolves by the keys the code requests) and only
added drift vs the en source.
Note: the larger count of untranslated keys/namespaces in non-en locales
is handled by i18next fallbackLng:'en' and is intentional fallback, not a
defect; this PR does not add machine translations.
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
3 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
Audited every locale namespace under
src/frontend/src/i18n/localesagainst theenreference (JSON validity, namespace parity, key parity, interpolation-placeholder parity). All files are valid JSON. This PR fixes the genuine correctness defects found inaccess-grantsfor es/fr/it/ja:panel.revokeConfirmin en is"...revoke access for {{user}}?", but the four translations had dropped the{{user}}interpolation ("...revoke this access permission?"), so the revoke-confirmation dialog could not name the affected user in those languages. Restored{{user}}in each.panel.columns.*(user/permission/granted/expires/actions),request.cancel,request.errorexisted in es/fr/it/ja but not in theensource and are not referenced anywhere in the codebase (grep = 0 hits). Since i18next resolves by the keys the code requests, these are unreachable and only add drift; removed for consistency with en.Scope / non-goals
The audit also shows non-en locales are missing many keys and 6 namespace files vs en. That is untranslated content, handled at runtime by
i18nextfallbackLng: 'en'(seesrc/i18n/config.ts) — functionally correct, not a defect. This PR deliberately does not add machine-generated translations.If
panel.columns.*were intended for a planned grants-table feature, the correct fix is to add them toenfirst (and wire them in code) rather than leave them only in non-en locales — happy to do that instead if preferred.Testing