Use PUT since this endpoint is idempotent

Signed-off-by: Danny Kopping <danny@coder.com>
coder · dannykopping · Aug 1, 2024 · Aug 1, 2024 · Aug 1, 2024 · Jul 25, 2024
commit 44a700b06240b48c1e6af7d1c15e5099c8707e8b
diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go
diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json
diff --git a/coderd/coderd.go b/coderd/coderd.go
@@ -1250,7 +1250,7 @@ func New(options *Options) *API {
 			r.Get("/settings", api.notificationsSettings)
 			r.Put("/settings", api.putNotificationsSettings)
 			r.Route("/templates", func(r chi.Router) {
-				r.Get("/system", api.getSystemNotificationTemplates)
+				r.Get("/system", api.systemNotificationTemplates)
 			})
 		})
 	})

diff --git a/coderd/notifications.go b/coderd/notifications.go
@@ -128,7 +128,7 @@ func (api *API) putNotificationsSettings(rw http.ResponseWriter, r *http.Request
 // @Tags Notifications
 // @Success 200 {array} codersdk.NotificationTemplate
 // @Router /notifications/templates/system [get]
-func (api *API) getSystemNotificationTemplates(rw http.ResponseWriter, r *http.Request) {
+func (api *API) systemNotificationTemplates(rw http.ResponseWriter, r *http.Request) {
 	ctx := r.Context()
 
 	templates, err := api.Database.GetNotificationTemplatesByKind(ctx, database.NotificationTemplateKindSystem)

diff --git a/codersdk/notifications.go b/codersdk/notifications.go
@@ -62,7 +62,7 @@ func (c *Client) PutNotificationsSettings(ctx context.Context, settings Notifica
 // UpdateNotificationTemplateMethod modifies a notification template to use a specific notification method, overriding
 // the method set in the deployment configuration.
 func (c *Client) UpdateNotificationTemplateMethod(ctx context.Context, notificationTemplateId uuid.UUID, method string) error {
-	res, err := c.Request(ctx, http.MethodPost,
+	res, err := c.Request(ctx, http.MethodPut,
 		fmt.Sprintf("/api/v2/notifications/templates/%s/method", notificationTemplateId),
 		UpdateNotificationTemplateMethod{Method: method},
 	)

diff --git a/docs/api/enterprise.md b/docs/api/enterprise.md
diff --git a/enterprise/coderd/coderd.go b/enterprise/coderd/coderd.go
@@ -395,7 +395,7 @@ func New(ctx context.Context, options *Options) (_ *API, err error) {
 			apiKeyMiddleware,
 			httpmw.RequireExperiment(api.AGPL.Experiments, codersdk.ExperimentNotifications),
 			httpmw.ExtractNotificationTemplateParam(options.Database),
-		).Post("/notifications/templates/{notification_template}/method", api.updateNotificationTemplateMethod)
+		).Put("/notifications/templates/{notification_template}/method", api.updateNotificationTemplateMethod)
 	})
 
 	if len(options.SCIMAPIKey) != 0 {

diff --git a/enterprise/coderd/notifications.go b/enterprise/coderd/notifications.go
@@ -20,7 +20,7 @@ import (
 // @Produce json
 // @Tags Enterprise
 // @Success 200
-// @Router /notifications/templates/{notification_template}/method [post]
+// @Router /notifications/templates/{notification_template}/method [put]
 func (api *API) updateNotificationTemplateMethod(rw http.ResponseWriter, r *http.Request) {
 	// TODO: authorization (restrict to admin/template admin?)
 	var (

diff --git a/enterprise/coderd/notifications_test.go b/enterprise/coderd/notifications_test.go
@@ -148,61 +148,6 @@ func TestUpdateNotificationTemplateMethod(t *testing.T) {
 		require.NotNil(t, template)
 		require.Equal(t, method, template.Method)
 	})
-
-	// t.Run("Settings modified", func(t *testing.T) {
-	// 	t.Parallel()
-	//
-	// 	client := coderdtest.New(t, nil)
-	// 	_ = coderdtest.CreateFirstUser(t, client)
-	//
-	// 	// given
-	// 	expected := codersdk.NotificationsSettings{
-	// 		NotifierPaused: true,
-	// 	}
-	//
-	// 	ctx := testutil.Context(t, testutil.WaitShort)
-	//
-	// 	// when
-	// 	err := client.PutNotificationsSettings(ctx, expected)
-	// 	require.NoError(t, err)
-	//
-	// 	// then
-	// 	actual, err := client.GetNotificationsSettings(ctx)
-	// 	require.NoError(t, err)
-	// 	require.Equal(t, expected, actual)
-	// })
-	//
-	// t.Run("Settings not modified", func(t *testing.T) {
-	// 	t.Parallel()
-	//
-	// 	// Empty state: notifications Settings are undefined now (default).
-	// 	client := coderdtest.New(t, nil)
-	// 	_ = coderdtest.CreateFirstUser(t, client)
-	// 	ctx := testutil.Context(t, testutil.WaitShort)
-	//
-	// 	// Change the state: pause notifications
-	// 	err := client.PutNotificationsSettings(ctx, codersdk.NotificationsSettings{
-	// 		NotifierPaused: true,
-	// 	})
-	// 	require.NoError(t, err)
-	//
-	// 	// Verify the state: notifications are paused.
-	// 	actual, err := client.GetNotificationsSettings(ctx)
-	// 	require.NoError(t, err)
-	// 	require.True(t, actual.NotifierPaused)
-	//
-	// 	// Change the stage again: notifications are paused.
-	// 	expected := actual
-	// 	err = client.PutNotificationsSettings(ctx, codersdk.NotificationsSettings{
-	// 		NotifierPaused: true,
-	// 	})
-	// 	require.NoError(t, err)
-	//
-	// 	// Verify the state: notifications are still paused, and there is no error returned.
-	// 	actual, err = client.GetNotificationsSettings(ctx)
-	// 	require.NoError(t, err)
-	// 	require.Equal(t, expected.NotifierPaused, actual.NotifierPaused)
-	// })
 }
 
 func getTemplateById(t *testing.T, ctx context.Context, api *codersdk.Client, id uuid.UUID) (*codersdk.NotificationTemplate, error) {