Merge pull request #80 from edx/schen/improve_analytics

fix(analytics): Add the proper labels to analytics for gradebook

documentation/decisions/0001-update-api-usage.rst

@@ -0,0 +1,46 @@
+Usage of the bulk-update API
+============================
+
+Context
+=======
+
+The LMS Grades API exposes a set of Gradebook-related endpoints:
+https://github.com/edx/edx-platform/blob/master/lms/djangoapps/grades/api/v1/gradebook_views.py
+The ``bulk-update`` endpoint defined therein allows for the creation/modification of subsection
+grades for multiple users and sections in a single request.  This allows clients of the API to limit
+the number of network requests made and to more easily manage client-side data.  Moreover,
+the course grade updates that occur during calls to this API are synchronous - the entire update operation
+is completed before a response is given to the client.
+
+For decisions made about the implementation of this API, see:
+https://github.com/edx/edx-platform/blob/master/lms/djangoapps/grades/docs/decisions/0001-gradebook-api.rst
+
+Decision
+========
+
+The Gradebook front-end will post data about a single subsection and user in a single request
+to the ``bulk-update`` API.  That is, we currently need only the "update" aspect of this
+endpoint, and not the "bulk" aspect, for satisfying the requirements of the current UX.
+
+Status
+======
+
+Accepted (circa December 2018)
+
+Consequences
+============
+
+This is a scenario in which the implementation of the API is coupled to the
+UX that depends on the API.  Because the course grade update is synchronous, it means
+the API response can contain the updated subsection and course grade data.  Because
+a response from the API contains this data, the UI can operate in a very familiar way:
+
+- A user clicks a button to submit a request with grade update data to the update endpoint.
+- On the server, the subsection and course grades are modified.
+- In the meantime, the client-side user looks at a spinner.
+- A response is returned with updated data and the spinner goes away.
+- Updated data is displayed to the user, along with a message indicative of the update.
+
+If the update becomes asynchronous, the user experience outlined above has to change.
+Because a single call to this endpoint updates grades data for only a single user,
+the endpoint does not necessarily have to utilize an asynchronous operation at this time.

package-lock.json

