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

feat(exports): Dashboard / Insight exporting #9830

Merged
merged 20 commits into from
May 27, 2022
Merged

Conversation

benjackwhite
Copy link
Contributor

@benjackwhite benjackwhite commented May 18, 2022

Problem

See #9759 for more info

Changes

  • Adds a new ExportedAsset model responsible for managing (and currently storing) exported assets such as pdf / png and in theory csv
  • Adds Buttons for exporting to Dashboards and Insights
  • Utilises Selenium for server-side rendering of the appropriate view and exporting

👉 Stay up-to-date with PostHog coding conventions for a smoother review.

How did you test this code?

Outstanding tasks

  • Production-isation of Selenium related code - Docker support, pre-downloading of chromium, ensuring memory requirements work etc.
  • Dedicated standalone page for exporting of Dashboards / Insights instead of hacky use of SharedDashboard page
  • Ensure tracking isn't enabled for exporter agent
  • Ideally an E2E test to ensure the whole printing of an image works reliably

Misc

Closes #9851
Also closes #9799

@benjackwhite benjackwhite changed the title Dashboard / Insight exporting feat(exports): Dashboard / Insight exporting May 18, 2022
Copy link
Member

@pauldambra pauldambra left a comment

Choose a reason for hiding this comment

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

I haven't run it yet so apologies if there are some incorrect assumptions in comments

Overall this is really cool 💖

Copy link
Collaborator

@mariusandra mariusandra left a comment

Choose a reason for hiding this comment

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

Gave it a first look, left some thoughts! Overall, 🤩.

While I got one export, I managed to get stuck in a state where the next ones didn't download. I'm not sure why, as I didn't really see any debug output.

2022-05-24 12 22 32

This could be on my setup somehow as well, e.g. Chrome being flakey.

However it's a bit weird that the toast disappears for a while, and only later the export succeeds. Could we keep the toast around until either a success or failure is detected... or the user closes it themselves?

@benjackwhite benjackwhite marked this pull request as ready for review May 24, 2022 16:11
@benjackwhite
Copy link
Contributor Author

Somewhere along the way it seems i really screwed up this branch. There are bunch of changed files that I didn't change... Need to figure out what happened and somehow revert

@benjackwhite benjackwhite force-pushed the feature/exporting-poc branch from 0a606e7 to 57eb8c6 Compare May 24, 2022 17:00
# Conflicts:
#	frontend/src/lib/constants.tsx
Added separator
@benjackwhite benjackwhite requested a review from pauldambra May 27, 2022 08:46
@pauldambra
Copy link
Member

I had to update Chrome to 102 before it would try to export...

Even then it's failing for me (hopefully me being silly :))

export

@benjackwhite benjackwhite requested a review from pauldambra May 27, 2022 11:50
@benjackwhite benjackwhite merged commit 57874f9 into master May 27, 2022
@benjackwhite benjackwhite deleted the feature/exporting-poc branch May 27, 2022 12:31
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.

Shared Dashboards should not show editing controls for Insights Retention number input broken (displays NaN)
4 participants