Merge pull request #3144 from target/remove-slack-usergroup-expflag

chore: remove slack usergroup exp flag
target · Jul 6, 2023 · 1405360 · 1405360
2 parents 05d1854 + 9482d34
commit 1405360
Show file tree

Hide file tree

Showing 8 changed files with 67 additions and 91 deletions.
diff --git a/app/initslack.go b/app/initslack.go
@@ -3,7 +3,6 @@ package app
 import (
 	"context"
 
-	"github.com/target/goalert/expflag"
 	"github.com/target/goalert/notification"
 	"github.com/target/goalert/notification/slack"
 )
@@ -19,9 +18,7 @@ func (app *App) initSlack(ctx context.Context) error {
 	}
 	app.notificationManager.RegisterSender(notification.DestTypeSlackChannel, "Slack-Channel", app.slackChan)
 	app.notificationManager.RegisterSender(notification.DestTypeSlackDM, "Slack-DM", app.slackChan.DMSender())
-	if expflag.ContextHas(ctx, expflag.SlackUserGroups) {
-		app.notificationManager.RegisterSender(notification.DestTypeSlackUG, "Slack-UserGroup", app.slackChan.UserGroupSender())
-	}
+	app.notificationManager.RegisterSender(notification.DestTypeSlackUG, "Slack-UserGroup", app.slackChan.UserGroupSender())
 
 	return nil
 }
diff --git a/expflag/flags.go b/expflag/flags.go
@@ -5,15 +5,13 @@ import "sort"
 type Flag string
 
 const (
-	Example         Flag = "example"
-	ChanWebhook     Flag = "chan-webhook"
-	SlackUserGroups Flag = "slack-ug"
+	Example     Flag = "example"
+	ChanWebhook Flag = "chan-webhook"
 )
 
 var desc = map[Flag]string{
-	Example:         "An example experimental flag to demonstrate usage.",
-	ChanWebhook:     "Enables webhooks as a notification channel type",
-	SlackUserGroups: "Enables updating Slack user groups with schedule on-call users.",
+	Example:     "An example experimental flag to demonstrate usage.",
+	ChanWebhook: "Enables webhooks as a notification channel type",
 }
 
 // AllFlags returns a slice of all experimental flags sorted by name.

