Skip to content
Snippets Groups Projects
Unverified Commit 85d31205 authored by adam-james's avatar adam-james Committed by GitHub
Browse files

Migrate PulseCards include_csv = true when the card has :display table (#36383)

* Migrate PulseCards include_csv = true when the card has :display table

Why? This is so that we can eliminate the behaviour of csv attachments being included in emails even when a user
doesn't have them seleted in the dashboard subscription pane. This is confusing behaviour and undesirable in many
cases.

The likely better behaviour is that attachments for any card are only included if explicitly asked for by a user. But,
in order to change the app to behave this way, we should first migrate things so that any user who relies on this old
behavior will still see the attachments they have come to expect.

This PR is still draft and some things must happen still:

 - [ ] the migration is incomplete! At the moment, this will include .csv attachments even for tables with n-rows <
 10. This migration would cause users to see more .csv attachments than before. So, the migration should only set
 include_csv = true when the row count of the results is greater than 10.
 - [ ] the migration # is NOT claimed in the slack channel yet. This should be claimed as soon as the migration
 approach is reviewed to work and an appropriate issue for this change is linked.

* Use the proper migration # which is now claimed appropriately

* Change SQL to work with H2

* Attachment message logic depends on include_csv/xls values now

Also lift the attachment message inclusion up a level so that it can be applied to all card types, not just tables.

* Fix a failing test

* Change copy for when attachments are included

Also change a test to explicitly include csv (since now that's the only way csvs will be attached).

* Fix test
parent d45bae64
No related merge requests found
Loading
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment