fix: do not show skipped sentences again in Swipe Review Tool (fixes #…

…421)
common-voice · Jun 27, 2021 · c5154b1 · c5154b1
1 parent 28f87f3
commit c5154b1
Show file tree

Hide file tree

Showing 14 changed files with 335 additions and 47 deletions.
diff --git a/web/package.json b/web/package.json
@@ -83,8 +83,8 @@
     ],
     "coverageThreshold": {
       "global": {
-        "branches": 90,
-        "functions": 82,
+        "branches": 89,
+        "functions": 81,
         "lines": 93,
         "statements": 91
       }

diff --git a/web/src/actions/sentences.ts b/web/src/actions/sentences.ts
@@ -21,6 +21,8 @@ export const ACTION_GOT_SENTENCES = 'GOT_SENTENCES';
 export const ACTION_REVIEWED_SENTENCES = 'REVIEWED_SENTENCES';
 export const ACTION_REVIEW_SENTENCES_FAILURE = 'REVIEW_SENTENCES_FAILURE';
 export const ACTION_REVIEW_RESET_MESSAGE = 'REVIEW_RESET_MESSAGE';
+export const ACTION_REVIEW_SAVE_SKIPPED_SENTENCES = 'ACTION_REVIEW_SAVE_SKIPPED_SENTENCES';
+export const ACTION_REVIEW_RESET_SKIPPED_SENTENCES = 'ACTION_REVIEW_RESET_SKIPPED_SENTENCES';
 
 type SentenceSubmission = {
   sentences: {
@@ -116,6 +118,19 @@ export function reviewSentences(data: ReviewedSentences, language: string): Thun
   };
 }
 
+export function saveSkippedSentences(sentenceIds: number[]) {
+  return {
+    type: ACTION_REVIEW_SAVE_SKIPPED_SENTENCES,
+    sentenceIds,
+  };
+}
+
+export function resetSkippedSentences() {
+  return {
+    type: ACTION_REVIEW_RESET_SKIPPED_SENTENCES,
+  };
+}
+
 function sendSubmitSentences() {
   return {
     type: ACTION_SUBMIT_SENTENCES_REQUEST,

diff --git a/web/src/components/review-form.test.tsx b/web/src/components/review-form.test.tsx
@@ -29,6 +29,7 @@ const sentences = [{
 
 const setStateMock = jest.fn();
 const onReviewed = jest.fn();
+const onSkip = jest.fn();
 
 beforeEach(() => {
   jest.clearAllMocks();
@@ -47,35 +48,65 @@ beforeEach(() => {
 
 describe('Normal Review Tool', () => {
   test('should render normal review tool', () => {
-    render(<ReviewForm sentences={sentences} onReviewed={onReviewed} language="en"/>);
+    render(
+      <ReviewForm
+        sentences={sentences}
+        onReviewed={onReviewed}
+        onSkip={onSkip}
+        language="en"
+      />
+    );
     expect(screen.queryByText(/Swipe right to approve the sentence/)).toBeNull();
   });
 
   test('should render message', () => {
     const message = 'Hi there!';
-    render(<ReviewForm sentences={sentences} message={message} onReviewed={onReviewed} language="en"/>);
+    render(
+      <ReviewForm
+        sentences={sentences}
+        message={message}
+        onReviewed={onReviewed}
+        onSkip={onSkip}
+        language="en"
+      />
+    );
     expect(screen.getByText(message)).toBeTruthy();
   });
 
-  test('should render message with no sentences', () => {
-    const noSentences: SentenceRecord[] = [];
-    render(<ReviewForm sentences={noSentences} onReviewed={onReviewed} language="en"/>);
-    expect(screen.getByText('nothing to review')).toBeTruthy();
-  });
-
   test('should render sentences with source', () => {
-    render(<ReviewForm sentences={sentences} onReviewed={onReviewed} language="en"/>);
+    render(
+      <ReviewForm
+        sentences={sentences}
+        onReviewed={onReviewed}
+        onSkip={onSkip}
+        language="en"
+      />
+    );
     expect(screen.queryAllByText(/Hi there/).length).toBe(5);
     expect(screen.queryAllByText(/Me/).length).toBe(5);
   });
 
   test('should render submit button', () => {
-    render(<ReviewForm sentences={sentences} onReviewed={onReviewed} language="en"/>);
+    render(
+      <ReviewForm
+        sentences={sentences}
+        onReviewed={onReviewed}
+        onSkip={onSkip}
+        language="en"
+      />
+    );
     expect(screen.getByText('Finish Review')).toBeTruthy();
   });
 
   test('should paginate', async () => {
-    render(<ReviewForm sentences={sentences} onReviewed={onReviewed} language="en"/>);
+    render(
+      <ReviewForm
+        sentences={sentences}
+        onReviewed={onReviewed}
+        onSkip={onSkip}
+        language="en"
+      />
+    );
 
     expect(screen.getByText('Hi there two')).toBeTruthy();
     await act(async () => await userEvent.click(screen.getByText('>')));
@@ -84,7 +115,14 @@ describe('Normal Review Tool', () => {
   });
 
   test('should call onReviewed with correct status', async () => {
-    render(<ReviewForm sentences={sentences} onReviewed={onReviewed} language="en"/>);
+    render(
+      <ReviewForm
+        sentences={sentences}
+        onReviewed={onReviewed}
+        onSkip={onSkip}
+        language="en"
+      />
+    );
 
     await userEvent.click(screen.getAllByText('👍')[0]);
     await userEvent.click(screen.getAllByText('👍')[1]);
@@ -126,24 +164,43 @@ describe('Normal Review Tool', () => {
 // test all the logic
 describe('Swipe Review Tool', () => {
   test('should render swipe review tool', () => {
-    render(<ReviewForm sentences={sentences} useSwipeReview={true} onReviewed={onReviewed} language="en"/>);
+    render(
+      <ReviewForm
+        sentences={sentences}
+        useSwipeReview={true}
+        onReviewed={onReviewed}
+        onSkip={onSkip}
+        language="en"
+      />
+    );
     expect(screen.getByText(/Swipe right to approve the sentence/)).toBeTruthy();
   });
 
   test('should render message', () => {
     const message = 'Hi there!';
-    render(<ReviewForm sentences={sentences} message={message} useSwipeReview={true} onReviewed={onReviewed} language="en"/>);
+    render(
+      <ReviewForm
+        sentences={sentences}
+        message={message}
+        useSwipeReview={true}
+        onSkip={onSkip}
+        onReviewed={onReviewed}
+        language="en"
+      />
+    );
     expect(screen.getByText(message)).toBeTruthy();
   });
 
-  test('should render message with no sentences', () => {
-    const noSentences: SentenceRecord[] = [];
-    render(<ReviewForm sentences={noSentences} useSwipeReview={true} onReviewed={onReviewed} language="en"/>);
-    expect(screen.getByText('nothing to review')).toBeTruthy();
-  });
-
   test('should render submit button', () => {
-    render(<ReviewForm sentences={sentences} useSwipeReview={true} onReviewed={onReviewed} language="en"/>);
+    render(
+      <ReviewForm
+        sentences={sentences}
+        useSwipeReview={true}
+        onReviewed={onReviewed}
+        onSkip={onSkip}
+        language="en"
+      />
+    );
     expect(screen.getByText('Finish Review')).toBeTruthy();
   });
 
@@ -176,7 +233,15 @@ describe('Swipe Review Tool', () => {
     };
 
     test('should skip sentence on swipe review tool skip button', async () => {
-      render(<ReviewForm sentences={[sentences[0]]} onReviewed={onReviewed} useSwipeReview={true} language="en"/>);
+      render(
+        <ReviewForm
+          sentences={[sentences[0]]}
+          onReviewed={onReviewed}
+          onSkip={onSkip}
+          useSwipeReview={true}
+          language="en"
+        />
+      );
 
       await userEvent.click(screen.getByText('Skip'));
 
@@ -192,7 +257,15 @@ describe('Swipe Review Tool', () => {
     });
 
     test('should approve sentence on approve button', async () => {
-      render(<ReviewForm sentences={[sentences[0]]} onReviewed={onReviewed} useSwipeReview={true} language="en"/>);
+      render(
+        <ReviewForm
+          sentences={[sentences[0]]}
+          onReviewed={onReviewed}
+          onSkip={onSkip}
+          useSwipeReview={true}
+          language="en"
+        />
+      );
 
       await userEvent.click(screen.getByText('Approve'));
 
@@ -208,7 +281,15 @@ describe('Swipe Review Tool', () => {
     });
 
     test('should reject sentence on reject button', async () => {
-      render(<ReviewForm sentences={[sentences[0]]} onReviewed={onReviewed} useSwipeReview={true} language="en"/>);
+      render(
+        <ReviewForm
+          sentences={[sentences[0]]}
+          onReviewed={onReviewed}
+          onSkip={onSkip}
+          useSwipeReview={true}
+          language="en"
+        />
+      );
 
       await userEvent.click(screen.getByText('Reject'));
 
@@ -224,7 +305,15 @@ describe('Swipe Review Tool', () => {
     });
 
     test('should set state of sentence on multiple button reviews', async () => {
-      render(<ReviewForm sentences={sentences} onReviewed={onReviewed} useSwipeReview={true} language="en"/>);
+      render(
+        <ReviewForm
+          sentences={sentences}
+          onReviewed={onReviewed}
+          onSkip={onSkip}
+          useSwipeReview={true}
+          language="en"
+        />
+      );
 
       await userEvent.click(screen.getByText('Reject'));
       await userEvent.click(screen.getByText('Approve'));
@@ -238,7 +327,15 @@ describe('Swipe Review Tool', () => {
     });
 
     test('should set state of sentence on multiple keyboard reviews', async () => {
-      render(<ReviewForm sentences={sentences} onReviewed={onReviewed} useSwipeReview={true} language="en"/>);
+      render(
+        <ReviewForm
+          sentences={sentences}
+          onReviewed={onReviewed}
+          onSkip={onSkip}
+          useSwipeReview={true}
+          language="en"
+        />
+      );
 
       await fireEvent.keyDown(document, { key: 'n' });
       await fireEvent.keyDown(document, { key: 'y' });

diff --git a/web/src/components/review-form.tsx b/web/src/components/review-form.tsx
@@ -14,6 +14,7 @@ const PAGE_SIZE = 5;
 type Props = {
   sentences: SentenceRecord[]
   onReviewed: (categorizedSentences: ReviewedState) => void
+  onSkip: (sentenceId: number) => void
   message?: string
   language?: string
   useSwipeReview?: boolean
@@ -23,7 +24,14 @@ type ReviewApproval = {
   [key: number]: boolean | undefined
 }
 
-export default function ReviewForm({ message, useSwipeReview, sentences, onReviewed, language }: Props) {
+export default function ReviewForm({
+  message,
+  useSwipeReview,
+  language,
+  sentences,
+  onReviewed,
+  onSkip,
+}: Props) {
   const [page, setPage] = useState(0);
   const [reviewedSentencesCount, setReviewedCount] = useState(0);
   const [reviewApproval, setReviewApproval] = useState<ReviewApproval>({});
@@ -52,14 +60,22 @@ export default function ReviewForm({ message, useSwipeReview, sentences, onRevie
     setReviewedCount((previousNumber) => previousNumber + 1);
   };
 
+  const skip = (index: number) => {
+    const sentence = sentences[index];
+    if (typeof sentence.id !== 'undefined') {
+      onSkip(sentence.id);
+    }
+  };
+
   if (!Array.isArray(sentences) || sentences.length < 1) {
-    return (<h2>nothing to review</h2>);
+    return null;
   }
 
   if (useSwipeReview) {
     return (
       <SwipeReviewForm
         onReviewSentence={reviewSentence}
+        onSkip={skip}
         onSubmit={onSubmit}
         sentences={sentences}
         page={page}

diff --git a/web/src/components/settings.test.tsx b/web/src/components/settings.test.tsx
@@ -5,10 +5,12 @@ import userEvent from '@testing-library/user-event';
 import Settings from './settings';
 
 const swipeToggleMock = jest.fn();
+const resetSkippedMock = jest.fn();
 
 beforeEach(() => {
   jest.resetAllMocks();
   swipeToggleMock.mockReset();
+  resetSkippedMock.mockReset();
 });
 
 test('should render title', () => {
@@ -38,9 +40,40 @@ test('should render button to use normal review tool', () => {
 });
 
 test('should call onToggleSwipeReview when review tool button is clicked', async () => {
-
   render(<Settings onToggleSwipeReview={swipeToggleMock}/>);
   await userEvent.click(screen.getByText('Use Swiping Review Tool'));
   expect(swipeToggleMock).toHaveBeenCalled();
 });
 
+test('should render button to reset skipped sentences', () => {
+  render(
+    <Settings
+      useSwipeReview={true}
+      onToggleSwipeReview={swipeToggleMock}
+      onResetSkipped={resetSkippedMock}
+    />
+  );
+  expect(screen.getByText('Show all skipped sentences again')).toBeTruthy();
+});
+
+test('should not render button to reset skipped sentences if no callback passed', () => {
+  render(
+    <Settings
+      useSwipeReview={true}
+      onToggleSwipeReview={swipeToggleMock}
+    />
+  );
+  expect(screen.queryByText('Show all skipped sentences again')).toBeNull();
+});
+
+test('should call onResetSkipped when reset button is clicked', async () => {
+  render(
+    <Settings
+      onToggleSwipeReview={swipeToggleMock}
+      onResetSkipped={resetSkippedMock}
+    />
+  );
+  await userEvent.click(screen.getByText('Show all skipped sentences again'));
+  expect(resetSkippedMock).toHaveBeenCalled();
+});
+