Skip to content

Commit

Permalink
Fix D&D resource display names (#5498)
Browse files Browse the repository at this point in the history
  • Loading branch information
jpople authored Nov 19, 2024
1 parent 2321052 commit ee3924d
Show file tree
Hide file tree
Showing 17 changed files with 306 additions and 17 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ The types of changes are:
### Fixed
- Fixed deletion of ConnectionConfigs that have related MonitorConfigs [#5478](https://github.com/ethyca/fides/pull/5478)
- Fixed extra delete icon on Domains page [#5513](https://github.com/ethyca/fides/pull/5513)
- Fixed incorrect display names on some D&D resources [#5498](https://github.com/ethyca/fides/pull/5498)

### Changed
- Allow hiding systems via a `hidden` parameter and add two flags on the `/system` api endpoint; `show_hidden` and `dnd_relevant`, to display only systems with integrations [#5484](https://github.com/ethyca/fides/pull/5484)
Expand Down
74 changes: 73 additions & 1 deletion clients/admin-ui/cypress/e2e/discovery-detection.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,11 @@ describe("discovery and detection", () => {
cy.getByTestId("name-header").should("contain", "Table name");
cy.getByTestId(
"row-my_bigquery_monitor.prj-bigquery-418515.test_dataset_1.consent-reports-20",
).should("exist");
).within(() => {
cy.getByTestId(
"row-my_bigquery_monitor.prj-bigquery-418515.test_dataset_1.consent-reports-20-col-name",
).should("contain", "consent-reports-20");
});
cy.getByTestId(
"row-my_bigquery_monitor.prj-bigquery-418515.test_dataset_1.consent-reports-19",
).should("exist");
Expand Down Expand Up @@ -278,6 +282,9 @@ describe("discovery and detection", () => {
it("should show columns for fields", () => {
cy.getByTestId("fidesTable-body").should("exist");
cy.getByTestId("column-name").should("contain", "Field name");
cy.getByTestId(
"row-my_bigquery_monitor.prj-bigquery-418515.test_dataset_1.consent-reports-20.Created-col-name",
).should("contain", "Created");
});

it("should allow navigation via row clicking on nested fields", () => {
Expand Down Expand Up @@ -430,5 +437,70 @@ describe("discovery and detection", () => {
});
});
});

describe("nested-field-level view", () => {
beforeEach(() => {
cy.intercept("GET", "/api/v1/plus/discovery-monitor/results?*", {
fixture:
"detection-discovery/results/discovery/nested-field-list.json",
}).as("getNestedDetectionFields");
cy.visit(
`${DATA_DETECTION_ROUTE}/my_bigquery_monitor.prj-bigquery-418515.test_dataset_1.consent-reports-20.address`,
);
cy.wait("@getNestedDetectionFields");
});

it("should show columns for nested fields", () => {
cy.getByTestId("fidesTable-body").should("exist");
cy.getByTestId("column-name").should("contain", "Field name");
cy.getByTestId(
"row-my_bigquery_monitor.prj-bigquery-418515.test_dataset_1.consent-reports-20.address.zip-col-name",
).should("contain", "zip");
cy.getByTestId(
"row-my_bigquery_monitor.prj-bigquery-418515.test_dataset_1.consent-reports-20.address.street.line_1-col-name",
).should("contain", "street.line_1");
});

it("should not allow navigation via row clicking", () => {
cy.getByTestId(
"row-my_bigquery_monitor.prj-bigquery-418515.test_dataset_1.consent-reports-20.address.street.line_1",
).click();
cy.url().should("not.contain", "street");
});
});
});

describe.only("without a db layer", () => {
it("shows resource name for resources that are not subfields", () => {
cy.intercept("GET", "/api/v1/plus/discovery-monitor/results?*", {
fixture: "detection-discovery/results/no-db-layer/field-list.json",
}).as("getFields");
cy.visit(
`${DATA_DISCOVERY_ROUTE}/dynamo_monitor.test_dataset_5.test_table_1`,
);
cy.getByTestId(
"row-dynamo_monitor.test_dataset_5.test_table_1.test_field_1-col-name",
).should("contain", "test_field_1");
});

it("shows subfield names in dot notation", () => {
cy.intercept("GET", "/api/v1/plus/discovery-monitor/results?*", {
fixture:
"detection-discovery/results/no-db-layer/nested-field-list.json",
}).as("getFields");
cy.visit(
`${DATA_DISCOVERY_ROUTE}/my_bigquery_monitor.prj-bigquery-418515.test_dataset_1.consent-reports-20.address`,
);
cy.getByTestId(
"row-dynamo_monitor.test_dataset_5.test_table_1.address.zip-col-name",
).should("contain", "zip");
cy.getByTestId(
"row-dynamo_monitor.test_dataset_5.test_table_1.address.street.line_1-col-name",
).should("contain", "street.line_1");
// should still see correct name when top_level_field_urn is not provided
cy.getByTestId(
"row-dynamo_monitor.test_dataset_5.test_table_1.address.street.line_2-col-name",
).should("contain", "street.line_2");
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
{
"items": [
{
"urn": "my_bigquery_monitor.prj-bigquery-418515.test_dataset_1.consent-reports-20.address.zip",
"user_assigned_data_categories": [],
"name": "zip",
"description": null,
"monitor_config_id": "my_bigquery_monitor",
"updated_at": null,
"source_modified": "2024-03-27T21:47:09.915000+00:00",
"classifications": [],
"diff_status": "monitored",
"child_diff_statuses": { "classification_addition": true },
"database_name": "prj-bigquery-418515",
"schema_name": "test_dataset_1",
"table_name": "consent-reports-20",
"top_level_field_name": "address",
"top_level_field_urn": "my_bigquery_monitor.prj-bigquery-418515.test_dataset_1.consent-reports-20.address"
},

{
"urn": "my_bigquery_monitor.prj-bigquery-418515.test_dataset_1.consent-reports-20.address.street.line_1",
"user_assigned_data_categories": [],
"name": "line_1",
"description": null,
"monitor_config_id": "my_bigquery_monitor",
"updated_at": null,
"source_modified": "2024-03-27T21:47:09.915000+00:00",
"classifications": [],
"diff_status": "monitored",
"child_diff_statuses": { "classification_addition": true },
"database_name": "prj-bigquery-418515",
"schema_name": "test_dataset_1",
"table_name": "consent-reports-20",
"top_level_field_name": "address",
"top_level_field_urn": "my_bigquery_monitor.prj-bigquery-418515.test_dataset_1.consent-reports-20.address"
},

{
"urn": "my_bigquery_monitor.prj-bigquery-418515.test_dataset_1.consent-reports-20.address.street.line_2",
"user_assigned_data_categories": [],
"name": "line_2",
"description": null,
"monitor_config_id": "my_bigquery_monitor",
"updated_at": null,
"source_modified": "2024-03-27T21:47:09.915000+00:00",
"classifications": [],
"diff_status": "monitored",
"child_diff_statuses": { "classification_addition": true },
"database_name": "prj-bigquery-418515",
"schema_name": "test_dataset_1",
"table_name": "consent-reports-20",
"top_level_field_name": "address",
"top_level_field_urn": "my_bigquery_monitor.prj-bigquery-418515.test_dataset_1.consent-reports-20.address"
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
{
"items": [
{
"urn": "dynamo_monitor.test_dataset_5.test_table_1.test_field_1",
"user_assigned_data_categories": [],
"name": "test_field_1",
"description": null,
"monitor_config_id": "dynamo_monitor",
"schema_name": "test_dataset_5",
"table_name": "test_table_1",
"updated_at": null,
"source_modified": "2024-03-27T21:45:55.755000+00:00",
"classifications": [],
"diff_status": "classification_addition",
"child_diff_statuses": {},
"parent_schema": "dynamo_monitor.test_dataset_5",
"database_name": null,
"data_type": null
}
],
"total": 1,
"page": 1,
"size": 25,
"pages": 1
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
{
"items": [
{
"urn": "dynamo_monitor.test_dataset_5.test_table_1.address.zip",
"user_assigned_data_categories": [],
"name": "zip",
"description": null,
"monitor_config_id": "dynamo_monitor",
"updated_at": null,
"source_modified": "2024-03-27T21:47:09.915000+00:00",
"classifications": [],
"diff_status": "monitored",
"child_diff_statuses": { "classification_addition": true },
"database_name": null,
"schema_name": "test_dataset_5",
"table_name": "test_table_1",
"top_level_field_name": "address",
"top_level_field_urn": "dynamo_monitor.test_dataset_5.test_table_1.address"
},

{
"urn": "dynamo_monitor.test_dataset_5.test_table_1.address.street.line_1",
"user_assigned_data_categories": [],
"name": "line_1",
"description": null,
"monitor_config_id": "dynamo_monitor",
"updated_at": null,
"source_modified": "2024-03-27T21:47:09.915000+00:00",
"classifications": [],
"diff_status": "monitored",
"child_diff_statuses": { "classification_addition": true },
"database_name": null,
"schema_name": "test_dataset_5",
"table_name": "test_table_1",
"top_level_field_name": "address",
"top_level_field_urn": "dynamo_monitor.test_dataset_5.test_table_1.address"
},

{
"urn": "dynamo_monitor.test_dataset_5.test_table_1.address.street.line_2",
"user_assigned_data_categories": [],
"name": "line_2",
"description": null,
"monitor_config_id": "dynamo_monitor",
"updated_at": null,
"source_modified": "2024-03-27T21:47:09.915000+00:00",
"classifications": [],
"diff_status": "monitored",
"child_diff_statuses": { "classification_addition": true },
"database_name": null,
"schema_name": "test_dataset_5",
"table_name": "test_table_1",
"top_level_field_name": "address",
"top_level_field_urn": null
}
]
}
Original file line number Diff line number Diff line change
@@ -1,22 +1,52 @@
import { DiscoveryMonitorItem } from "~/features/data-discovery-and-detection/types/DiscoveryMonitorItem";

const TOP_LEVEL_FIELD_URN_PARTS = 5;
const URN_SEPARATOR = ".";
/**
* Helper method for deriving a resource's display name from its URN
*/
const getResourceName = ({
name,
urn,
monitor_config_id,
database_name,
schema_name,
table_name,
top_level_field_name,
top_level_field_urn,
}: DiscoveryMonitorItem) => {
// use name as-is if resource is not a subfield
if (!top_level_field_name) {
return name;
}

const splitUrn = urn.split(URN_SEPARATOR);
// for subfields, we want to show all subfield names separated by "."
// format is "monitor.project?.dataset.table.field.[any number of subfields]"

const getResourceName = (resource: DiscoveryMonitorItem) => {
const URN_SEPARATOR = ".";
const splitUrn = resource.urn.split(URN_SEPARATOR);
if (
!resource.parent_table_urn ||
splitUrn.length === TOP_LEVEL_FIELD_URN_PARTS
) {
// use name as-is if it's not a subfield
return resource.name;
// this case *should* catch all subfields
if (top_level_field_urn) {
return urn.replace(`${top_level_field_urn}${URN_SEPARATOR}`, "");
}
// TODO HJ-162: better handle case where field name contains "."

// URN format is "monitor.project.dataset.field.[any number of subfields]"
// for a subfield, we want to show all subfield names separated by "."
return splitUrn.slice(TOP_LEVEL_FIELD_URN_PARTS).join(URN_SEPARATOR);
// as a fallback, parse URN manually and remove higher-level segments
const segmentsToRemove = [
monitor_config_id,
database_name,
schema_name,
table_name,
top_level_field_name,
];

segmentsToRemove.forEach((part) => {
if (part) {
const index = splitUrn.indexOf(part);
if (index > -1) {
splitUrn.splice(index, 1);
}
}
});

return splitUrn.join(URN_SEPARATOR);
};

export default getResourceName;
2 changes: 2 additions & 0 deletions clients/admin-ui/src/types/api/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ export type { DatabaseConfig } from "./models/DatabaseConfig";
export type { DatabaseHealthCheck } from "./models/DatabaseHealthCheck";
export type { DataCategory } from "./models/DataCategory";
export type { DataFlow } from "./models/DataFlow";
export type { DatahubDocsSchema } from "./models/DatahubDocsSchema";
export { DATAMAP_GROUPING } from "./models/DATAMAP_GROUPING";
export type { DatamapReport } from "./models/DatamapReport";
export { DataResponsibilityTitle } from "./models/DataResponsibilityTitle";
Expand Down Expand Up @@ -309,6 +310,7 @@ export type { Page_UserResponse_ } from "./models/Page_UserResponse_";
export type { ParamValue } from "./models/ParamValue";
export type { PartialPrivacyCenterConfig } from "./models/PartialPrivacyCenterConfig";
export type { PartialPrivacyRequestOption } from "./models/PartialPrivacyRequestOption";
export { PeriodicIntegrationFrequency } from "./models/PeriodicIntegrationFrequency";
export type { PlusApplicationConfig } from "./models/PlusApplicationConfig";
export type { PolicyMaskingSpec } from "./models/PolicyMaskingSpec";
export type { PolicyMaskingSpecResponse } from "./models/PolicyMaskingSpecResponse";
Expand Down
1 change: 1 addition & 0 deletions clients/admin-ui/src/types/api/models/ConnectionType.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
export enum ConnectionType {
ATTENTIVE_EMAIL = "attentive_email",
BIGQUERY = "bigquery",
DATAHUB = "datahub",
DYNAMODB = "dynamodb",
FIDES = "fides",
GENERIC_CONSENT_EMAIL = "generic_consent_email",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import type { AccessLevel } from "./AccessLevel";
import type { BigQueryDocsSchema } from "./BigQueryDocsSchema";
import type { ConnectionType } from "./ConnectionType";
import type { DatahubDocsSchema } from "./DatahubDocsSchema";
import type { DynamicErasureEmailDocsSchema } from "./DynamicErasureEmailDocsSchema";
import type { DynamoDBDocsSchema } from "./DynamoDBDocsSchema";
import type { EmailDocsSchema } from "./EmailDocsSchema";
Expand Down Expand Up @@ -39,6 +40,7 @@ export type CreateConnectionConfigurationWithSecrets = {
description?: string | null;
secrets?:
| BigQueryDocsSchema
| DatahubDocsSchema
| DynamicErasureEmailDocsSchema
| DynamoDBDocsSchema
| EmailDocsSchema
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import type { AccessLevel } from "./AccessLevel";
import type { ActionType } from "./ActionType";
import type { BigQueryDocsSchema } from "./BigQueryDocsSchema";
import type { ConnectionType } from "./ConnectionType";
import type { DatahubDocsSchema } from "./DatahubDocsSchema";
import type { DynamicErasureEmailDocsSchema } from "./DynamicErasureEmailDocsSchema";
import type { DynamoDBDocsSchema } from "./DynamoDBDocsSchema";
import type { EmailDocsSchema } from "./EmailDocsSchema";
Expand Down Expand Up @@ -40,6 +41,7 @@ export type CreateConnectionConfigurationWithSecretsExtended = {
description?: string | null;
secrets?:
| BigQueryDocsSchema
| DatahubDocsSchema
| DynamicErasureEmailDocsSchema
| DynamoDBDocsSchema
| EmailDocsSchema
Expand Down
23 changes: 23 additions & 0 deletions clients/admin-ui/src/types/api/models/DatahubDocsSchema.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/* istanbul ignore file */
/* tslint:disable */
/* eslint-disable */

import type { PeriodicIntegrationFrequency } from "./PeriodicIntegrationFrequency";

/**
* Datahub Schema for API docs.
*/
export type DatahubDocsSchema = {
/**
* The URL of your DataHub server.
*/
datahub_server_url: string;
/**
* The token used to authenticate with your DataHub server.
*/
datahub_token: string;
/**
* The frequency at which the integration should run. Defaults to daily.
*/
frequency?: PeriodicIntegrationFrequency;
};
1 change: 1 addition & 0 deletions clients/admin-ui/src/types/api/models/Field.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ export type Field = {
sub_field_urns?: Array<string>;
direct_child_urns?: Array<string>;
top_level_field_name?: string | null;
top_level_field_urn?: string | null;
source_data_type?: string | null;
sub_fields?: Array<Field> | null;
};
Loading

0 comments on commit ee3924d

Please sign in to comment.