feat: add active email toggle and dropdown on dashboard #15852

AAfghahi · 2021-07-23T02:03:50Z

SUMMARY

This is part of a redesign of how users can add reports in superset. Specifically, that people can add a report on a dashboard. This feature adds a dropdown when there is a report present, so that you can toggle its active status.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Screen.Recording.2021-07-21.at.4.34.02.PM.mov

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

Has associated issue:
Changes UI
Includes DB Migration (follow approval process in SIP-59)
- Migration is atomic, supports rollback & is backwards-compatible
- Confirm DB migration upgrade and downgrade tested
- Runtime estimates and downtime expectations provided
Introduces new feature or API
Removes existing feature or API

codecov · 2021-07-23T02:32:16Z

Codecov Report

Merging #15852 (bffde76) into selfSubscribeReports (9e31846) will decrease coverage by 0.05%.
The diff coverage is 24.48%.

@@                   Coverage Diff                    @@
##           selfSubscribeReports   #15852      +/-   ##
========================================================
- Coverage                 77.02%   76.97%   -0.06%     
========================================================
  Files                       988      989       +1     
  Lines                     51955    51997      +42     
  Branches                   7090     7093       +3     
========================================================
+ Hits                      40020    40026       +6     
- Misses                    11710    11746      +36     
  Partials                    225      225

Flag	Coverage Δ
javascript	`71.70% <24.48%> (-0.10%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...ntend/src/dashboard/containers/DashboardHeader.jsx	`100.00% <ø> (ø)`
...erset-frontend/src/reports/reducers/reportState.js	`26.66% <0.00%> (-30.48%)`	⬇️
...perset-frontend/src/reports/actions/reportState.js	`30.76% <12.50%> (-29.24%)`	⬇️
...frontend/src/dashboard/components/Header/index.jsx	`65.47% <33.33%> (ø)`
.../ReportModal/HeaderReportActionsDropdown/index.tsx	`38.88% <38.88%> (ø)`
superset-frontend/src/common/components/index.tsx	`100.00% <100.00%> (ø)`
superset-frontend/src/components/Select/Select.tsx	`73.05% <0.00%> (-1.37%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9e31846...bffde76. Read the comment docs.

eschutho · 2021-07-23T19:41:35Z

superset-frontend/src/dashboard/components/Header/HeaderReportActionsDropdown/index.tsx

+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React, { useState } from 'react';


do you think we can put this dropdown in the same root folder as the report modal?

eschutho · 2021-07-23T19:42:13Z

superset-frontend/src/dashboard/components/Header/HeaderReportActionsDropdown/index.tsx

+}: {
+  showReportModal: () => void;
+  hideReportModal: () => void;
+  report: JsonObject;


can we define a type for the report?

eschutho · 2021-07-23T19:43:09Z

superset-frontend/src/dashboard/components/Header/HeaderReportActionsDropdown/index.tsx

+  // useOnClickOutside(ref, () => setVisible(false));
+
+  const toggleActive = async (data: AlertObject, checked: boolean) => {
+    if (data && data.id) {


optional chaining

eschutho · 2021-07-23T19:44:16Z

superset-frontend/src/dashboard/components/Header/HeaderReportActionsDropdown/index.tsx

+  const toggleActive = async (data: AlertObject, checked: boolean) => {
+    if (data && data.id) {
+      const update_id = data.id;
+      await updateResource(update_id, { active: checked }).then(() => {


the resource hooks should automatically update the state and return them. Can you use that instead of having separate state here?

eschutho · 2021-07-23T19:47:33Z

superset-frontend/src/dashboard/components/Header/index.jsx

@@ -156,7 +157,7 @@ class Header extends React.PureComponent {
    this.hidePropertiesModal = this.hidePropertiesModal.bind(this);
    this.showReportModal = this.showReportModal.bind(this);
    this.hideReportModal = this.hideReportModal.bind(this);
-    this.handleReportClick = this.handleReportClick.bind(this);
+    this.reportModal = this.reportModal.bind(this);


can we make this more "action/verb" sounding for a method, like renderReportModal, or else we can make it a separate component outside the header (could be the same file)

yeah I wasn't happy with this and couldn't place my finger on it, I had spaced on using render, sorry.

eschutho · 2021-07-23T19:50:42Z

superset-frontend/src/dashboard/components/Header/index.jsx

-    if (!attachedReportExists) {
-      this.showReportModal();
-    }
+    return !attachedReportExists ? (


can you flip the return logic so that you can lead with the positive statement rather than negative? Just for ease of readability.

yes, that makes so much sense.

eschutho

looks good. Just a few small things

eschutho · 2021-07-26T19:04:34Z

superset-frontend/src/dashboard/components/Header/index.jsx

+        report={this.props.report.result}
+      />
+    ) : (
+      <>


nit for another iteration, but I don't think you need these extra brackets since you only have one child

eschutho · 2021-07-26T19:06:52Z

Just need to rebase some extra commits out of here before we merge.

* first draft * added testing props * created Api call * added click logic * made the fetch report into a action/reducer * abstracted report action * first commit * toggle started * on blue * event listener * toggle * toggle active + dropdown * fixed cypress tests * revisions * removed useState * added toggle active to state

pull-request-size bot added the size/L label Jul 23, 2021

AAfghahi force-pushed the ch19374_reportsDropdown branch 2 times, most recently from e6cff06 to f09ac62 Compare July 23, 2021 02:08

eschutho reviewed Jul 23, 2021

View reviewed changes

eschutho requested changes Jul 23, 2021

View reviewed changes

AAfghahi force-pushed the ch19374_reportsDropdown branch 3 times, most recently from 751bf99 to 4be1a8f Compare July 23, 2021 22:05

eschutho force-pushed the selfSubscribeReports branch 4 times, most recently from cb3896e to b81f120 Compare July 26, 2021 18:08

eschutho reviewed Jul 26, 2021

View reviewed changes

eschutho approved these changes Jul 26, 2021

View reviewed changes

AAfghahi force-pushed the ch19374_reportsDropdown branch from f52816a to 522a161 Compare July 26, 2021 21:11

AAfghahi added 8 commits July 26, 2021 17:13

first draft

78e23a8

added testing props

3466aee

created Api call

330f678

added click logic

176b068

made the fetch report into a action/reducer

27e288d

abstracted report action

2c7b2a6

first commit

42a9c49

toggle started

3573a43

AAfghahi added 7 commits July 26, 2021 17:13

on blue

8dc47f0

event listener

d2f7816

toggle

07d5ac8

toggle active + dropdown

91be237

fixed cypress tests

bf08919

revisions

2ae57eb

removed useState

2ef298d

AAfghahi force-pushed the ch19374_reportsDropdown branch 5 times, most recently from c71c4b5 to fac1f59 Compare July 27, 2021 20:32

added toggle active to state

bffde76

AAfghahi force-pushed the ch19374_reportsDropdown branch from fac1f59 to bffde76 Compare July 27, 2021 20:35

eschutho merged commit 4d5f17d into apache:selfSubscribeReports Jul 27, 2021

eschutho deleted the ch19374_reportsDropdown branch July 27, 2021 22:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add active email toggle and dropdown on dashboard #15852

feat: add active email toggle and dropdown on dashboard #15852

AAfghahi commented Jul 23, 2021

codecov bot commented Jul 23, 2021 •

edited

Loading

eschutho Jul 23, 2021

eschutho Jul 23, 2021

eschutho Jul 23, 2021

eschutho Jul 23, 2021

eschutho Jul 23, 2021

AAfghahi Jul 23, 2021

eschutho Jul 23, 2021

AAfghahi Jul 23, 2021

eschutho left a comment

eschutho Jul 26, 2021

eschutho commented Jul 26, 2021

feat: add active email toggle and dropdown on dashboard #15852

feat: add active email toggle and dropdown on dashboard #15852

Conversation

AAfghahi commented Jul 23, 2021

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

codecov bot commented Jul 23, 2021 • edited Loading

Codecov Report

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

eschutho left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eschutho commented Jul 26, 2021

codecov bot commented Jul 23, 2021 •

edited

Loading