test(feedback): Write some test for feedback utils and error returns

packages/core/src/feedback.md

@@ -0,0 +1,42 @@
+For reference, the fully built event looks something like this:
+
+```json
+{
+  "type": "feedback",
+  "event_id": "d2132d31b39445f1938d7e21b6bf0ec4",
+  "timestamp": 1597977777.6189718,
+  "dist": "1.12",
+  "platform": "javascript",
+  "environment": "production",
+  "release": 42,
+  "tags": { "transaction": "/organizations/:orgId/performance/:eventSlug/" },
+  "sdk": { "name": "name", "version": "version" },
+  "user": {
+    "id": "123",
+    "username": "user",
+    "email": "user@site.com",
+    "ip_address": "192.168.11.12"
+  },
+  "request": {
+    "url": null,
+    "headers": {
+      "user-Agent": "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/15.5 Safari/605.1.15"
+    }
+  },
+  "contexts": {
+    "feedback": {
+      "message": "test message",
+      "contact_email": "test@example.com",
+      "type": "feedback"
+    },
+    "trace": {
+      "trace_id": "4C79F60C11214EB38604F4AE0781BFB2",
+      "span_id": "FA90FDEAD5F74052",
+      "type": "trace"
+    },
+    "replay": {
+      "replay_id": "e2d42047b1c5431c8cba85ee2a8ab25d"
+    }
+  }
+}
+```

packages/core/src/feedback.ts

