Skip to content
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

Add "data type" column to D&D tables #5218

Merged
merged 11 commits into from
Aug 21, 2024
Merged

Conversation

jpople
Copy link
Contributor

@jpople jpople commented Aug 20, 2024

Closes PROD-2379

Description Of Changes

Adds a "data type" column to fields in D&D tables.

Screenshot 2024-08-20 at 11 10 50

Code Changes

  • Regenerate types
  • Refactor D&D table cells; collect them all in one directory and change some names for consistency

Steps to Confirm

Running fidesplus backend on branch https://github.com/ethyca/fidesplus/pull/1567/files, "Data type" column should appear on fields in D&D result tables.

Pre-Merge Checklist

  • All CI Pipelines Succeeded
  • Issue Requirements are Met
  • Update CHANGELOG.md

Copy link

vercel bot commented Aug 20, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
fides-plus-nightly ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 21, 2024 1:54pm

@jpople jpople requested a review from lucanovera August 20, 2024 16:18
Copy link

cypress bot commented Aug 20, 2024



Test summary

4 0 0 0


Run details

Project fides
Status Passed
Commit 5b096922c6 ℹ️
Started Aug 21, 2024 2:12 PM
Ended Aug 21, 2024 2:12 PM
Duration 00:37 💡
OS Linux Ubuntu -
Browser Electron 106

View run in Cypress Cloud ➡️


This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Cloud

Copy link
Contributor

@lucanovera lucanovera left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job organizing the files. I see the data type column correctly in the Data Detection portion.
Captura de pantalla 2024-08-20 a la(s) 3 23 30 p  m

But no values appear in the Data Discovery column, looking at the response from the API I see that the source_data_type is null but the data_type says string. So I wonder if this is a backend issue or is it expected to get it as data_type and that should be the fallback value to source_data type in the FE.

Also, I saw you removed the "--" fallback for description in the useDetectionResultColumns on line 124 but it is still there in other parts of the file and in useDiscoveryResultColumns too, we'll probably want to keep a consistency there.

@jpople
Copy link
Contributor Author

jpople commented Aug 20, 2024

@lucanovera

I saw you removed the "--" fallback for description in the useDetectionResultColumns on line 124 but it is still there in other parts of the file and in useDiscoveryResultColumns too, we'll probably want to keep a consistency there.

Thanks for catching this, fixed.

But no values appear in the Data Discovery column, looking at the response from the API I see that the source_data_type is null but the data_type says string. So I wonder if this is a backend issue or is it expected to get it as data_type and that should be the fallback value to source_data type in the FE.

This was a backend and frontend issue-- I was querying the wrong field and the backend was returning null for the right field (source_data_type is correct). It should be resolved, try pulling the latest and you should see source_data_type populated in both detection and discovery tables.

@lucanovera
Copy link
Contributor

@lucanovera

I saw you removed the "--" fallback for description in the useDetectionResultColumns on line 124 but it is still there in other parts of the file and in useDiscoveryResultColumns too, we'll probably want to keep a consistency there.

Thanks for catching this, fixed.

But no values appear in the Data Discovery column, looking at the response from the API I see that the source_data_type is null but the data_type says string. So I wonder if this is a backend issue or is it expected to get it as data_type and that should be the fallback value to source_data type in the FE.

This was a backend and frontend issue-- I was querying the wrong field and the backend was returning null for the right field (source_data_type is correct). It should be resolved, try pulling the latest and you should see source_data_type populated in both detection and discovery tables.

Thanks for the updates. I just tested with the latest from BE and FE branches and I see the data types correctly.

Copy link
Contributor

@lucanovera lucanovera left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved!

@jpople jpople merged commit 8333be1 into main Aug 21, 2024
13 checks passed
@jpople jpople deleted the jpople/prod-2379/data-type-column branch August 21, 2024 16:13
Copy link

cypress bot commented Aug 21, 2024



Test summary

4 0 0 0


Run details

Project fides
Status Passed
Commit 8333be1
Started Aug 21, 2024 4:27 PM
Ended Aug 21, 2024 4:28 PM
Duration 00:38 💡
OS Linux Ubuntu -
Browser Electron 106

View run in Cypress Cloud ➡️


This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Cloud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants