Skip to content

Conversation

khaledosman
Copy link
Contributor

@khaledosman khaledosman commented Mar 13, 2025

What's changing

Closes #1150

  • Setup error tracking with sentry for the frontend
  • Adds new banner to capture user's consent to opt-in for error reporting
  • Adds a new field to the settings page for users to opt-in / opt-out
  • Consent is stored in localStorage so the banned is shown on first visit only

How to test it

Error tracking will be disabled when running in dev mode via npm run dev, but should be available when building for production via npm run build (which is what make local-up calls)

Steps to test the changes:

  1. install the new added sentry dependency by running npm install in the frontend directory
  2. run in dev mode via npm run dev <-- there should be no consent banner or errors being sent to sentry
  3. run in prod mode via make local-up <-- there should be a consent banner (result saved in localStorage) and errors being sent to sentry after consent
  4. Settings page should show user's opt-in status
  5. To show the consent banner again you will need to delete the "sentry-consent" key from your browser's localStorage options because its only shown for first time user experience

Additional notes for reviewers

image

I already...

  • Tested the changes in a working environment to ensure they work as expected
  • Added some tests for any new functionality
  • Updated the documentation (both comments in code and product documentation under /docs)
  • Checked if a (backend) DB migration step was required and included it if required

@khaledosman khaledosman marked this pull request as ready for review March 14, 2025 12:47
@khaledosman khaledosman changed the title feat: set up crash reporting with sentry on the frontend feat: set up error reporting with sentry on the frontend Mar 14, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements frontend crash reporting by integrating Sentry and introducing a user consent banner to opt in or out of error reporting. The key changes include adding error tracking support, creating a new ConsentBanner component, and integrating consent logic on the settings page and App.vue.

Reviewed Changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated no comments.

Show a summary per file
File Description
lumigator/frontend/src/components/views/LDatasets.vue Removed redundant SCSS variable assignment for cleaner styles.
lumigator/frontend/src/components/layout/LPageHeader.vue Removed redundant SCSS variable assignment.
lumigator/frontend/src/components/experiments/LJobsTable.vue Removed redundant SCSS variable assignment.
lumigator/frontend/src/components/experiments/LExperimentsEmpty.vue Removed redundant SCSS variable assignment.
lumigator/frontend/src/components/experiments/LExperimentsDrawer.vue Removed redundant SCSS variable assignment.
lumigator/frontend/src/components/experiments/LExperimentTabs.vue Removed redundant SCSS variable assignment.
lumigator/frontend/src/components/experiments/LExperimentTable.vue Removed redundant SCSS variable assignment.
lumigator/frontend/src/components/experiments/LExperimentLogs.vue Removed redundant SCSS variable assignment.
lumigator/frontend/src/components/experiments/LExperimentForm.vue Removed redundant SCSS variable assignment.
lumigator/frontend/src/components/experiments/LExperimentDetails.vue Removed redundant SCSS variable assignment.
lumigator/frontend/src/components/datasets/LJobDetails.vue Removed redundant SCSS variable assignment.
lumigator/frontend/src/components/datasets/LInferenceJobsTable.vue Removed redundant SCSS variable assignment.
lumigator/frontend/src/components/datasets/LDatasetTable.vue Removed redundant SCSS variable assignment.
lumigator/frontend/src/components/datasets/LDatasetEmpty.vue Removed redundant SCSS variable assignment.
lumigator/frontend/src/components/datasets/LDatasetDetails.vue Removed redundant SCSS variable assignment.
lumigator/frontend/src/components/common/ConsentBanner.vue Introduced new component for obtaining user consent on error reporting.
lumigator/frontend/src/App.vue Integrated the ConsentBanner and updated link syntax for consistency.
Comments suppressed due to low confidence (1)

lumigator/frontend/src/App.vue:58

  • Consider adding tests to cover the ConsentBanner integration and the consent logic transitioning between development and production builds.
<ConsentBanner v-if="!hasResponded && isProdBuild" @accept="acceptConsent" @reject="rejectConsent"></ConsentBanner>

@hasangzl
Copy link
Contributor

@agpituk Is there a page we can link to for people who want to learn more about our data policy?

@hasangzl
Copy link
Contributor

@khaledosman Shouldn't we implement the banner with our pop-up component? The same one we use for deleting experiments?

@khaledosman
Copy link
Contributor Author

khaledosman commented Mar 17, 2025

@khaledosman Shouldn't we implement the banner with our pop-up component? The same one we use for deleting experiments?

We could, but it might be too annoying showing a popup in the middle of the screen the first time the user opens the app, I prefer the banner approach since its not as noisy and doesn't hide the application from the user unless they respond to it.

But we could probably use the Drawer component from primevue, which we already use for logs for example

@hasangzl
Copy link
Contributor

@khaledosman I see. I thought the user had to respond to this before they could continue using the interface. In that case, the drawer could work. The Template and Heedless options under Toast component are also interesting. Let's discuss in our call later.

@hasangzl
Copy link
Contributor

See design

@khaledosman
Copy link
Contributor Author

Should be good to go now once we link the privacy policy and set the sentry DSN.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add crash report tracking to the UI

3 participants