Hide SyncGradesButton for auto-grading non-gradable assignments (#6869

) * Hide SyncGradesButton for auto-grading non-gradable assignments * Mock history-related hooks in dashboard hooks-test
hypothesis · Nov 25, 2024 · 1f28c64 · 1f28c64
1 parent ad56a63
commit 1f28c64
Show file tree

Hide file tree

Showing 4 changed files with 73 additions and 63 deletions.
diff --git a/lms/static/scripts/frontend_apps/api-types.ts b/lms/static/scripts/frontend_apps/api-types.ts
@@ -187,6 +187,7 @@ export type Assignment = {
   title: string;
   /** Date in which the assignment was created, in ISO format */
   created: ISODateTime;
+  is_gradable: boolean;
 };
 
 export type Student = {

diff --git a/lms/static/scripts/frontend_apps/components/dashboard/AssignmentActivity.tsx b/lms/static/scripts/frontend_apps/components/dashboard/AssignmentActivity.tsx
@@ -95,6 +95,7 @@ export default function AssignmentActivity() {
     replaceURLParams(routes.assignment, { assignment_id: assignmentId }),
   );
   const isAutoGradingAssignment = !!assignment.data?.auto_grading_config;
+  const isGradable = !!assignment.data?.is_gradable;
   const segments = useMemo((): DashboardActivityFiltersProps['segments'] => {
     const { data } = assignment;
     if (
@@ -161,12 +162,17 @@ export default function AssignmentActivity() {
 
   const syncURL = useMemo(
     () =>
-      isAutoGradingAssignment
+      isAutoGradingAssignment && isGradable
         ? replaceURLParams(routes.assignment_grades_sync, {
             assignment_id: assignmentId,
           })
         : null,
-    [assignmentId, isAutoGradingAssignment, routes.assignment_grades_sync],
+    [
+      assignmentId,
+      isAutoGradingAssignment,
+      isGradable,
+      routes.assignment_grades_sync,
+    ],
   );
   const [lastSyncParams, setLastSyncParams] = useState<QueryParams>({});
   const lastSync = usePolledAPIFetch<GradingSync>({
@@ -345,7 +351,7 @@ export default function AssignmentActivity() {
             }
           />
         )}
-        {isAutoGradingAssignment && auto_grading_sync_enabled && (
+        {isAutoGradingAssignment && auto_grading_sync_enabled && isGradable && (
           <SyncGradesButton
             studentsToSync={studentsToSync}
             lastSync={lastSync}

diff --git a/lms/static/scripts/frontend_apps/components/dashboard/test/AssignmentActivity-test.js b/lms/static/scripts/frontend_apps/components/dashboard/test/AssignmentActivity-test.js
@@ -56,6 +56,7 @@ describe('AssignmentActivity', () => {
       id: 12,
       title: 'The course',
     },
+    is_gradable: true,
   };
 
   let fakeUseAPIFetch;
@@ -497,34 +498,58 @@ describe('AssignmentActivity', () => {
       {
         syncEnabled: true,
         isAutoGradingAssignment: true,
-        shouldShowButton: true,
+        isGradable: true,
       },
       {
         syncEnabled: false,
         isAutoGradingAssignment: true,
-        shouldShowButton: false,
+        isGradable: true,
       },
       {
         syncEnabled: true,
         isAutoGradingAssignment: false,
-        shouldShowButton: false,
+        isGradable: true,
       },
       {
         syncEnabled: false,
         isAutoGradingAssignment: false,
-        shouldShowButton: false,
+        isGradable: true,
       },
-    ].forEach(({ isAutoGradingAssignment, syncEnabled, shouldShowButton }) => {
-      it('shows sync button when both sync and auto-grading are enabled', () => {
+      {
+        syncEnabled: false,
+        isAutoGradingAssignment: true,
+        isGradable: false,
+      },
+      {
+        syncEnabled: false,
+        isAutoGradingAssignment: false,
+        isGradable: true,
+      },
+      {
+        syncEnabled: true,
+        isAutoGradingAssignment: false,
+        isGradable: false,
+      },
+      {
+        syncEnabled: false,
+        isAutoGradingAssignment: false,
+        isGradable: false,
+      },
+    ].forEach(({ isAutoGradingAssignment, syncEnabled, isGradable }) => {
+      it('shows sync button when sync and auto-grading are enabled, and the assignment is gradable', () => {
         setUpFakeUseAPIFetch({
           ...activeAssignment,
+          is_gradable: isGradable,
           auto_grading_config: isAutoGradingAssignment ? {} : null,
         });
         fakeConfig.dashboard.auto_grading_sync_enabled = syncEnabled;
 
         const wrapper = createComponent();
 
-        assert.equal(wrapper.exists('SyncGradesButton'), shouldShowButton);
+        assert.equal(
+          wrapper.exists('SyncGradesButton'),
+          isAutoGradingAssignment && syncEnabled && isGradable,
+        );
       });
     });
 

diff --git a/lms/static/scripts/frontend_apps/utils/dashboard/test/hooks-test.js b/lms/static/scripts/frontend_apps/utils/dashboard/test/hooks-test.js
@@ -1,8 +1,12 @@
 import { mount } from 'enzyme';
 
-import { useDashboardFilters } from '../hooks';
+import { useDashboardFilters, $imports } from '../hooks';
 
 describe('useDashboardFilters', () => {
+  let fakeUseLocation;
+  let fakeUseSearch;
+  let fakeNavigate;
+
   function FakeComponent() {
     const { filters, updateFilters, urlWithFilters } = useDashboardFilters();
 
@@ -49,13 +53,17 @@ describe('useDashboardFilters', () => {
     );
   }
 
-  function setCurrentURL(url) {
-    history.replaceState(null, '', url);
-  }
-
   beforeEach(() => {
-    // Reset query string
-    setCurrentURL('?');
+    fakeUseSearch = sinon.stub().returns('');
+    fakeNavigate = sinon.stub();
+    fakeUseLocation = sinon.stub().returns(['', fakeNavigate]);
+
+    $imports.$mock({
+      'wouter-preact': {
+        useSearch: fakeUseSearch,
+        useLocation: fakeUseLocation,
+      },
+    });
   });
 
   function createComponent() {
@@ -127,7 +135,7 @@ describe('useDashboardFilters', () => {
       expectedSegments,
     }) => {
       it('reads params from the query', () => {
-        setCurrentURL(initialQueryString);
+        fakeUseSearch.returns(initialQueryString);
 
         const wrapper = createComponent();
 
@@ -142,91 +150,61 @@ describe('useDashboardFilters', () => {
   [
     {
       buttonId: 'update-courses',
-      getResult: getCurrentCourses,
-      expectedResult: '111,222,333',
       expectedQueryString: '?course_id=111&course_id=222&course_id=333',
     },
     {
       buttonId: 'update-assignments',
-      getResult: getCurrentAssignments,
-      expectedResult: '123,456,789',
       expectedQueryString:
         '?assignment_id=123&assignment_id=456&assignment_id=789',
     },
     {
       buttonId: 'update-students',
-      getResult: getCurrentStudents,
-      expectedResult: 'abc,def',
       expectedQueryString: '?student_id=abc&student_id=def',
     },
     {
       buttonId: 'update-segments',
-      getResult: getCurrentSegments,
-      expectedResult: 'foo,bar',
       expectedQueryString: '?segment_id=foo&segment_id=bar',
     },
-  ].forEach(({ buttonId, getResult, expectedResult, expectedQueryString }) => {
+  ].forEach(({ buttonId, expectedQueryString }) => {
     it('persists updated values in query string', () => {
       const wrapper = createComponent();
 
       wrapper.find(`[data-testid="${buttonId}"]`).simulate('click');
 
-      assert.equal(getResult(wrapper), expectedResult);
-      assert.equal(location.search, expectedQueryString);
+      assert.calledWith(fakeNavigate, expectedQueryString);
     });
   });
 
   it('preserves unknown query params', () => {
-    setCurrentURL('?foo=bar&something=else');
+    fakeUseSearch.returns('?foo=bar&something=else');
 
     const wrapper = createComponent();
     wrapper.find('[data-testid="update-courses"]').simulate('click');
 
-    assert.equal(
+    assert.calledWith(
+      fakeNavigate,
       '?foo=bar&something=else&course_id=111&course_id=222&course_id=333',
-      location.search,
     );
   });
 
   it('preserves path', () => {
-    setCurrentURL('/foo/bar');
+    fakeUseLocation.returns(['/foo/bar', fakeNavigate]);
 
     const wrapper = createComponent();
     wrapper.find('[data-testid="update-courses"]').simulate('click');
 
-    assert.equal('?course_id=111&course_id=222&course_id=333', location.search);
-    assert.equal('/foo/bar', location.pathname);
+    assert.calledWith(
+      fakeNavigate,
+      '/foo/bar?course_id=111&course_id=222&course_id=333',
+    );
   });
 
-  [
-    {
-      buttonId: 'update-courses',
-      expectedURL: '/hello/world?course_id=111&course_id=222&course_id=333',
-    },
-    {
-      buttonId: 'update-assignments',
-      expectedURL:
-        '/hello/world?assignment_id=123&assignment_id=456&assignment_id=789',
-    },
-    {
-      buttonId: 'update-students',
-      expectedURL: '/hello/world?student_id=abc&student_id=def',
-    },
-    {
-      buttonId: 'update-segments',
-      expectedURL: '/hello/world?segment_id=foo&segment_id=bar',
-    },
-  ].forEach(({ buttonId, expectedURL }) => {
-    it('builds URLs with filters', () => {
-      // Current URL should be ignored
-      setCurrentURL('/foo/bar');
+  it('ignores current path when one is provided', () => {
+    // Current URL should be ignored
+    fakeUseLocation.returns(['/foo/bar', fakeNavigate]);
 
-      const wrapper = createComponent();
-
-      assert.equal(getURLWithFilters(wrapper), '/hello/world');
+    const wrapper = createComponent();
 
-      wrapper.find(`[data-testid="${buttonId}"]`).simulate('click');
-      assert.equal(getURLWithFilters(wrapper), expectedURL);
-    });
+    assert.equal(getURLWithFilters(wrapper), '/hello/world');
   });
 });