Skip to content

Conversation

@rolodato
Copy link
Contributor

Thanks for submitting a PR! Please check the boxes below:

  • I have added information to docs/ if required so people know about the feature!
  • I have filled in the "Changes" section below?
  • I have filled in the "How did you test this code" section below?
  • I have used a Conventional Commit title for this Pull Request

Changes

Previously, we were requiring users to enter an email address so that they could receive these reports:

image

This is not ideal for several reasons:

  • Not all self-hosted customers have an email provider set up. This change lets these customers access reports without needing to set up email.
  • Even if all customers did have email providers, email is a generally awful medium to work with. HTML, PDF, etc are much easier to use and test, for both us and customers.
  • The email report was synchronously generated and sent as part of the same request; we are not sacrificing performance by returning the output via email or HTML.

This change also intentionally breaks the /sales-dashboard/email-usage route by renaming it to /sales-dashboard/usage. It is extremely unlikely that this will actually break anyone's links, considering it's not a documented route and this route would be used ~2x a year at most.

Alternatives considered

If we really want to keep emails for some reason, we could have a plain-text report be sent directly from the client, by using a mailto link from the same HTML-rendered report. I decided to keep HTML only for simplicity.

How did you test this code?

Manually by browsing to /sales-dashboard/usage on any API instance.

@rolodato rolodato requested review from a team as code owners April 30, 2025 18:59
@rolodato rolodato requested review from khvn26 and removed request for a team April 30, 2025 18:59
@vercel
Copy link

vercel bot commented Apr 30, 2025

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

3 Skipped Deployments
Name Status Preview Comments Updated (UTC)
docs ⬜️ Ignored (Inspect) Visit Preview May 5, 2025 2:53pm
flagsmith-frontend-preview ⬜️ Ignored (Inspect) Visit Preview May 5, 2025 2:53pm
flagsmith-frontend-staging ⬜️ Ignored (Inspect) Visit Preview May 5, 2025 2:53pm

@github-actions github-actions bot added api Issue related to the REST API docs Documentation updates and removed docs Documentation updates labels Apr 30, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Apr 30, 2025

Docker builds report

Image Build Status Security report
ghcr.io/flagsmith/flagsmith-api-test:pr-5407 Finished ✅ Skipped
ghcr.io/flagsmith/flagsmith-e2e:pr-5407 Finished ✅ Skipped
ghcr.io/flagsmith/flagsmith-frontend:pr-5407 Finished ✅ Results
ghcr.io/flagsmith/flagsmith-api:pr-5407 Finished ✅ Results
ghcr.io/flagsmith/flagsmith:pr-5407 Finished ✅ Results
ghcr.io/flagsmith/flagsmith-private-cloud:pr-5407 Finished ✅ Results

@github-actions github-actions bot added the feature New feature or request label Apr 30, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Apr 30, 2025

Uffizzi Preview deployment-63319 was deleted.

@codecov
Copy link

codecov bot commented Apr 30, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.63%. Comparing base (705560b) to head (b0e23be).
Report is 9 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5407      +/-   ##
==========================================
+ Coverage   97.62%   97.63%   +0.01%     
==========================================
  Files        1238     1239       +1     
  Lines       43037    43065      +28     
==========================================
+ Hits        42014    42046      +32     
+ Misses       1023     1019       -4     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Contributor

@matthewelwell matthewelwell left a comment

Choose a reason for hiding this comment

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

Happy for you to add # pragma: no cover where needed if it's easier than writing tests to cover the view.

@github-actions github-actions bot added docs Documentation updates feature New feature or request and removed feature New feature or request docs Documentation updates labels May 5, 2025
@rolodato rolodato merged commit 3ad0124 into main May 5, 2025
33 checks passed
@rolodato rolodato deleted the feat/sales-dashboard-usage-without-email branch May 5, 2025 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api Issue related to the REST API feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants