-
Notifications
You must be signed in to change notification settings - Fork 72
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Datamap Report: Rename columns #5400
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
728b811
to
865c7c7
Compare
fides Run #10525
Run Properties:
|
Project |
fides
|
Branch Review |
refs/pull/5400/merge
|
Run status |
Passed #10525
|
Run duration | 00m 36s |
Commit |
f44cd2272e ℹ️: Merge 1016633cbd59fbae867e612ff6f46b3e1f9f9577 into 7247873f8bb59c4963aa868485e5...
|
Committer | Jason Gill |
View all properties for this run ↗︎ |
Test results | |
---|---|
Failures |
0
|
Flaky |
0
|
Pending |
0
|
Skipped |
0
|
Passing |
4
|
Upgrade your plan to view test results. | |
View all changes introduced in this branch ↗︎ |
if (!userCanSeeReports) { | ||
return null; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I moved this logic to the parent component so none of this loads if the user doesn't have permissions to view it.
@@ -35,7 +35,7 @@ export const GlobalFilterV2 = ({ | |||
}, [value, onClear]); | |||
|
|||
return ( | |||
<Box maxWidth="510px" width="100%"> | |||
<Box maxWidth="424px" width="100%"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
matches newer designs and leaves more room for the extra buttons
|
||
// eslint-disable-next-line @typescript-eslint/naming-convention | ||
export enum DATAMAP_LOCAL_STORAGE_KEYS { | ||
COLUMN_ORDER = "datamap-column-order", | ||
COLUMN_VISIBILITY = "datamap-column-visibility", | ||
COLUMN_SIZING = "datamap-column-sizing", | ||
COLUMN_EXPANSION_STATE = "datamap-column-expansion-state", | ||
CUSTOM_REPORT_ID = "datamap-custom-report-id", | ||
FILTERS = "datamap-filters", | ||
GROUP_BY = "datamap-group-by", | ||
SORTING_STATE = "datamap-sorting-state", | ||
WRAPPING_COLUMNS = "datamap-wrapping-columns", | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moved to new reporting constants file
@@ -114,7 +114,7 @@ const SystemInfo = ({ system }: SystemInfoProps) => { | |||
</Box> | |||
<Box marginTop={3}> | |||
<CustomTextArea | |||
label="System Description" | |||
label="System description" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just a drive-by fix
// eslint-disable-next-line @typescript-eslint/naming-convention | ||
export enum COLUMN_IDS { | ||
SYSTEM_NAME = "system_name", | ||
DATA_USE = "data_use", | ||
DATA_CATEGORY = "data_categories", | ||
DATA_SUBJECT = "data_subjects", | ||
LEGAL_NAME = "legal_name", | ||
DPO = "dpo", | ||
LEGAL_BASIS_FOR_PROCESSING = "legal_basis_for_processing", | ||
ADMINISTRATING_DEPARTMENT = "administrating_department", | ||
COOKIE_MAX_AGE_SECONDS = "cookie_max_age_seconds", | ||
PRIVACY_POLICY = "privacy_policy", | ||
LEGAL_ADDRESS = "legal_address", | ||
COOKIE_REFRESH = "cookie_refresh", | ||
DATA_SECURITY_PRACTICES = "data_security_practices", | ||
DATA_SHARED_WITH_THIRD_PARTIES = "DATA_SHARED_WITH_THIRD_PARTIES", | ||
DATA_STEWARDS = "data_stewards", | ||
DECLARATION_NAME = "declaration_name", | ||
DESCRIPTION = "description", | ||
DOES_INTERNATIONAL_TRANSFERS = "does_international_transfers", | ||
DPA_LOCATION = "dpa_location", | ||
DESTINATIONS = "egress", | ||
EXEMPT_FROM_PRIVACY_REGULATIONS = "exempt_from_privacy_regulations", | ||
FEATURES = "features", | ||
FIDES_KEY = "fides_key", | ||
FLEXIBLE_LEGAL_BASIS_FOR_PROCESSING = "flexible_legal_basis_for_processing", | ||
IMPACT_ASSESSMENT_LOCATION = "impact_assessment_location", | ||
SOURCES = "ingress", | ||
JOINT_CONTROLLER_INFO = "joint_controller_info", | ||
LEGAL_BASIS_FOR_PROFILING = "legal_basis_for_profiling", | ||
LEGAL_BASIS_FOR_TRANSFERS = "legal_basis_for_transfers", | ||
LEGITIMATE_INTEREST_DISCLOSURE_URL = "legitimate_interest_disclosure_url", | ||
LINK_TO_PROCESSOR_CONTRACT = "link_to_processor_contract", | ||
PROCESSES_PERSONAL_DATA = "processes_personal_data", | ||
REASON_FOR_EXEMPTION = "reason_for_exemption", | ||
REQUIRES_DATA_PROTECTION_ASSESSMENTS = "requires_data_protection_assessments", | ||
RESPONSIBILITY = "responsibility", | ||
RETENTION_PERIOD = "retention_period", | ||
SHARED_CATEGORIES = "shared_categories", | ||
SPECIAL_CATEGORY_LEGAL_BASIS = "special_category_legal_basis", | ||
SYSTEM_DEPENDENCIES = "system_dependencies", | ||
THIRD_COUNTRY_SAFEGUARDS = "third_country_safeguards", | ||
THIRD_PARTIES = "third_parties", | ||
COOKIES = "cookies", | ||
USES_COOKIES = "uses_cookies", | ||
USES_NON_COOKIE_ACCESS = "uses_non_cookie_access", | ||
USES_PROFILING = "uses_profiling", | ||
SYSTEM_UNDECLARED_DATA_CATEGORIES = "system_undeclared_data_categories", | ||
DATA_USE_UNDECLARED_DATA_CATEGORIES = "data_use_undeclared_data_categories", | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved to new reporting constants file
const keyWithoutPrefix = key.replace(/^(system_|privacy_declaration_)/, ""); | ||
const displayText = _.upperFirst(keyWithoutPrefix.replaceAll("_", " ")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moved to a util
@@ -150,7 +150,6 @@ export const PrivacyExperiencesTable = () => { | |||
), | |||
header: (props) => <DefaultHeaderCell value="Locations" {...props} />, | |||
meta: { | |||
displayText: "Locations", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just cleaning up a little "Cargo Cult Coding" happening here, these displayText
entries were never used on any table except for the Datamap report
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅ Tested locally, everything working as expected for me. Great work!
fides Run #10528
Run Properties:
|
Project |
fides
|
Branch Review |
main
|
Run status |
Passed #10528
|
Run duration | 00m 38s |
Commit |
0f2fe91028: Datamap Report: Rename columns (#5400)
|
Committer | Jason Gill |
View all properties for this run ↗︎ |
Test results | |
---|---|
Failures |
0
|
Flaky |
0
|
Pending |
0
|
Skipped |
0
|
Passing |
4
|
Upgrade your plan to view test results. | |
View all changes introduced in this branch ↗︎ |
Closes HJ-1
Description Of Changes
Fides Plus users should be able to edit the name of the columns in reporting views. User's changes will be saved locally in the browser, but will also be saved as part of custom reports.
Code Changes
EditableHeader
to support edit mode hereSteps to Confirm
a. Each input should have a placeholder which is the equivalent of the original column header name
b. 3 new buttons should be present: "Reset All", "Cancel", and "Apply" (primary)
a. Newly entered names should appear as the Header name of their respective columns
b. 3 buttons are no longer visible
Pre-Merge Checklist
CHANGELOG.md