diff --git a/graphql2/graphqlapp/mutation.go b/graphql2/graphqlapp/mutation.go
@@ -65,9 +65,6 @@ func (a *Mutation) SetScheduleOnCallNotificationRules(ctx context.Context, input
 			var nfyChan *notificationchannel.Channel
 			switch r.Target.Type {
 			case assignment.TargetTypeSlackUserGroup:
-				if !expflag.ContextHas(ctx, expflag.SlackUserGroups) {
-					return validation.NewFieldError(fmt.Sprintf("Rules[%d].Target.Type", i), "Slack user groups are not enabled.")
-				}
 				grpID, chanID, _ := strings.Cut(r.Target.ID, ":")
 				grp, err := a.SlackStore.UserGroup(ctx, grpID)
 				if err != nil {

diff --git a/graphql2/graphqlapp/slack.go b/graphql2/graphqlapp/slack.go
@@ -9,12 +9,10 @@ import (
 	"strings"
 
 	"github.com/target/goalert/config"
-	"github.com/target/goalert/expflag"
 	"github.com/target/goalert/graphql2"
 	"github.com/target/goalert/notification/slack"
 	"github.com/target/goalert/permission"
 	"github.com/target/goalert/search"
-	"github.com/target/goalert/validation"
 )
 
 func (q *Query) SlackChannel(ctx context.Context, id string) (*slack.Channel, error) {
@@ -23,17 +21,11 @@ func (q *Query) SlackChannel(ctx context.Context, id string) (*slack.Channel, er
 
 // SlackUserGroup is a GraphQL resolver for a Slack user group.
 func (q *Query) SlackUserGroup(ctx context.Context, id string) (*slack.UserGroup, error) {
-	if !expflag.ContextHas(ctx, expflag.SlackUserGroups) {
-		return nil, validation.NewGenericError("Slack user groups are not enabled")
-	}
 	return q.SlackStore.UserGroup(ctx, id)
 }
 
 // SlackUserGroups is a GraphQL resolver for a list of Slack user groups.
 func (q *Query) SlackUserGroups(ctx context.Context, input *graphql2.SlackUserGroupSearchOptions) (conn *graphql2.SlackUserGroupConnection, err error) {
-	if !expflag.ContextHas(ctx, expflag.SlackUserGroups) {
-		return nil, validation.NewGenericError("Slack user groups are not enabled")
-	}
 	if input == nil {
 		input = &graphql2.SlackUserGroupSearchOptions{}
 	}

diff --git a/test/smoke/slackusergroup_test.go b/test/smoke/slackusergroup_test.go
@@ -4,7 +4,6 @@ import (
 	"testing"
 	"time"
 
-	"github.com/target/goalert/expflag"
 	"github.com/target/goalert/test/smoke/harness"
 )
 
@@ -37,7 +36,7 @@ func TestSlackUserGroups(t *testing.T) {
 	values
 		({{uuid "sid"}}, '{"V1":{"OnCallNotificationRules": [{"ChannelID": {{uuidJSON "ug"}}, "Time": "00:00" }]}}');
 `
-	h := harness.NewHarnessWithFlags(t, sql, "slack-ug", expflag.FlagSet{expflag.SlackUserGroups})
+	h := harness.NewHarness(t, sql, "slack-ug")
 
 	defer h.Close()
 

diff --git a/web/src/app/schedules/on-call-notifications/ScheduleOnCallNotificationsForm.tsx b/web/src/app/schedules/on-call-notifications/ScheduleOnCallNotificationsForm.tsx
@@ -44,7 +44,6 @@ export default function ScheduleOnCallNotificationsForm(
   const [slackEnabled] = useConfigValue('Slack.Enable')
   const [webhookEnabled] = useConfigValue('Webhook.Enable')
   const webhookChannelEnabled = useExpFlag('chan-webhook')
-  const slackUGEnabled = useExpFlag('slack-ug')
   const { zone } = useScheduleTZ(scheduleID)
 
   const handleRuleChange = (e: React.ChangeEvent<HTMLInputElement>): void => {
@@ -79,17 +78,15 @@ export default function ScheduleOnCallNotificationsForm(
       >
         SLACK CHANNEL
       </MenuItem>,
-      ...(slackUGEnabled
-        ? [
-            <MenuItem
-              key='SLACK_UG'
-              value='slackUserGroup'
-              disabled={!slackEnabled}
-            >
-              SLACK USER GROUP
-            </MenuItem>,
-          ]
-        : []),
+      [
+        <MenuItem
+          key='SLACK_UG'
+          value='slackUserGroup'
+          disabled={!slackEnabled}
+        >
+          SLACK USER GROUP
+        </MenuItem>,
+      ],
       ...(webhookChannelEnabled
         ? [
             <MenuItem
@@ -102,7 +99,7 @@ export default function ScheduleOnCallNotificationsForm(
           ]
         : []),
     ],
-    [slackEnabled, slackUGEnabled, webhookEnabled, webhookChannelEnabled],
+    [slackEnabled, webhookEnabled, webhookChannelEnabled],
   )
 
   function renderTypeFields(type: TargetType): JSX.Element {

diff --git a/web/src/cypress/e2e/schedules.cy.ts b/web/src/cypress/e2e/schedules.cy.ts
@@ -1,5 +1,5 @@
 import { Chance } from 'chance'
-import { testScreen, testScreenWithFlags } from '../support/e2e'
+import { testScreen } from '../support/e2e'
 import { Schedule, ScheduleTarget } from '../../schema'
 import users from '../fixtures/users.json'
 
@@ -618,62 +618,58 @@ function testSchedules(screen: ScreenFormat): void {
 
 testScreen('Schedules', testSchedules)
 
-testScreenWithFlags(
-  'Slack User Group Support',
-  (screen: ScreenFormat) => {
-    describe('Schedule On-Call Notifications', () => {
-      let sched: Schedule
-      it('should create notification rules with slack user groups', () => {
-        cy.createSchedule({ timeZone: 'UTC' }).then((s: Schedule) => {
-          sched = s
-          return cy.visit('/schedules/' + sched.id + '/on-call-notifications')
-        })
+testScreen('Slack User Group Support', (screen: ScreenFormat) => {
+  describe('Schedule On-Call Notifications', () => {
+    let sched: Schedule
+    it('should create notification rules with slack user groups', () => {
+      cy.createSchedule({ timeZone: 'UTC' }).then((s: Schedule) => {
+        sched = s
+        return cy.visit('/schedules/' + sched.id + '/on-call-notifications')
+      })
 
-        // on change
-        if (screen === 'mobile') {
-          cy.pageFab()
-        } else {
-          cy.get('button').contains('Create Notification Rule').click()
-        }
+      // on change
+      if (screen === 'mobile') {
+        cy.pageFab()
+      } else {
+        cy.get('button').contains('Create Notification Rule').click()
+      }
 
-        cy.dialogTitle('Create Notification Rule')
-        cy.dialogForm({
-          ruleType: 'on-change',
-          notificationType: 'SLACK USER GROUP',
-          selectUserGroup: 'foobar',
-          errorChannel: 'foobar',
-        })
+      cy.dialogTitle('Create Notification Rule')
+      cy.dialogForm({
+        ruleType: 'on-change',
+        notificationType: 'SLACK USER GROUP',
+        selectUserGroup: 'foobar',
+        errorChannel: 'foobar',
+      })
 
-        cy.dialogFinish('Submit')
-        cy.get('body').should('contain', '#foobar')
-        cy.get('body').should('contain', 'Notifies when on-call changes')
+      cy.dialogFinish('Submit')
+      cy.get('body').should('contain', '#foobar')
+      cy.get('body').should('contain', 'Notifies when on-call changes')
 
-        // time of day
-        if (screen === 'mobile') {
-          cy.pageFab()
-        } else {
-          cy.get('button').contains('Create Notification Rule').click()
-        }
-        cy.dialogTitle('Create Notification Rule')
-        cy.dialogForm({
-          targetID: 'foobar',
-          ruleType: 'time-of-day',
-          notificationType: 'SLACK USER GROUP',
-          selectUserGroup: 'foobar',
-          errorChannel: 'foobar',
-          time: '00:00',
-          'weekdayFilter[0]': false,
-          'weekdayFilter[1]': true,
-          'weekdayFilter[2]': false,
-          'weekdayFilter[3]': false,
-          'weekdayFilter[4]': false,
-          'weekdayFilter[5]': false,
-          'weekdayFilter[6]': false,
-        })
-        cy.dialogFinish('Submit')
-        cy.get('#content').should('contain', 'Notifies Mon at 12:00 AM')
+      // time of day
+      if (screen === 'mobile') {
+        cy.pageFab()
+      } else {
+        cy.get('button').contains('Create Notification Rule').click()
+      }
+      cy.dialogTitle('Create Notification Rule')
+      cy.dialogForm({
+        targetID: 'foobar',
+        ruleType: 'time-of-day',
+        notificationType: 'SLACK USER GROUP',
+        selectUserGroup: 'foobar',
+        errorChannel: 'foobar',
+        time: '00:00',
+        'weekdayFilter[0]': false,
+        'weekdayFilter[1]': true,
+        'weekdayFilter[2]': false,
+        'weekdayFilter[3]': false,
+        'weekdayFilter[4]': false,
+        'weekdayFilter[5]': false,
+        'weekdayFilter[6]': false,
       })
+      cy.dialogFinish('Submit')
+      cy.get('#content').should('contain', 'Notifies Mon at 12:00 AM')
     })
-  },
-  ['slack-ug'],
-)
+  })
+})
diff --git a/web/src/expflag.d.ts b/web/src/expflag.d.ts
@@ -1,3 +1,3 @@
 // Code generated by expflag/cmd/tsgen DO NOT EDIT.
 
-type ExpFlag = 'chan-webhook' | 'example' | 'slack-ug'
+type ExpFlag = 'chan-webhook' | 'example'