Merge pull request #34 from Li357/3.0.1-b3-fixes

Make error notification and reporting consistent across screens, refactor code

packages/app/App.tsx

@@ -25,10 +25,8 @@ import { getScheduleTypeOnDate, isScheduleEmpty } from './src/utils/query-schedu
 import Settings from './src/screens/Settings';
 import AddSchedule from './src/screens/AddSchedule';
 import registerNotificationScheduler, { scheduleNotifications } from './src/utils/notifications';
-import { reportScheduleCaution, notify } from './src/utils/utils';
+import { reportScheduleCaution, reportError } from './src/utils/utils';
 import client from './src/utils/bugsnag';
-import { LoginError } from './src/utils/error';
-import { LOGIN_CREDENTIALS_CHANGED_MSG } from './src/constants/fetch';
 
 interface AppComponentState {
   rehydrated: boolean;
@@ -77,34 +75,26 @@ export default class App extends Component<{}, AppComponentState> {
     } = store.getState();
     const now = new Date();
 
-    try {
-      if (semesterOneStart === null || semesterTwoStart === null || semesterTwoEnd === null) {
-        return;
-      }
-      if (isAfter(now, semesterTwoEnd)) {
-        client.leaveBreadcrumb('Refreshing dates after end of year');
+    if (semesterOneStart === null || semesterTwoStart === null || semesterTwoEnd === null) {
+      return;
+    }
+    if (isAfter(now, semesterTwoEnd)) {
+      client.leaveBreadcrumb('Refreshing dates after end of year');
 
-        await store.dispatch(fetchDates(now.getFullYear()));
-        store.dispatch(setRefreshed([false, false]));
-        return PushNotification.cancelAllLocalNotifications();
-      }
+      await store.dispatch(fetchDates(now.getFullYear()));
+      store.dispatch(setRefreshed([false, false]));
+      return PushNotification.cancelAllLocalNotifications();
+    }
 
-      const shouldRefresh = (isAfter(now, semesterTwoStart) && !refreshedSemesterTwo)
-        || (isAfter(now, semesterOneStart) && !refreshedSemesterOne);
-      if (isScheduleEmpty(schedule) || shouldRefresh) {
-        client.leaveBreadcrumb('Refreshing semesters one/two');
+    const shouldRefresh = (isAfter(now, semesterTwoStart) && !refreshedSemesterTwo)
+      || (isAfter(now, semesterOneStart) && !refreshedSemesterOne);
+    if (isScheduleEmpty(schedule) || shouldRefresh) {
+      client.leaveBreadcrumb('Refreshing semesters one/two');
 
-        await store.dispatch(fetchUserInfo(username, password));
-        await scheduleNotifications();
-      }
-      await registerNotificationScheduler();
-    } catch (error) {
-      if (error instanceof LoginError) {
-        return notify('Error', LOGIN_CREDENTIALS_CHANGED_MSG);
-      }
-      // rethrow to handle in handleRehydrate
-      throw error;
+      await store.dispatch(fetchUserInfo(username, password));
+      await scheduleNotifications();
     }
+    await registerNotificationScheduler();
   }
 
   private silentlyUpdateData = async () => {
@@ -188,8 +178,7 @@ export default class App extends Component<{}, AppComponentState> {
       }
       this.setState({ rehydrated: true });
     } catch (error) {
-      notify('Error', 'Something went wrong, please try restarting the app.');
-      client.notify(error);
+      reportError(error);
     }
   }
 

packages/app/__tests__/info/process-test.ts

@@ -1,18 +1,20 @@
-import fetch from 'react-native-fetch-polyfill';
 
 import {
   getLoginURL, getLoginError, getSchoolPictureFromHTML, processName,
-  getUserScheduleFromHTML, getUserInfoFromHTML, getTeacherSchedules, getTeacherSearchURL, getTeacherURL,
+  getUserScheduleFromHTML, getUserInfoFromHTML,
+  getTeacherSchedules, getTeacherSearchURL, getTeacherURL, fetchTeachersFromQuery,
 } from '../../src/utils/process-info';
-import { LOGIN_URL, SCHOOL_PICTURE_BLANK_SYMBOL } from '../../src/constants/fetch';
+import { fetch } from '../../src/utils/utils';
+import { LOGIN_URL, SCHOOL_PICTURE_BLANK_SYMBOL, TEACHER_FETCH_LIMIT } from '../../src/constants/fetch';
 import { processSchedule } from '../../src/utils/process-schedule';
 import { getStudent$, getError$, getNew$, getTeacher$, fetchMock, open, TEST_HTML_DIR } from '../test-utils/fetch';
 import { TeacherSchedule } from '../../src/types/schedule';
 
 fetchMock.config.fetch = fetch;
 fetchMock
   .post(getTeacherURL(1, 'John', '12345'), open(`${TEST_HTML_DIR}/teacher.html`))
-  .post(getTeacherURL(2, 'John', '12345'), open(`${TEST_HTML_DIR}/teacher.html`));
+  .post(getTeacherURL(2, 'John', '12345'), open(`${TEST_HTML_DIR}/teacher.html`))
+  .post(getTeacherSearchURL('test', TEACHER_FETCH_LIMIT, 'John', '12345'), []);
 
 describe('processing user info', () => {
   describe('getLoginURL', () => {
@@ -129,6 +131,12 @@ describe('processing user info', () => {
     });
   });
 
+  describe('fetchTeachersFromQuery', () => {
+    it('should launch request to fetch teachers list', async () => {
+      expect(await fetchTeachersFromQuery('test', 'John', '12345')).toEqual([]);
+    });
+  });
+
   describe('getTeacherSchedules', () => {
     it('should get specified teacher schedules', async () => {
       const teacherOneURL = getTeacherURL(1, 'John', '12345');

packages/app/__tests__/misc/utils-test.ts

@@ -1,5 +1,11 @@
-import { insert, sortByProps, getWithFallback, splice, sum, last, reportError } from '../../src/utils/utils';
+import { fetch, insert, sortByProps, getWithFallback, splice, sum, last, reportError } from '../../src/utils/utils';
 import client from '../../src/utils/bugsnag';
+import { LoginError, NetworkError } from '../../src/utils/error';
+import { fetchMock } from '../test-utils/fetch';
+
+fetchMock.config.fetch = fetch;
+fetchMock
+  .get('/', new Promise((res) => setTimeout(res, 1000)));
 
 describe('array utils', () => {
   describe('splice', () => {
@@ -148,11 +154,27 @@ describe('array utils', () => {
     });
   });
 
+  describe('custom fetch', () => {
+    it('should throw NetworkError instead of TypeError', async () => {
+      await expect(fetch('/', { timeout: 500 })).rejects.toThrowError(NetworkError);
+    });
+  });
+
   describe('reportError', () => {
-    it('should alert and notify bugsnag', () => {
+    const bugsnagNotify = client.notify as jest.Mock<void, [Error]>;
+
+    it('should not report login and network errors to bugsnag', () => {
+      reportError(new LoginError());
+      expect(bugsnagNotify).not.toBeCalled();
+
+      reportError(new NetworkError());
+      expect(bugsnagNotify).not.toBeCalled();
+    });
+
+    it('should notify bugsnag otherwise', () => {
       const error = new Error('Test Error');
       reportError(error);
-      expect((client.notify as jest.Mock<void, [Error]>).mock.calls[0]).toEqual([error]);
+      expect(bugsnagNotify.mock.calls[0]).toEqual([error]);
     });
   });
 });

packages/app/package.json

@@ -26,6 +26,7 @@
     "@react-native-community/push-notification-ios": "^1.0.2",
     "bugsnag-react-native": "2.22.2",
     "date-fns": "^2.0.0-alpha.1",
+    "lodash.debounce": "^4.0.8",
     "react": "16.8.6",
     "react-native": "0.60.4",
     "react-native-background-fetch": "^2.6.1",
@@ -60,6 +61,7 @@
     "@types/cheerio": "^0.22.11",
     "@types/fetch-mock": "^7.3.0",
     "@types/jest": "^24.0.13",
+    "@types/lodash.debounce": "^4.0.6",
     "@types/node": "^12.0.2",
     "@types/react": "^16.8.18",
     "@types/react-native": "^0.57.59",

packages/app/src/actions/async.ts

@@ -1,5 +1,4 @@
 import { ThunkAction } from 'redux-thunk';
-import fetch from 'react-native-fetch-polyfill';
 import { isAfter } from 'date-fns';
 
 import {
@@ -9,6 +8,7 @@ import {
   SetRefreshedAction,
   DatesState,
 } from '../types/store';
+import { fetch } from '../utils/utils';
 import {
   getLoginURL, parseHTMLFromURL, getUserScheduleFromHTML, getUserInfoFromHTML, getLoginError, getSchoolPictureFromHTML,
   getTeacherSchedules,
@@ -39,6 +39,7 @@ export function fetchUserInfo(username: string, password: string) {
     // if we are not doing a manual refresh, clear the current user
     const loginURL = getLoginURL(username, password);
     if (username !== user.username) {
+      // In the future, refactor so all direct fetch calls are abstracted away
       await fetch(loginURL, { method: 'POST', timeout: FETCH_TIMEOUT });
     }
 

packages/app/src/constants/fetch.ts

@@ -5,11 +5,16 @@ export const PACKAGE_NAME = 'com.li357.whs';
 export const IOS_MAX_NOTIFICATIONS = 64;
 export const MAX_NOTIFICATION_SETUP_TIMEOUT = 25000;
 
-export const NETWORK_REQUEST_FAILED = 'Network request failed';
+export const SUCCESS = 'Success';
+export const CAUTION = 'Caution';
+export const ERROR = 'Error';
+export const REFRESHED_MSG = 'Your information has been refreshed.';
 export const NETWORK_REQUEST_FAILED_MSG = `An error occurred while fetching data. \
 Please check your internet connection.`;
-export const LOGIN_CREDENTIALS_CHANGED_MSG = 'Your login credentials have changed. Please try logging in again.';
-export const UNKNOWN_ERROR_MSG = 'Unknown error occurred and has been reported.';
+export const LOGIN_CREDENTIALS_CHANGED_MSG = 'Your login credentials may have changed. Please try logging in again.';
+export const NO_SPACE_MSG = `There is not enough space on your phone to save your login, schedule, and other critical \
+information. Please clear up some space and retry logging in.`;
+export const UNKNOWN_ERROR_MSG = 'Something went wrong. Please try restarting the app.';
 
 export const SCHOOL_WEBSITE = 'https://westside-web.azurewebsites.net';
 export const LOGIN_URL = `${SCHOOL_WEBSITE}/account/login`;

packages/app/src/screens/AddSchedule.tsx

@@ -1,14 +1,12 @@
 import React, { useState, useEffect } from 'react';
 import { ScrollView } from 'react-native';
 import { useSelector, useDispatch } from 'react-redux';
-import fetch from 'react-native-fetch-polyfill';
 import styled from 'styled-components/native';
 import Icon from 'react-native-vector-icons/MaterialIcons';
 
 import authorizedRoute from '../components/common/authorizedRoute';
 import Input from '../components/common/Input';
 import Subtext from '../components/common/Subtext';
-import { FETCH_TIMEOUT, TEACHER_FETCH_LIMIT } from '../constants/fetch';
 import { RawTeacherData } from '../types/schedule';
 import { reportError, notify } from '../utils/utils';
 import TeacherItem from '../components/add-schedule/TeacherItem';
@@ -19,8 +17,11 @@ import {
 } from '../constants/style';
 import { AppState } from '../types/store';
 import { addTeacherSchedule, setTeacherSchedules } from '../actions/creators';
-import { getUserScheduleFromHTML, parseHTMLFromURL, getTeacherSearchURL, getTeacherURL } from '../utils/process-info';
+import {
+  getUserScheduleFromHTML, parseHTMLFromURL, getTeacherURL, fetchTeachersFromQuery,
+} from '../utils/process-info';
 import client from '../utils/bugsnag';
+import { SUCCESS } from '../constants/fetch';
 
 const ListContainer = styled.View`
   border-radius: ${FORM_BORDER_RADIUS};
@@ -58,12 +59,7 @@ export default authorizedRoute('Add Schedule', function AddSchedule() {
   const fetchTeachers = async (teacherQuery: string) => {
     controller.abort();
     try {
-      const response = await fetch(getTeacherSearchURL(teacherQuery, TEACHER_FETCH_LIMIT, username, password), {
-        method: 'POST',
-        timeout: FETCH_TIMEOUT,
-        signal,
-      });
-      const { teachers: json } = await response.json();
+      const { teachers: json } = await fetchTeachersFromQuery(teacherQuery, username, password, signal);
       const teachersAvailable = json.filter(({ firstName, lastName, email }: RawTeacherData) => {
         const alreadyAdded = teacherSchedules.some((teacherObj) => teacherObj.name === `${firstName} ${lastName}`);
         return email !== null && !alreadyAdded;
@@ -87,7 +83,7 @@ export default authorizedRoute('Add Schedule', function AddSchedule() {
       const $ = await parseHTMLFromURL(url, { signal, method: 'POST' });
       const schedule = await getUserScheduleFromHTML($);
       dispatch(addTeacherSchedule({ name, url, schedule }));
-      notify('Success', `${name}'s schedule added!`);
+      notify(SUCCESS, `${name}'s schedule added!`);
       setTeachers([]);
       setQuery('');
     } catch (error) {
@@ -102,7 +98,7 @@ export default authorizedRoute('Add Schedule', function AddSchedule() {
 
     const removed = teacherSchedules.filter((teacher) => teacher.name !== name);
     dispatch(setTeacherSchedules(removed));
-    notify('Success', `${name}'s schedule removed.`);
+    notify(SUCCESS, `${name}'s schedule removed.`);
     setRemovingTeacher(false);
   };
 

packages/app/src/screens/Login.tsx

@@ -65,11 +65,12 @@ export default memo(function Login(props: NavigationScreenProps) {
       dispatch(setDaySchedule(getScheduleTypeOnDate(now, updatedDates)));
       props.navigation.navigate('Dashboard');
     } catch (error) {
-      setError(true);
-      setLoading(false);
-      if (!(error instanceof LoginError)) {
+      if (error instanceof LoginError) {
+        setError(true);
+      } else {
         reportError(error);
       }
+      setLoading(false);
     }
   };
 

packages/app/src/screens/Settings.tsx

@@ -18,8 +18,7 @@ import ButtonGroup from '../components/drawer/ButtonGroup';
 import client from '../utils/bugsnag';
 import { SUBTEXT_SIZE } from '../constants/style';
 import { scheduleNotifications } from '../utils/notifications';
-import { LoginError } from '../utils/error';
-import { LOGIN_CREDENTIALS_CHANGED_MSG } from '../constants/fetch';
+import { SUCCESS, REFRESHED_MSG } from '../constants/fetch';
 
 const SettingIcon = styled(Icon)`
   font-size: ${SUBTEXT_SIZE};
@@ -49,12 +48,8 @@ export default authorizedRoute('Settings', function Settings() {
     try {
       await dispatch(fetchUserInfo(username, password));
       await scheduleNotifications();
-      notify('Success', 'Your information has been refreshed.');
+      notify(SUCCESS, REFRESHED_MSG);
     } catch (error) {
-      if (error instanceof LoginError) {
-        return notify('Error', LOGIN_CREDENTIALS_CHANGED_MSG);
-      }
-      notify('Error', error);
       reportError(error);
     }
     setRefreshing(false);

packages/app/src/utils/bugsnag.ts

@@ -29,7 +29,7 @@ function serializeState(state: AppState): IMetadata {
 }
 
 const config = new Configuration();
-config.codeBundleId = '3.0.1-b2';
+config.codeBundleId = '3.0.1-b3';
 config.notifyReleaseStages = ['production'];
 config.registerBeforeSendCallback((report) => {
   const state = store.getState();

packages/app/src/utils/process-info.ts

@@ -1,12 +1,13 @@
-import fetch from 'react-native-fetch-polyfill';
 import { load } from 'react-native-cheerio';
 import { DateType, DateSchema } from '@whs/server';
 
+import { fetch } from './utils';
 import { processSchedule } from './process-schedule';
 import {
   HEADER_SELECTOR, STUDENT_OVERVIEW_SELECTOR, STUDENT_ID_SELECTOR,
   SCHOOL_PICTURE_SELECTOR, SCHOOL_PICTURE_REGEX, SCHOOL_PICTURE_BLANK_FLAG, SCHOOL_PICTURE_BLANK_SYMBOL,
-  SCHEDULE_SELECTOR, SCHEDULE_REGEX, LOGIN_URL, FETCH_TIMEOUT, LOGIN_ERROR_SELECTOR, DATES_URL, SEARCH_URL, TEACHER_URL,
+  SCHEDULE_SELECTOR, SCHEDULE_REGEX, LOGIN_URL, FETCH_TIMEOUT, LOGIN_ERROR_SELECTOR, DATES_URL,
+  SEARCH_URL, TEACHER_URL, TEACHER_FETCH_LIMIT,
 } from '../constants/fetch';
 import { UserInfo, UserOverviewMap, UserOverviewKeys } from '../types/store';
 import { Schedule, TeacherSchedule, RawSchedule } from '../types/schedule';
@@ -23,7 +24,7 @@ export async function parseHTMLFromURL(url: string, options?: RequestInit) {
     ...options,
   });
   if (!response.ok) {
-    throw new NetworkError('Fetch from URL was not successful!');
+    throw new NetworkError();
   }
   const html = await response.text();
   return load(html);
@@ -112,6 +113,18 @@ export function getLoginError($: CheerioSelector) {
   return $(LOGIN_ERROR_SELECTOR).text().trim();
 }
 
+export async function fetchTeachersFromQuery(query: string, username: string, password: string, signal?: AbortSignal) {
+  const response = await fetch(getTeacherSearchURL(query, TEACHER_FETCH_LIMIT, username, password), {
+    method: 'POST',
+    timeout: FETCH_TIMEOUT,
+    signal,
+  });
+  if (!response.ok) {
+    throw new NetworkError();
+  }
+  return response.json();
+}
+
 /**
  * Refreshes a given collection of teacher schedules
  * @param teacherSchedules old teacher schedules to refresh
@@ -138,7 +151,7 @@ export async function getDates(type: DateType, year: number): Promise<DateSchema
     },
   });
   if (!response.ok) {
-    throw new NetworkError('Fetch for dates was not successful!');
+    throw new NetworkError();
   }
   return response.json();
 }