@@ -12,6 +12,7 @@ export function captureFeedback(
 ): string {
   const { message, name, email, url, source, associatedEventId, tags } = params;
 
+  // See https://github.com/getsentry/sentry-javascript/blob/main/packages/core/src/feedback.md for an example feedback object
   const feedbackEvent: FeedbackEvent = {
     contexts: {
       feedback: dropUndefinedKeys({

packages/feedback/src/core/sendFeedback.test.ts

@@ -30,6 +30,27 @@ describe('sendFeedback', () => {
     patchedDecoder && delete global.window.TextDecoder;
   });
 
+  it('fails when the message is falsey', async () => {
+    mockSdk();
+
+    expect(() =>
+      sendFeedback({
+        message: '',
+      }),
+    ).toThrowError(new Error('Unable to submit feedback with empty message.'));
+  });
+
+  it('fails when the client is missing', async () => {
+    mockSdk();
+    getCurrentScope().setClient(undefined);
+
+    expect(() =>
+      sendFeedback({
+        message: 'mi',
+      }),
+    ).toThrowError(new Error('No client setup, cannot send feedback.'));
+  });
+
   it('sends feedback with minimal options', async () => {
     mockSdk();
     const mockTransport = jest.spyOn(getClient()!.getTransport()!, 'send');

packages/feedback/src/core/sendFeedback.ts

@@ -13,7 +13,7 @@ export const sendFeedback: SendFeedback = (
   hint: EventHint & { includeReplay?: boolean } = { includeReplay: true },
 ): Promise<string> => {
   if (!params.message) {
-    throw new Error('Unable to submit feedback with empty message');
+    throw new Error('Unable to submit feedback with empty message.');
   }
 
   // We want to wait for the feedback to be sent (or not)
@@ -26,6 +26,7 @@ export const sendFeedback: SendFeedback = (
   if (params.tags && Object.keys(params.tags).length) {
     getCurrentScope().setTags(params.tags);
   }
+  // See https://github.com/getsentry/sentry-javascript/blob/main/packages/core/src/feedback.md for an example feedback object
   const eventId = captureFeedback(
     {
       source: FEEDBACK_API_SOURCE,
@@ -67,45 +68,3 @@ export const sendFeedback: SendFeedback = (
     });
   });
 };
-
-/*
- * For reference, the fully built event looks something like this:
- * {
- *     "type": "feedback",
- *     "event_id": "d2132d31b39445f1938d7e21b6bf0ec4",
- *     "timestamp": 1597977777.6189718,
- *     "dist": "1.12",
- *     "platform": "javascript",
- *     "environment": "production",
- *     "release": 42,
- *     "tags": {"transaction": "/organizations/:orgId/performance/:eventSlug/"},
- *     "sdk": {"name": "name", "version": "version"},
- *     "user": {
- *         "id": "123",
- *         "username": "user",
- *         "email": "user@site.com",
- *         "ip_address": "192.168.11.12",
- *     },
- *     "request": {
- *         "url": None,
- *         "headers": {
- *             "user-Agent": "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/15.5 Safari/605.1.15"
- *         },
- *     },
- *     "contexts": {
- *         "feedback": {
- *             "message": "test message",
- *             "contact_email": "test@example.com",
- *             "type": "feedback",
- *         },
- *         "trace": {
- *             "trace_id": "4C79F60C11214EB38604F4AE0781BFB2",
- *             "span_id": "FA90FDEAD5F74052",
- *             "type": "trace",
- *         },
- *         "replay": {
- *             "replay_id": "e2d42047b1c5431c8cba85ee2a8ab25d",
- *         },
- *     },
- *   }
- */

packages/feedback/src/modal/getUser.test.ts

@@ -0,0 +1,25 @@
+import { getCurrentScope, getGlobalScope, getIsolationScope } from '@sentry/core';
+import type { User } from '@sentry/types';
+import { getUser } from './getUser';
+
+const currentUser: User = { username: 'current' };
+
+describe('getUser', () => {
+  beforeEach(() => {
+    getCurrentScope().clear();
+    getIsolationScope().clear();
+    getGlobalScope().clear();
+  });
+
+  it('should prefer the user from the current scope', () => {
+    getCurrentScope().setUser(currentUser);
+
+    expect(getUser()).toEqual(expect.objectContaining(currentUser));
+  });
+
+  it('should return the empty user if no explicit user is set', () => {
+    getCurrentScope().setUser(null);
+
+    expect(getUser()).toEqual({ email: undefined, id: undefined, ip_address: undefined, username: undefined });
+  });
+});

packages/feedback/src/modal/getUser.tsx

@@ -0,0 +1,15 @@
+import { getCurrentScope, getGlobalScope, getIsolationScope } from '@sentry/core';
+import type { User } from '@sentry/types';
+
+export function getUser(): User | undefined {
+  const currentUser = getCurrentScope().getUser();
+  if (currentUser && Object.keys(currentUser).length) {
+    return currentUser;
+  }
+  const isolationUser = getIsolationScope().getUser();
+  if (isolationUser && Object.keys(isolationUser).length) {
+    return isolationUser;
+  }
+  const globalUser = getGlobalScope().getUser();
+  return globalUser;
+}

packages/feedback/src/modal/integration.tsx

@@ -1,23 +1,11 @@
-import { getCurrentScope, getGlobalScope, getIsolationScope } from '@sentry/core';
-import type { FeedbackFormData, FeedbackModalIntegration, IntegrationFn, User } from '@sentry/types';
+import type { FeedbackFormData, FeedbackModalIntegration, IntegrationFn } from '@sentry/types';
+
 import { h, render } from 'preact';
 import * as hooks from 'preact/hooks';
 import { DOCUMENT } from '../constants';
 import { Dialog } from './components/Dialog';
 import { createDialogStyles } from './components/Dialog.css';
-
-function getUser(): User | undefined {
-  const currentUser = getCurrentScope().getUser();
-  const isolationUser = getIsolationScope().getUser();
-  const globalUser = getGlobalScope().getUser();
-  if (currentUser && Object.keys(currentUser).length) {
-    return currentUser;
-  }
-  if (isolationUser && Object.keys(isolationUser).length) {
-    return isolationUser;
-  }
-  return globalUser;
-}
+import { getUser } from './getUser';
 
 export const feedbackModalIntegration = ((): FeedbackModalIntegration => {
   return {

packages/feedback/src/util/validate.test.ts

@@ -0,0 +1,44 @@
+import { getMissingFields } from './validate';
+
+const BASE_FEEDBACK = {
+  message: '',
+  name: '',
+  email: '',
+  attachments: undefined,
+};
+const BASE_OPTIONS = {
+  emailLabel: 'Email',
+  isEmailRequired: false,
+  isNameRequired: false,
+  messageLabel: 'Message',
+  nameLabel: 'Name',
+};
+
+describe('validate', () => {
+  it('should be invalid when message is falsey', () => {
+    const result = getMissingFields(BASE_FEEDBACK, BASE_OPTIONS);
+
+    expect(result).toEqual(['Message']);
+  });
+
+  it('should be valid when message is filled out', () => {
+    const result = getMissingFields({ ...BASE_FEEDBACK, message: 'Hello world' }, BASE_OPTIONS);
+
+    expect(result).toEqual([]);
+  });
+
+  it('should be invalid when name & email are required', () => {
+    const result = getMissingFields(BASE_FEEDBACK, { ...BASE_OPTIONS, isNameRequired: true, isEmailRequired: true });
+
+    expect(result).toEqual(['Name', 'Email', 'Message']);
+  });
+
+  it('should be valid when name & email are required and not falsey', () => {
+    const result = getMissingFields(
+      { message: 'Hello world', name: 'Alice', email: 'alice@example.com', attachments: undefined },
+      { ...BASE_OPTIONS, isNameRequired: true, isEmailRequired: true },
+    );
+
+    expect(result).toEqual([]);
+  });
+});