@@ -10469,14 +10469,12 @@
         "balanced-match": {
           "version": "1.0.0",
           "bundled": true,
-          "dev": true,
-          "optional": true
+          "dev": true
         },
         "brace-expansion": {
           "version": "1.1.11",
           "bundled": true,
           "dev": true,
-          "optional": true,
           "requires": {
             "balanced-match": "^1.0.0",
             "concat-map": "0.0.1"
@@ -10496,8 +10494,7 @@
         "concat-map": {
           "version": "0.0.1",
           "bundled": true,
-          "dev": true,
-          "optional": true
+          "dev": true
         },
         "console-control-strings": {
           "version": "1.1.0",
@@ -10645,7 +10642,6 @@
           "version": "3.0.4",
           "bundled": true,
           "dev": true,
-          "optional": true,
           "requires": {
             "brace-expansion": "^1.1.7"
           }

src/components/Gradebook/index.jsx

@@ -176,7 +176,7 @@ export default class Gradebook extends React.Component {
     return 'Tracks';
   };
 
-  roundGrade = percent => parseFloat(percent.toFixed(DECIMAL_PRECISION));
+  roundGrade = percent => parseFloat((percent || 0).toFixed(DECIMAL_PRECISION));
 
   formatter = {
     percent: (entries, areGradesFrozen) => entries.map((entry) => {

src/components/PageButtons/index.jsx

@@ -1,10 +1,10 @@
 import React from 'react';
-import {
-  Button,
-} from '@edx/paragon';
+import { Button } from '@edx/paragon';
 
 
-export default function PageButtons({prevPage, nextPage, selectedTrack, selectedCohort, getPrevNextGrades}) {
+export default function PageButtons({
+  prevPage, nextPage, selectedTrack, selectedCohort, getPrevNextGrades, match,
+}) {
   return (
     <div
       className="d-flex justify-content-center"
@@ -15,14 +15,14 @@ export default function PageButtons({prevPage, nextPage, selectedTrack, selected
         style={{ margin: '20px' }}
         buttonType="primary"
         disabled={!prevPage}
-        onClick={() => getPrevNextGrades(prevPage, selectedCohort, selectedTrack)}
+        onClick={() => getPrevNextGrades(prevPage, selectedCohort, selectedTrack, match.params.courseId)}
       />
       <Button
         label="Next Page"
         style={{ margin: '20px' }}
         buttonType="primary"
         disabled={!nextPage}
-        onClick={() => getPrevNextGrades(nextPage, selectedCohort, selectedTrack)}
+        onClick={() => getPrevNextGrades(nextPage, selectedCohort, selectedTrack, match.params.courseId)}
       />
     </div>
   );

src/containers/GradebookPage/index.jsx

@@ -30,18 +30,17 @@ const mapStateToProps = state => (
     assignmnetTypes: state.assignmentTypes.results,
     areGradesFrozen: state.assignmentTypes.areGradesFrozen,
     showSpinner: shouldShowSpinner(state),
-    canUserViewGradebook: state.roles.canUserViewGradebook
+    canUserViewGradebook: state.roles.canUserViewGradebook,
   }
 );
 
-function shouldShowSpinner (state) {
-  if (state.roles.canUserViewGradebook === true){
+function shouldShowSpinner(state) {
+  if (state.roles.canUserViewGradebook === true) {
     return state.grades.showSpinner;
-  } else if (state.roles.canUserViewGradebook === false){
+  } else if (state.roles.canUserViewGradebook === false) {
     return false;
-  } else { // canUserViewGradebook === null
-    return true;
-  }
+  } // canUserViewGradebook === null
+  return true;
 }
 
 const mapDispatchToProps = dispatch => (
@@ -52,8 +51,8 @@ const mapDispatchToProps = dispatch => (
     searchForUser: (courseId, searchText, cohort, track) => {
       dispatch(fetchMatchingUserGrades(courseId, searchText, cohort, track, false));
     },
-    getPrevNextGrades: (endpoint, cohort, track) => {
-      dispatch(fetchPrevNextGrades(endpoint, cohort, track));
+    getPrevNextGrades: (endpoint, cohort, track, courseId) => {
+      dispatch(fetchPrevNextGrades(endpoint, cohort, track, courseId));
     },
     getCohorts: (courseId) => {
       dispatch(fetchCohorts(courseId));

src/data/actions/grades.js

@@ -32,7 +32,7 @@ const sortGrades = (columnName, direction) => {
 const startedFetchingGrades = () => ({ type: STARTED_FETCHING_GRADES });
 const finishedFetchingGrades = () => ({ type: FINISHED_FETCHING_GRADES });
 const errorFetchingGrades = () => ({ type: ERROR_FETCHING_GRADES });
-const gotGrades = (grades, cohort, track, headings, prev, next) => ({
+const gotGrades = (grades, cohort, track, headings, prev, next, courseId) => ({
   type: GOT_GRADES,
   grades,
   cohort,
@@ -40,15 +40,18 @@ const gotGrades = (grades, cohort, track, headings, prev, next) => ({
   headings,
   prev,
   next,
+  courseId,
 });
 
 const gradeUpdateRequest = () => ({ type: GRADE_UPDATE_REQUEST });
-const gradeUpdateSuccess = responseData => ({
+const gradeUpdateSuccess = (courseId, responseData) => ({
   type: GRADE_UPDATE_SUCCESS,
+  courseId,
   payload: { responseData },
 });
-const gradeUpdateFailure = error => ({
+const gradeUpdateFailure = (courseId, error) => ({
   type: GRADE_UPDATE_FAILURE,
+  courseId,
   payload: { error },
 });
 
@@ -77,6 +80,7 @@ const fetchGrades = (courseId, cohort, track, showSuccess) => (
           headingMapper(defaultAssignmentFilter)(dispatch, data.results[0]),
           data.previous,
           data.next,
+          courseId,
         ));
         dispatch(finishedFetchingGrades());
         dispatch(updateBanner(!!showSuccess));
@@ -100,6 +104,7 @@ const fetchMatchingUserGrades = (courseId, searchText, cohort, track, showSucces
           headingMapper(defaultAssignmentFilter)(dispatch, data.results[0]),
           data.previous,
           data.next,
+          courseId,
         ));
         dispatch(finishedFetchingGrades());
         dispatch(updateBanner(showSuccess));
@@ -110,7 +115,7 @@ const fetchMatchingUserGrades = (courseId, searchText, cohort, track, showSucces
   }
 );
 
-const fetchPrevNextGrades = (endpoint, cohort, track) => (
+const fetchPrevNextGrades = (endpoint, cohort, track, courseId) => (
   (dispatch) => {
     dispatch(startedFetchingGrades());
     return apiClient.get(endpoint)
@@ -123,6 +128,7 @@ const fetchPrevNextGrades = (endpoint, cohort, track) => (
           headingMapper(defaultAssignmentFilter)(dispatch, data.results[0]),
           data.previous,
           data.next,
+          courseId,
         ));
         dispatch(finishedFetchingGrades());
       })
@@ -139,11 +145,11 @@ const updateGrades = (courseId, updateData, searchText, cohort, track) => (
     return LmsApiService.updateGradebookData(courseId, updateData)
       .then(response => response.data)
       .then((data) => {
-        dispatch(gradeUpdateSuccess(data));
+        dispatch(gradeUpdateSuccess(courseId, data));
         dispatch(fetchMatchingUserGrades(courseId, searchText, cohort, track, true));
       })
       .catch((error) => {
-        dispatch(gradeUpdateFailure(error));
+        dispatch(gradeUpdateFailure(courseId, error));
       });
   }
 );

src/data/actions/grades.test.js

@@ -110,6 +110,7 @@ describe('actions', () => {
           ],
           prev: responseData.previous,
           next: responseData.next,
+          courseId,
         },
         { type: FINISHED_FETCHING_GRADES },
         { type: UPDATE_BANNER, showSuccess: false },

src/data/reducers/grades.js

@@ -35,6 +35,7 @@ const grades = (state = initialState, action) => {
         prevPage: action.prev,
         nextPage: action.next,
         showSpinner: false,
+        courseId: action.courseId,
       };
     case STARTED_FETCHING_GRADES:
       return {

src/data/reducers/grades.test.js

@@ -113,6 +113,7 @@ describe('grades reducer', () => {
       prevPage: expectedPrev,
       nextPage: expectedNext,
       showSpinner: false,
+      courseId,
     };
     expect(grades(undefined, {
       type: GOT_GRADES,
@@ -123,6 +124,7 @@ describe('grades reducer', () => {
       track: expectedTrack,
       cohort: expectedCohortId,
       showSpinner: true,
+      courseId,
     })).toEqual(expected);
   });
 

src/data/store.js

@@ -10,14 +10,18 @@ import { GOT_GRADES, GRADE_UPDATE_SUCCESS, GRADE_UPDATE_FAILURE } from './consta
 import reducers from './reducers';
 
 const loggerMiddleware = createLogger();
+const trackingCategory = 'gradebook';
 
 const eventsMap = {
   [GOT_ROLES]: trackPageView(action => ({
+    category: trackingCategory,
     page: action.courseId,
   })),
   [GOT_GRADES]: trackEvent(action => ({
     name: 'Grades displayed or paginated',
     properties: {
+      category: trackingCategory,
+      courseId: action.courseId,
       track: action.track,
       cohort: action.cohort,
       prev: action.prev,
@@ -27,12 +31,16 @@ const eventsMap = {
   [GRADE_UPDATE_SUCCESS]: trackEvent(action => ({
     name: 'Grades Updated',
     properties: {
+      category: trackingCategory,
+      courseId: action.courseId,
       updatedGrades: action.payload.responseData,
     },
   })),
   [GRADE_UPDATE_FAILURE]: trackEvent(action => ({
     name: 'Grades Fail to Update',
     properties: {
+      category: trackingCategory,
+      courseId: action.courseId,
       error: action.payload.error,
     },
   })),