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

Custom report models #5344

Merged
merged 10 commits into from
Oct 9, 2024
Merged

Custom report models #5344

merged 10 commits into from
Oct 9, 2024

Conversation

galvana
Copy link
Contributor

@galvana galvana commented Oct 1, 2024

Closes PROD-2673

Description Of Changes

Introduces the CustomReport model with the following properties:

  • id
  • name
  • type
  • created_by
  • config

A custom report, for now, is defined as an object containing the table state (to be determined by the front-end) and a mapping of column keys to custom labels.

Code Changes

  • New CustomReport model

Steps to Confirm

Pre-Merge Checklist

  • All CI Pipelines Succeeded
  • Issue Requirements are Met
  • Update CHANGELOG.md
  • If there are any database migrations:
    • Ensure that your downrev is up to date with the latest revision on main
    • Ensure that your downgrade() migration is correct and works

Copy link

vercel bot commented Oct 1, 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 Oct 9, 2024 7:08am

@galvana galvana changed the title Prod 2676 be datamap report columns Custom report models Oct 1, 2024
Copy link

cypress bot commented Oct 1, 2024

fides    Run #10366

Run Properties:  status check passed Passed #10366  •  git commit 0e5a324197 ℹ️: Merge 9d349b8ac3a9552f0ddb7dcd1d5e989b33282bcf into c294fb532a23645f7d237b50f27d...
Project fides
Run status status check passed Passed #10366
Run duration 00m 38s
Commit git commit 0e5a324197 ℹ️: Merge 9d349b8ac3a9552f0ddb7dcd1d5e989b33282bcf into c294fb532a23645f7d237b50f27d...
Committer Adrian Galvan
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 4
⚠️ You've recorded test results over your free plan limit.
Upgrade your plan to view test results.

@erosselli erosselli self-assigned this Oct 2, 2024
yield custom_report
custom_report.delete(db)

def test_create_custom_report(self, db: Session, user: FidesUser):
Copy link
Contributor

Choose a reason for hiding this comment

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

this test doesn't have any assertions -- is that expected?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was just asserting that we could create it with no errors but I added field assertions now

sa.Column("name", sa.String(), nullable=True),
sa.Column("type", sa.String(), nullable=False),
sa.Column("created_by", sa.String(), nullable=True),
sa.Column("config", postgresql.JSONB(astext_type=sa.Text()), nullable=True),
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we storing the table_state and column_map within the config (as opposed to using two separate columns) because we expect that we'll need more things in the config if/when we implement custom reports in other parts of the UI?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's right, especially for table_state, Jason is still trying to figure out the schema for the front-end. It's tricky because the metadata to capture a Tanstack table is different from an ANT table so I just provided a flexible solution for now.

Copy link

codecov bot commented Oct 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.33%. Comparing base (eb9ba5e) to head (65bb222).
Report is 8 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5344      +/-   ##
==========================================
- Coverage   85.44%   85.33%   -0.12%     
==========================================
  Files         378      380       +2     
  Lines       23896    24021     +125     
  Branches     3189     3219      +30     
==========================================
+ Hits        20419    20499      +80     
- Misses       2892     2932      +40     
- Partials      585      590       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@galvana galvana merged commit 86bb3f7 into main Oct 9, 2024
13 of 14 checks passed
@galvana galvana deleted the PROD-2676-BE-datamap-report-columns branch October 9, 2024 07:06
Copy link

cypress bot commented Oct 9, 2024

fides    Run #10367

Run Properties:  status check passed Passed #10367  •  git commit 86bb3f70c8: Custom report models (#5344)
Project fides
Run status status check passed Passed #10367
Run duration 00m 38s
Commit git commit 86bb3f70c8: Custom report models (#5344)
Committer Adrian Galvan
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 4
⚠️ You've recorded test results over your free plan limit.
Upgrade your plan to view test results.

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