Skip to content

feat(forms): assign compliance forms to owners of a specific type#17787

Open
acrylJonny wants to merge 5 commits into
masterfrom
jd-form-owner-types-oss
Open

feat(forms): assign compliance forms to owners of a specific type#17787
acrylJonny wants to merge 5 commits into
masterfrom
jd-form-owner-types-oss

Conversation

@acrylJonny

Copy link
Copy Markdown
Collaborator

Motivation

Compliance forms can already be assigned to entity owners, but every owner gets every form regardless of ownership type. Teams that distinguish technical, business, data steward (etc.) owners on the same entity have no way to route a given form to the right cohort.

This PR adds an optional ownershipTypes list on FormActorAssignment that narrows owner-based assignment to owners whose ownership type matches the form's filter. Empty / unset preserves today's behaviour.

What changed

Layer Change
Model FormActorAssignment.ownershipTypes: optional array[Urn] — searchable, relationship-mapped to the ownershipType entity.
Java util OwnershipUtils.isOwnerOfEntityWithType mirrors OwnerServiceUtils.isOwnerEqual's legacy-enum fallback so owners persisted with only the deprecated Owner.type enum (no typeUrn) still match.
Service FormService.isFormAssignedToUser calls into it; tests cover both typeUrn-only and enum-only paths.
Patch builder FormInfoPatchBuilder.{add,remove}AssignedOwnershipType. Folds the existing user/group add/remove paths onto a shared helper (also removes the copy-paste error message bug in the SaaS-side setAssignedGroups).
GraphQL FormActorAssignment.ownershipTypes (typed), FormActorAssignmentInput.ownershipTypes (create-form), FormActorAssignmentUpdateInput.ownershipTypes{ToAdd,ToRemove} (update). Wired through FormMapper, FormUtils, UpdateFormResolver, and the loadable-type batch resolver in GmsGraphQLEngine.
Frontend useIsUserAssigned reads formAssociation.form.info.actors.ownershipTypes and filters owners; formFields fragment fetches the new block.
Python SDK Actors.ownership_types on the YAML recipe model (so the field documented under actors: is actually accepted by Pydantic), plus FormPatchBuilder.{add,remove}_assigned_ownership_type.
Docs forms.md + compliance-forms guides document the new field and the built-in ownership-type URNs.

Behaviour

# in a Forms recipe
actors:
  owners: true
  ownership_types:
    - urn:li:ownershipType:__system__technical_owner
    - urn:li:ownershipType:__system__data_steward

A user is treated as assigned only if (a) they're explicitly named, or (b) they own the entity with one of the listed ownership types. Empty / unset list preserves the existing "any owner is assigned" behaviour.

Tests

  • OwnershipUtilsTest — covers no-filter, single-type, multi-type, no-match, group-membership, and the enum-only legacy-owner fallback.
  • FormServiceOwnershipTest — end-to-end test that isFormAssignedToUser returns the expected result across all of those combinations against mocked FormInfo + Ownership aspects.
  • FormInfo/__tests__/utils.test.ts — covers useIsUserAssigned for typed and untyped ownership filtering.

Checklist

  • PR conforms to the Contributing Guideline (especially PR Title Format)
  • Tests added/updated
  • Docs updated
  • Breaking changes: none — the new field is optional with a backward-compatible default.

Notes

Originally proposed as #7076 (closed); this is a clean revival rebased against current master.

Made with Cursor

acrylJonny and others added 2 commits June 5, 2026 14:18
Compliance forms can already be assigned to entity owners, but every owner
gets every form regardless of ownership type. Teams that distinguish
technical, business, data-steward (etc.) owners on the same entity have no
way to route a given form to the right cohort.

This PR adds an optional `ownershipTypes` list to `FormActorAssignment` that
narrows owner-based assignment to owners whose ownership type matches the
form's filter. Empty / unset preserves today's behaviour.

Stack:
- Model: `FormActorAssignment.ownershipTypes: optional array[Urn]`,
  searchable + relationship-mapped to the `ownershipType` entity.
- Java: `OwnershipUtils.isOwnerOfEntityWithType` mirrors
  `OwnerServiceUtils.isOwnerEqual`'s legacy-enum fallback so owners persisted
  with only the deprecated `Owner.type` enum (no `typeUrn`) still match.
  `FormService.isFormAssignedToUser` calls into it; tests cover both
  `typeUrn`-only and enum-only paths.
- Patch builder: `FormInfoPatchBuilder.{add,remove}AssignedOwnershipType`,
  and the existing user/group add/remove paths are folded onto a shared
  helper that takes the field name (removes copy-paste error in the original
  `setAssignedUsers`/`setAssignedGroups` error message).
- GraphQL: `FormActorAssignment.ownershipTypes` (typed), plus
  `FormActorAssignmentInput.ownershipTypes` (create-form) and
  `FormActorAssignmentUpdateInput.ownershipTypes{ToAdd,ToRemove}` (update),
  wired through `FormMapper`, `FormUtils`, `UpdateFormResolver`, and the
  loadable-type batch resolver in `GmsGraphQLEngine`.
- Frontend: `useIsUserAssigned` reads `formAssociation.form.info.actors.ownershipTypes`
  and filters owners accordingly; `formFields` fragment fetches the new
  `ownershipTypes { urn type info { name description } }` block.
- Python SDK: `Actors.ownership_types` on the YAML recipe model (so the
  field documented under `actors:` is actually accepted), plus
  `FormPatchBuilder.{add,remove}_assigned_ownership_type`.
- Docs: `forms.md` + compliance-forms guides document the new field and the
  built-in ownership-type URNs.

Tests:
- `OwnershipUtilsTest` — covers no-filter, single-type, multi-type, no-match,
  group-membership, and the enum-only legacy-owner fallback.
- `FormServiceOwnershipTest` — end-to-end test that
  `isFormAssignedToUser` returns the expected result across all those
  combinations against mocked `FormInfo` + `Ownership` aspects.
- `FormInfo/__tests__/utils.test.ts` — covers the `useIsUserAssigned`
  helper for typed and untyped ownership filtering.

Co-authored-by: Cursor <cursoragent@cursor.com>
- Accept Maybe<Owner[]> (Owner[] | null | undefined) in isAssignedToForm
  and isOwnerWithType to match the GraphQL-generated entityData.ownership
  shape. Without this the FE type-check fails on the call site that
  passes entityData?.ownership?.owners.
- Replace the SaaS-only utils.test.ts (which imported non-exported
  helpers and a missing mocks module) with a focused
  useIsUserAssigned.test.ts that exercises only the new behavior:
  isAssignedToMe, owner-assignment, and ownership-type filtering.
  Mocks now satisfy the required Owner.associatedUrn field.

Fixes the datahub-web-react-lint CI job on #17783.

Co-authored-by: Cursor <cursoragent@cursor.com>
@github-actions github-actions Bot added ingestion PR or Issue related to the ingestion of metadata docs Issues and Improvements to docs product PR or Issue related to the DataHub UI/UX devops PR or Issue related to DataHub backend & deployment labels Jun 5, 2026
@alwaysmeticulous

alwaysmeticulous Bot commented Jun 5, 2026

Copy link
Copy Markdown

✅ Meticulous spotted 0 visual differences across 1554 screens tested: view results.

Meticulous evaluated ~10 hours of user flows against your PR.

Expected differences? Click here. Last updated for commit 0c6138a Merge remote-tracking branch 'origin/jd-form-owner-types-oss' into jd-fo.... This comment will update as new commits are pushed.

@datahub-connector-tests

datahub-connector-tests Bot commented Jun 5, 2026

Copy link
Copy Markdown

Connector Tests Results

All connector tests passed for commit 6286089

View full test logs →

To skip connector tests, add the skip-connector-tests label (org members only).

Autogenerated by the connector-tests CI pipeline.

@maggiehays maggiehays added the needs-review Label for PRs that need review from a maintainer. label Jun 5, 2026
@codecov

codecov Bot commented Jun 17, 2026

Copy link
Copy Markdown

❌ 1 Tests Failed:

Tests completed Failed Passed Skipped
23819 1 23818 194
View the top 1 failed test(s) by shortest run time
tests.integration.druid.test_druid::test_druid_ingest
Stack Traces | 3.82s run time
docker_compose_runner = <function docker_compose_runner.<locals>.run at 0x7f6daaf04c20>

    @pytest.fixture(scope="module")
    def druid_up(docker_compose_runner):
>       with docker_compose_runner(
            DOCKER_DIR / "docker-compose.yml", "druid"
        ) as docker_services:

.../integration/druid/test_druid.py:22: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
.../hostedtoolcache/Python/3.11.15....../x64/lib/python3.11/contextlib.py:137: in __enter__
    return next(self.gen)
           ^^^^^^^^^^^^^^
.../datahub/testing/docker_utils.py:66: in run
    with pytest_docker.plugin.get_docker_services(
.../hostedtoolcache/Python/3.11.15....../x64/lib/python3.11/contextlib.py:137: in __enter__
    return next(self.gen)
           ^^^^^^^^^^^^^^
venv/lib/python3.11........./site-packages/pytest_docker/plugin.py:212: in get_docker_services
    docker_compose.execute(command)
venv/lib/python3.11........./site-packages/pytest_docker/plugin.py:140: in execute
    return execute(command, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

command = 'docker compose --parallel -1 -f ".../druid/docker/docker-compose.yml" -p "pytest4653-druid" up --build --wait'
success_codes = (0,), ignore_stderr = False

    def execute(command: str, success_codes: Iterable[int] = (0,), ignore_stderr: bool = False) -> Union[bytes, Any]:
        """Run a shell command."""
        try:
            stderr_pipe = subprocess.DEVNULL if ignore_stderr else subprocess.STDOUT
            output = subprocess.check_output(command, stderr=stderr_pipe, shell=True)
            status = 0
        except subprocess.CalledProcessError as error:
            output = error.output or b""
            status = error.returncode
            command = error.cmd
    
        if status not in success_codes:
>           raise Exception(
                'Command {} returned {}: """{}""".'.format(command, status, output.decode("utf-8"))
            )
E           Exception: Command docker compose --parallel -1 -f ".../druid/docker/docker-compose.yml" -p "pytest4653-druid" up --build --wait returned 1: """time="2026-06-22T14:11:01Z" level=warning msg=".../druid/docker/docker-compose.yml: the attribute `version` is obsolete, it will be ignored, please remove it to avoid potential confusion"
E            postgres Pulling 
E            router Pulling 
E            zookeeper Pulling 
E            broker Pulling 
E            historical Pulling 
E            coordinator Pulling 
E            middlemanager Pulling 
E            postgres Error Head "https://registry-1.docker..../postgres/manifests/latest": unknown:
E            historical  Interrupted
E            coordinator  Interrupted
E            middlemanager  Interrupted
E            zookeeper  Interrupted
E            router  Interrupted
E            broker  Interrupted
E           Error response from daemon: Head "https://registry-1.docker..../postgres/manifests/latest": unknown:
E           """.

venv/lib/python3.11........./site-packages/pytest_docker/plugin.py:37: Exception

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

@codecov

codecov Bot commented Jun 17, 2026

Copy link
Copy Markdown

Bundle Report

Changes will increase total bundle size by 343 bytes (0.0%) ⬆️. This is within the configured threshold ✅

Detailed changes
Bundle name Size Change
datahub-react-web-esm 29.69MB 343 bytes (0.0%) ⬆️

Affected Assets, Files, and Routes:

view changes for bundle: datahub-react-web-esm

Assets Changed:

Asset Name Size Change Total Size Change (%)
assets/index-*.js 343 bytes 8.99MB 0.0%

Files in assets/index-*.js:

  • ./src/app/entity/shared/containers/profile/sidebar/FormInfo/useIsUserAssigned.ts → Total Size: 1.42kB

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

devops PR or Issue related to DataHub backend & deployment docs Issues and Improvements to docs ingestion PR or Issue related to the ingestion of metadata needs-review Label for PRs that need review from a maintainer. product PR or Issue related to the DataHub UI/UX

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants