feat: allow select tabs for scheduled delivery #11266

Shifan-Gu · 2024-08-26T13:39:53Z

Closes:

Description:

This change allows user to select the tabs of the contents which they want include in the scheduled delivery.

UX:
When include all checkbox is selected, select tabs dropdown would be disabled and scheduled delivery will include all tabs.

When include all checkbox is not selected, then user can select the tabs they want include in the scheduled delivery

Then scheduled delivery will filter content based on the tabs selected.

Reviewer actions

I have manually tested the changes in the preview environment
I have reviewed the code
I understand that "request changes" will block this PR from merging

netlify · 2024-08-26T13:39:56Z

👷 Deploy request for peaceful-bassi-cbf284 pending review.

Visit the deploys page to approve it

Name	Link
🔨 Latest commit	`2b35ab3`

hamzahc1 · 2024-09-01T14:10:51Z

@Shifan-Gu do you have the link to the ticket that this PR relates to?

PriPatel · 2024-09-01T15:46:23Z

@hamzahc1 The dev work for this ticket started before I finished speccing the milestone, so there is no spec for this. I can reopen the ticket.

The solution was agreed over a figma file. If I can get a render environment, I'll be able to test this and make sure everything works as expected.

All other changes in the future link to well-specced tickets. this is just an exception!

ZeRego · 2024-09-02T12:54:33Z

@PriPatel
PR env: https://lightdash-pr-11336.onrender.com/login
Reminder that emails are in a sandbox and not sent to your email inbox. Let me know if you need help with this.

PriPatel

The checkbox should be selected by default.
Change Dashboard Tabs to Tabs
!IMPORTANT : If i dont have multiple dashboard tabs, I should not see this option in the form at all. Right now, this is affecting users who don't have multiple tabs and it is confusing
This should also affect the delivery if the user has 'image' selected. Right now it only affects .csvs

Shifan-Gu · 2024-09-02T13:52:41Z

* [ ] The checkbox should be selected by default. * [ ] Change `Dashboard Tabs` to `Tabs` * [ ] !IMPORTANT : If i dont have multiple dashboard tabs, I should not see this option in the form at all. Right now, this is affecting users who don't have multiple tabs and it is confusing

Thank you for the comments! @PriPatel, i will update it.

…cheduled-delivery

…livery' into feat/select-tabs-in-scheduled-delivery

rephus

I think we can improve the types on db, replace jsonb with string[].
The rest looks good, once the types are fixed I'll test this on render

packages/backend/src/database/migrations/20240822143117_add_scheduler_selected_tabs_column.ts

packages/backend/src/database/entities/scheduler.ts

rephus · 2024-09-20T06:54:45Z

packages/backend/src/scheduler/SchedulerTask.ts

