Can not specify recipient for slack or email for Superset Reports

[steele-boolkin]

Bug description

HI,

On Superset Reports, I can't add a recipient under the notification section (for slack or email). Its almost as if the validation (that it has been supplied) does not recognise it.

On Superset Alerts i do not have this issue but it presents on Reports.

How to reproduce the bug

1. Go to Superset Reports
2. Add a new report
3. Add a slack or email recipient

Screenshots/recordings

Superset version

4.0.1

Python version

3.10

Node version

16

Browser

Chrome

Additional context

No response

Checklist

• [X] I have searched Superset docs and Slack and didn't find a solution to my problem.
• [ ] I have searched the GitHub issue tracker and didn't find a similar bug report.
• [ ] I have checked Superset's logs for errors and if I found a relevant Python stacktrace, I included it here as text in the "additional context" section.

[dosubot[bot]]

Hey @steele-boolkin! I'm here to help you with any bugs, questions, or contributions. Let's tackle this together!

The issue you're experiencing with specifying recipients for Slack or email notifications in Superset Reports, while not occurring in Superset Alerts, is due to differences in the implementation of the user interface and configuration handling between these two features.

In Superset Alerts, the AlertReportModal component allows users to dynamically add and configure notification methods, including specifying recipients for each method. This is facilitated by the NotificationMethod component, which provides a user interface for selecting notification methods and entering recipient details.

Here is a relevant part of the code from AlertReportModal.tsx:

const NotificationMethodAdd: FunctionComponent<NotificationMethodAddProps> = ({
  status = 'active',
  onClick,
}) => {
  if (status === 'hidden') {
    return null;
  }

  const checkStatus = () => {
    if (status !== 'disabled') {
      onClick();
    }
  };

  return (
    <StyledNotificationAddButton className={status} onClick={checkStatus}>
      <i className="fa fa-plus" />{' '}
      {status === 'active'
        ? t('Add another notification method')
        : t('Add delivery method')}
    </StyledNotificationAddButton>
  );
};

This component allows users to add multiple notification methods and specify recipients for each method dynamically.

In contrast, Superset Reports do not provide a similar interface for dynamically adding and configuring notification methods and recipients. The recipient configuration for reports is predefined and not adjustable at runtime, which is why you can't specify a recipient for Slack or email notifications in Superset Reports.

Additionally, the ReportSchedule model in superset/reports/models.py defines the structure for report schedules, including the relationship with ReportRecipients. However, the configuration for recipients is predefined and not designed to be dynamically adjusted through the user interface:

class ReportRecipients(Model, AuditMixinNullable):
    """
    Report Recipients, meant to support multiple notification types, eg: Slack, email
    """

    __tablename__ = "report_recipient"
    id = Column(Integer, primary_key=True)
    type = Column(String(50), nullable=False)
    recipient_config_json = Column(MediumText(), default="{}")
    report_schedule_id = Column(
        Integer, ForeignKey("report_schedule.id"), nullable=False
    )
    report_schedule = relationship(
        ReportSchedule,
        backref=backref("recipients", cascade="all,delete,delete-orphan"),
        foreign_keys=[report_schedule_id],
    )

This difference in implementation results in the observed behavior where specifying recipients is possible for alerts but not for reports ^[1][2].

Details

About Dosu This response is meant to be useful and save you time. It is not meant to be a precise solution, but rather a starting point for your own research.

Help me be more useful! Please leave a 👍 if this is helpful and 👎 if it is irrelevant.

To continue the conversation, mention @dosu.

[bennyhawk]

+1

I am facing the same issue. After a bit of shallow searching, this issue from early 2021 also seems to be somewhat similar, if it helps.

Superset | Issue 12715

[SkinnyPigeon]

We've also got the same issue

[sfirke]

Are you all talking about #28149 which was fixed in #28409? Or is this something different?

[SkinnyPigeon]

For us it is that issue and we're waiting on the fix to upgrade to 4.x.x

[giacomochiarella]

same issue here, Superset 4.0.1. Cannot change existing alerts/reports recipients and cannot create new alerts/reports because the save button is not enabled after adding all the information. Looks like the issue is in the Notification settings because all the sections have the green thick except the notification section which has a red cross

[steele-boolkin]

Are you all talking about #28149 which was fixed in #28409? Or is this something different?

Thanks !

It looks like it. Where do we see if it will be included in 4.0.2?

[sfirke]

4.0.2 hasn't been cut yet, I think @michael-s-molina is managing that release and I believe he has said to expect mid-June. When the release candidate comes out for testing and a vote, anyone can check that this is fixed. I personally would vote against a release that didn't have this fix but I'm sure it will be included.

Anyone can subscribe to the dev@apache email list to stay informed about releases. And anyone can test the release candidates and vote on them.

[sfirke]

Alright folks watching this issue: Superset 4.0.2 is up for a vote! Anyone is welcome to test out the release and vote on it. Test it by deploying the docker image: apache/superset/4.0.2rc1 or apache/superset/4.0.2rc1-dev.

Vote by subscribing to the public email list https://lists.apache.org/[email protected] and replying to the email Michael sent this morning! Please test that this bug is fixed and that everything else works as it should. I am going to copy-paste this message onto a couple of other issues for visibility, sorry to folks who get multiple messages.