+                }${
+                    selectedTabs
+                        ? `?selectedTabs=${encodeURI(
+                              JSON.stringify(selectedTabs),


you don't need to do stringify if you use text[] in db

i removed the stringify call in the model layer, but keep this call because it seems a straightforward way to encode the string array and decode it at the endpoint.

Ok, that works, another option is to pass an array using the query parameter multiple times like.

?selectedTabs=tab0&selectedTabs=tab1...

Both ways are accepted

rephus · 2024-09-20T06:55:43Z

packages/frontend/src/features/scheduler/components/SchedulerForm.tsx

@@ -235,13  240,39 @@ const SchedulerForm: FC<Props> = ({
    isThresholdAlert,
    itemsMap,
 }) => {
+    const isDashboard = resource && resource.type === 'dashboard';
+    const isDashboardTabsEnabled = useFeatureFlagEnabled(


rephus · 2024-09-20T06:56:55Z

packages/frontend/src/features/scheduler/components/SchedulerForm.tsx

+                ? getFormValuesFromScheduler({
+                      ...savedSchedulerData,
+                      ...(isDashboardScheduler(savedSchedulerData) && {
+                          selectedTabs: isDashboardTabsAvailable


we can probably extract all this logic in a variable outside the method call, just for readability.

rephus · 2024-09-20T06:58:13Z

packages/frontend/src/features/scheduler/components/SchedulerForm.tsx

+                                        size="xs"
+                                        label="Include all tabs"
+                                        labelPosition="right"
+                                        checked={allTabsSelected}


this should be enabled by default, right ?

yes, but this is not done by setting a default value for "allTabsSelected", but instead by setting initial the form value "selectedTabs" to include all the tabs, then dynamically derive the value from this form value.

…at/select-tabs-in-scheduled-delivery

rephus

The code look ok, but when I was testing, I found some issues.

CSV export

not selecting any tab in the csv export, sends an email with nothing in it. Ideally we should prevent the scheduler from being created in the first place... This is a minor bug , which means we could merge this PR without this, and create a separate issue to fix it later.

Image export

It seems the tab filter is not working, if I try to filter a tab, it always shows me everything. I tested /minimal and hte parameter seems to work, perhaps we are not passing the paramenter correctly inside the headless browser.
selecting no tabs simply returns all tabs, I believe this is related with the previous point, where we don't do any filtering. Similar to the CSV UI, we should not allow the scheduler to be created.

I also tested this without tabs, and it works as expected, so it doesn't break dashboards without tabs 👍 that's great.

rephus · 2024-09-24T07:02:19Z

packages/backend/src/models/SchedulerModel/index.ts

@@ -317,6  323,10 @@ export class SchedulerModel {
                        : null,
                    notification_frequency:
                        scheduler.notificationFrequency || null,
+                    selected_tabs:
+                        'selectedTabs' in scheduler && scheduler.selectedTabs
+                            ? (scheduler.selectedTabs as string[])


is this casting needed here ?

Shifan-Gu · 2024-09-25T11:09:17Z

The code look ok, but when I was testing, I found some issues.

CSV export

not selecting any tab in the csv export, sends an email with nothing in it. Ideally we should prevent the scheduler from being created in the first place... This is a minor bug , which means we could merge this PR without this, and create a separate issue to fix it later.

Image export

It seems the tab filter is not working, if I try to filter a tab, it always shows me everything. I tested /minimal and hte parameter seems to work, perhaps we are not passing the paramenter correctly inside the headless browser.

selecting no tabs simply returns all tabs, I believe this is related with the previous point, where we don't do any filtering. Similar to the CSV UI, we should not allow the scheduler to be created.

I also tested this without tabs, and it works as expected, so it doesn't break dashboards without tabs 👍 that's great.

Hi @rephus thanks for the testing work!

I have some issue with my local env to reproduce the issue, so haven't figure out the root cause for the issue. but i have a question for you is that, are you using standalone scheduler deployment to test this change?

rephus

Nice work

# [0.1274.0](0.1273.5...0.1274.0) (2024-09-26) ### Features * allow select tabs for scheduled delivery ([#11266](#11266)) ([55587b8](55587b8))

lightdash-bot · 2024-09-26T07:34:53Z

🎉 This PR is included in version 0.1274.0 🎉

The release is available on:

0.1274.0
GitHub release

Your semantic-release bot 📦🚀

* feat: allow select tabs for scheduled delivery * support image option * handle delete tabs * handle delete tabs * change selected tabs db type * fix image export issue

# [0.1274.0](lightdash/lightdash@0.1273.5...0.1274.0) (2024-09-26) ### Features * allow select tabs for scheduled delivery ([lightdash#11266](lightdash#11266)) ([55587b8](lightdash@55587b8))

feat: allow select tabs for scheduled delivery

1fcde1e

Merge branch 'main' into feat/select-tabs-in-scheduled-delivery

976a7ae

hamzahc1 requested review from a team and ZeRego and removed request for a team September 1, 2024 14:10

owlas requested a deployment to duplicate_feat/select-tabs-in-scheduled-delivery - jaffle_db_pg_13 PR #11336 September 2, 2024 12:18 — with Render Abandoned

owlas temporarily deployed to duplicate_feat/select-tabs-in-scheduled-delivery - headless-browser PR #11336 September 2, 2024 12:18 — with Render Destroyed

owlas temporarily deployed to duplicate_feat/select-tabs-in-scheduled-delivery - lightdash PR #11336 September 2, 2024 12:19 — with Render Destroyed

ZeRego requested a review from PriPatel September 2, 2024 12:53

ZeRego self-assigned this Sep 2, 2024

ZeRego mentioned this pull request Sep 2, 2024

feat: select tabs in scheduled delivery [duplicate] #11336

Closed

PriPatel reviewed Sep 2, 2024

View reviewed changes

Shifan-Gu added 5 commits September 3, 2024 15:16

support image option

1021511

handle delete tabs

50cf3ad

handle delete tabs

7c67896

Merge remote-tracking branch 'origin/main' into feat/select-tabs-in-s…

51cd0d6

…cheduled-delivery

Merge remote-tracking branch 'origin/feat/select-tabs-in-scheduled-de…

e00a076

…livery' into feat/select-tabs-in-scheduled-delivery

ZeRego mentioned this pull request Sep 3, 2024

feat/select tabs in scheduled delivery [duplicate] #11375

Closed

owlas requested a deployment to duplicate_feat/select-tabs-in-scheduled-delivery - jaffle_db_pg_13 PR #11375 September 3, 2024 15:50 — with Render Abandoned

owlas deployed to duplicate_feat/select-tabs-in-scheduled-delivery - headless-browser PR #11375 September 3, 2024 15:50 — with Render Active

owlas deployed to duplicate_feat/select-tabs-in-scheduled-delivery - lightdash PR #11375 September 3, 2024 15:51 — with Render View deployment

ZeRego requested a review from PriPatel September 4, 2024 09:37

owlas deployed to duplicate_feat/select-tabs-in-scheduled-delivery - lightdash PR #11375 September 6, 2024 17:09 — with Render View deployment

owlas deployed to duplicate_feat/select-tabs-in-scheduled-delivery - headless-browser PR #11375 September 6, 2024 17:09 — with Render Active

owlas deployed to duplicate_feat/select-tabs-in-scheduled-delivery - lightdash PR #11375 September 10, 2024 17:09 — with Render View deployment

ZeRego removed their request for review September 16, 2024 11:20

ZeRego removed their assignment Sep 16, 2024

rephus requested changes Sep 20, 2024

View reviewed changes

Shifan-Gu added 2 commits September 23, 2024 14:47

Merge branch 'main' of https://github.com/Shifan-Gu/lightdash into fe…

abb06f0

…at/select-tabs-in-scheduled-delivery

change selected tabs db type

90ce2f9

Shifan-Gu requested a review from rephus September 23, 2024 15:27

joaoviana assigned rephus Sep 23, 2024

owlas temporarily deployed to duplicate_feat/select-tabs-in-scheduled-delivery - lightdash PR #11375 September 24, 2024 07:02 — with Render Destroyed

owlas temporarily deployed to duplicate_feat/select-tabs-in-scheduled-delivery - headless-browser PR #11375 September 24, 2024 07:02 — with Render Destroyed

owlas temporarily deployed to duplicate_feat/select-tabs-in-scheduled-delivery - headless-browser PR #11375 September 24, 2024 07:03 — with Render Destroyed

rephus requested changes Sep 24, 2024

View reviewed changes

fix image export issue

2b35ab3

Shifan-Gu requested a review from rephus September 25, 2024 20:24

rephus approved these changes Sep 26, 2024

View reviewed changes

rephus merged commit 55587b8 into lightdash:main Sep 26, 2024
6 checks passed

lightdash-bot pushed a commit that referenced this pull request Sep 26, 2024

chore(release): 0.1274.0

c5e7671

# [0.1274.0](0.1273.5...0.1274.0) (2024-09-26) ### Features * allow select tabs for scheduled delivery ([#11266](#11266)) ([55587b8](55587b8))

lightdash-bot added the released label Sep 26, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: allow select tabs for scheduled delivery #11266

feat: allow select tabs for scheduled delivery #11266

Shifan-Gu commented Aug 26, 2024

netlify bot commented Aug 26, 2024 •

edited

Loading

hamzahc1 commented Sep 1, 2024

PriPatel commented Sep 1, 2024 •

edited

Loading

ZeRego commented Sep 2, 2024

PriPatel left a comment •

edited

Loading

Shifan-Gu commented Sep 2, 2024

rephus left a comment

rephus Sep 20, 2024

Shifan-Gu Sep 23, 2024

rephus Sep 24, 2024

rephus Sep 20, 2024

rephus Sep 20, 2024

rephus Sep 20, 2024

Shifan-Gu Sep 23, 2024

rephus left a comment

rephus Sep 24, 2024

Shifan-Gu commented Sep 25, 2024

CSV export

Image export

rephus left a comment

lightdash-bot commented Sep 26, 2024

feat: allow select tabs for scheduled delivery #11266

feat: allow select tabs for scheduled delivery #11266

Conversation

Shifan-Gu commented Aug 26, 2024

Description:

Reviewer actions

netlify bot commented Aug 26, 2024 • edited Loading

👷 Deploy request for peaceful-bassi-cbf284 pending review.

hamzahc1 commented Sep 1, 2024

PriPatel commented Sep 1, 2024 • edited Loading

ZeRego commented Sep 2, 2024

PriPatel left a comment • edited Loading

Choose a reason for hiding this comment

Shifan-Gu commented Sep 2, 2024

rephus left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rephus left a comment

Choose a reason for hiding this comment

CSV export

Image export

Choose a reason for hiding this comment

Shifan-Gu commented Sep 25, 2024

CSV export

Image export

rephus left a comment

Choose a reason for hiding this comment

lightdash-bot commented Sep 26, 2024

netlify bot commented Aug 26, 2024 •

edited

Loading

PriPatel commented Sep 1, 2024 •

edited

Loading

PriPatel left a comment •

edited

Loading