{"action":"closed","number":1092,"pull_request":{"url":"https://api.github.com/repos/nusmodifications/nusmods/pulls/1092","id":207896108,"node_id":"MDExOlB1bGxSZXF1ZXN0MjA3ODk2MTA4","html_url":"https://github.com/nusmodifications/nusmods/pull/1092","diff_url":"https://github.com/nusmodifications/nusmods/pull/1092.diff","patch_url":"https://github.com/nusmodifications/nusmods/pull/1092.patch","issue_url":"https://api.github.com/repos/nusmodifications/nusmods/issues/1092","number":1092,"state":"closed","locked":false,"title":"Refactor request middleware and improve API error handling ","user":{"login":"ZhangYiJiang","id":445650,"node_id":"MDQ6VXNlcjQ0NTY1MA==","avatar_url":"https://avatars0.githubusercontent.com/u/445650?v=4","gravatar_id":"","url":"https://api.github.com/users/ZhangYiJiang","html_url":"https://github.com/ZhangYiJiang","followers_url":"https://api.github.com/users/ZhangYiJiang/followers","following_url":"https://api.github.com/users/ZhangYiJiang/following{/other_user}","gists_url":"https://api.github.com/users/ZhangYiJiang/gists{/gist_id}","starred_url":"https://api.github.com/users/ZhangYiJiang/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ZhangYiJiang/subscriptions","organizations_url":"https://api.github.com/users/ZhangYiJiang/orgs","repos_url":"https://api.github.com/users/ZhangYiJiang/repos","events_url":"https://api.github.com/users/ZhangYiJiang/events{/privacy}","received_events_url":"https://api.github.com/users/ZhangYiJiang/received_events","type":"User","site_admin":false},"body":"This PR deals with long standing issues with how we handle requests. It supersedes #715 and adds a bunch of other stuff. \r\n\r\nFirst, we formalize our API request handling strategy. We have two different data sources - \r\n\r\n- Fetch from API, which is async\r\n- Loaded from Redux, which is a cache for previously loaded data \r\n\r\nTo load data from the API\r\n\r\n1. Component dispatch a Request action. Each request to an unique API endpoint uses a different string key (previously the action type is used, but a single type of action may call different endpoints, so we add a unique key instead). The dispatch function returns a Promise (via Redux Thunk) to the component. \r\n2. Request middleware intercepts the action and does an actual fetch request. \r\n   - It also immediately fires a REQUEST action \r\n   - When the fetch is done, it fires a SUCCESS action with the payload \r\n   - If the fetch fails, it fires a FAILURE action with the error as the payload \r\n3. Request reducer stores the request action result (REQUEST, SUCCESS, FAILURE) keyed by the request key. The state is for when components other than the one which initiated the request wants to display eg. errors or statuses - components should if possible consume the Promise instead, since that's much simpler. \r\n4. Other reducers in module and venue bank consumes SUCCESS actions and update their cache accordingly. \r\n5. Meanwhile, the component directly pulls the data it needs from module and venue bank states. (This is essentially the stale-while-revalidate strategy Workbox uses) \r\n    - If this is the first time the data is loaded, then the component displays a spinner \r\n    - Otherwise the component displays the cached data immediately. 99% of the time the request result matches the state in the cache, so they don't notice anything.\r\n    - If the data is different, then the rendered component also updates. Again this is fine for the most part because we expect the changes to be small \r\n        - Exception: Sometimes modules remove or change lessons. We already automatically validate and force the local timetable state into a valid state, but we have yet to notify the user when this happens \r\n    - If the request fails, the promise returned to the component is rejected. We can then pop a notification informing the user they're seeing stale data, or if the data has never been loaded, show an error component \r\n   \r\nPhew, okay, so please help me check if I missed anything in the strategy above.  \r\n\r\nNow here are the actual changes in this PR \r\n\r\n- Refactor request middleware, state and reducer to allow for the above to be implemented \r\n- Remove existing helper and request actions, since they're never used anywhere \r\n- Add error handling to AppShell and ModulePageContainer \r\n\r\nFuture todos: \r\n\r\n- Show notification in case of timetable validation issues \r\n- Add retry options for API load errors \r\n- Add network warning when navigator.online is false for API load errors ","created_at":"2018-08-13T07:54:04Z","updated_at":"2018-08-13T09:08:44Z","closed_at":"2018-08-13T09:08:44Z","merged_at":"2018-08-13T09:08:44Z","merge_commit_sha":"c6e0a8cc898c993bff1ab116481f60f8663d6d7f","assignee":null,"assignees":[],"requested_reviewers":[],"requested_teams":[],"labels":[],"milestone":null,"commits_url":"https://api.github.com/repos/nusmodifications/nusmods/pulls/1092/commits","review_comments_url":"https://api.github.com/repos/nusmodifications/nusmods/pulls/1092/comments","review_comment_url":"https://api.github.com/repos/nusmodifications/nusmods/pulls/comments{/number}","comments_url":"https://api.github.com/repos/nusmodifications/nusmods/issues/1092/comments","statuses_url":"https://api.github.com/repos/nusmodifications/nusmods/statuses/a4dfae58ce12a05835c2914ee789fd503003ae62","head":{"label":"nusmodifications:refactor-request-middleware","ref":"refactor-request-middleware","sha":"a4dfae58ce12a05835c2914ee789fd503003ae62","user":{"login":"nusmodifications","id":7428865,"node_id":"MDEyOk9yZ2FuaXphdGlvbjc0Mjg4NjU=","avatar_url":"https://avatars3.githubusercontent.com/u/7428865?v=4","gravatar_id":"","url":"https://api.github.com/users/nusmodifications","html_url":"https://github.com/nusmodifications","followers_url":"https://api.github.com/users/nusmodifications/followers","following_url":"https://api.github.com/users/nusmodifications/following{/other_user}","gists_url":"https://api.github.com/users/nusmodifications/gists{/gist_id}","starred_url":"https://api.github.com/users/nusmodifications/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/nusmodifications/subscriptions","organizations_url":"https://api.github.com/users/nusmodifications/orgs","repos_url":"https://api.github.com/users/nusmodifications/repos","events_url":"https://api.github.com/users/nusmodifications/events{/privacy}","received_events_url":"https://api.github.com/users/nusmodifications/received_events","type":"Organization","site_admin":false},"repo":{"id":5481861,"node_id":"MDEwOlJlcG9zaXRvcnk1NDgxODYx","name":"nusmods","full_name":"nusmodifications/nusmods","owner":{"login":"nusmodifications","id":7428865,"node_id":"MDEyOk9yZ2FuaXphdGlvbjc0Mjg4NjU=","avatar_url":"https://avatars3.githubusercontent.com/u/7428865?v=4","gravatar_id":"","url":"https://api.github.com/users/nusmodifications","html_url":"https://github.com/nusmodifications","followers_url":"https://api.github.com/users/nusmodifications/followers","following_url":"https://api.github.com/users/nusmodifications/following{/other_user}","gists_url":"https://api.github.com/users/nusmodifications/gists{/gist_id}","starred_url":"https://api.github.com/users/nusmodifications/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/nusmodifications/subscriptions","organizations_url":"https://api.github.com/users/nusmodifications/orgs","repos_url":"https://api.github.com/users/nusmodifications/repos","events_url":"https://api.github.com/users/nusmodifications/events{/privacy}","received_events_url":"https://api.github.com/users/nusmodifications/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/nusmodifications/nusmods","description":"🏫 Unofficial course planning platform for National University of Singapore.","fork":false,"url":"https://api.github.com/repos/nusmodifications/nusmods","forks_url":"https://api.github.com/repos/nusmodifications/nusmods/forks","keys_url":"https://api.github.com/repos/nusmodifications/nusmods/keys{/key_id}","collaborators_url":"https://api.github.com/repos/nusmodifications/nusmods/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/nusmodifications/nusmods/teams","hooks_url":"https://api.github.com/repos/nusmodifications/nusmods/hooks","issue_events_url":"https://api.github.com/repos/nusmodifications/nusmods/issues/events{/number}","events_url":"https://api.github.com/repos/nusmodifications/nusmods/events","assignees_url":"https://api.github.com/repos/nusmodifications/nusmods/assignees{/user}","branches_url":"https://api.github.com/repos/nusmodifications/nusmods/branches{/branch}","tags_url":"https://api.github.com/repos/nusmodifications/nusmods/tags","blobs_url":"https://api.github.com/repos/nusmodifications/nusmods/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/nusmodifications/nusmods/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/nusmodifications/nusmods/git/refs{/sha}","trees_url":"https://api.github.com/repos/nusmodifications/nusmods/git/trees{/sha}","statuses_url":"https://api.github.com/repos/nusmodifications/nusmods/statuses/{sha}","languages_url":"https://api.github.com/repos/nusmodifications/nusmods/languages","stargazers_url":"https://api.github.com/repos/nusmodifications/nusmods/stargazers","contributors_url":"https://api.github.com/repos/nusmodifications/nusmods/contributors","subscribers_url":"https://api.github.com/repos/nusmodifications/nusmods/subscribers","subscription_url":"https://api.github.com/repos/nusmodifications/nusmods/subscription","commits_url":"https://api.github.com/repos/nusmodifications/nusmods/commits{/sha}","git_commits_url":"https://api.github.com/repos/nusmodifications/nusmods/git/commits{/sha}","comments_url":"https://api.github.com/repos/nusmodifications/nusmods/comments{/number}","issue_comment_url":"https://api.github.com/repos/nusmodifications/nusmods/issues/comments{/number}","contents_url":"https://api.github.com/repos/nusmodifications/nusmods/contents/{+path}","compare_url":"https://api.github.com/repos/nusmodifications/nusmods/compare/{base}...{head}","merges_url":"https://api.github.com/repos/nusmodifications/nusmods/merges","archive_url":"https://api.github.com/repos/nusmodifications/nusmods/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/nusmodifications/nusmods/downloads","issues_url":"https://api.github.com/repos/nusmodifications/nusmods/issues{/number}","pulls_url":"https://api.github.com/repos/nusmodifications/nusmods/pulls{/number}","milestones_url":"https://api.github.com/repos/nusmodifications/nusmods/milestones{/number}","notifications_url":"https://api.github.com/repos/nusmodifications/nusmods/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/nusmodifications/nusmods/labels{/name}","releases_url":"https://api.github.com/repos/nusmodifications/nusmods/releases{/id}","deployments_url":"https://api.github.com/repos/nusmodifications/nusmods/deployments","created_at":"2012-08-20T13:58:35Z","updated_at":"2018-08-13T00:31:37Z","pushed_at":"2018-08-13T09:08:44Z","git_url":"git://github.com/nusmodifications/nusmods.git","ssh_url":"git@github.com:nusmodifications/nusmods.git","clone_url":"https://github.com/nusmodifications/nusmods.git","svn_url":"https://github.com/nusmodifications/nusmods","homepage":"https://nusmods.com","size":11049,"stargazers_count":204,"watchers_count":204,"language":"JavaScript","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":97,"mirror_url":null,"archived":false,"open_issues_count":74,"license":{"key":"mit","name":"MIT License","spdx_id":"MIT","url":"https://api.github.com/licenses/mit","node_id":"MDc6TGljZW5zZTEz"},"forks":97,"open_issues":74,"watchers":204,"default_branch":"master"}},"base":{"label":"nusmodifications:master","ref":"master","sha":"26b7abb29851701185691d0f25e7490dc59ae8e3","user":{"login":"nusmodifications","id":7428865,"node_id":"MDEyOk9yZ2FuaXphdGlvbjc0Mjg4NjU=","avatar_url":"https://avatars3.githubusercontent.com/u/7428865?v=4","gravatar_id":"","url":"https://api.github.com/users/nusmodifications","html_url":"https://github.com/nusmodifications","followers_url":"https://api.github.com/users/nusmodifications/followers","following_url":"https://api.github.com/users/nusmodifications/following{/other_user}","gists_url":"https://api.github.com/users/nusmodifications/gists{/gist_id}","starred_url":"https://api.github.com/users/nusmodifications/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/nusmodifications/subscriptions","organizations_url":"https://api.github.com/users/nusmodifications/orgs","repos_url":"https://api.github.com/users/nusmodifications/repos","events_url":"https://api.github.com/users/nusmodifications/events{/privacy}","received_events_url":"https://api.github.com/users/nusmodifications/received_events","type":"Organization","site_admin":false},"repo":{"id":5481861,"node_id":"MDEwOlJlcG9zaXRvcnk1NDgxODYx","name":"nusmods","full_name":"nusmodifications/nusmods","owner":{"login":"nusmodifications","id":7428865,"node_id":"MDEyOk9yZ2FuaXphdGlvbjc0Mjg4NjU=","avatar_url":"https://avatars3.githubusercontent.com/u/7428865?v=4","gravatar_id":"","url":"https://api.github.com/users/nusmodifications","html_url":"https://github.com/nusmodifications","followers_url":"https://api.github.com/users/nusmodifications/followers","following_url":"https://api.github.com/users/nusmodifications/following{/other_user}","gists_url":"https://api.github.com/users/nusmodifications/gists{/gist_id}","starred_url":"https://api.github.com/users/nusmodifications/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/nusmodifications/subscriptions","organizations_url":"https://api.github.com/users/nusmodifications/orgs","repos_url":"https://api.github.com/users/nusmodifications/repos","events_url":"https://api.github.com/users/nusmodifications/events{/privacy}","received_events_url":"https://api.github.com/users/nusmodifications/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/nusmodifications/nusmods","description":"🏫 Unofficial course planning platform for National University of Singapore.","fork":false,"url":"https://api.github.com/repos/nusmodifications/nusmods","forks_url":"https://api.github.com/repos/nusmodifications/nusmods/forks","keys_url":"https://api.github.com/repos/nusmodifications/nusmods/keys{/key_id}","collaborators_url":"https://api.github.com/repos/nusmodifications/nusmods/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/nusmodifications/nusmods/teams","hooks_url":"https://api.github.com/repos/nusmodifications/nusmods/hooks","issue_events_url":"https://api.github.com/repos/nusmodifications/nusmods/issues/events{/number}","events_url":"https://api.github.com/repos/nusmodifications/nusmods/events","assignees_url":"https://api.github.com/repos/nusmodifications/nusmods/assignees{/user}","branches_url":"https://api.github.com/repos/nusmodifications/nusmods/branches{/branch}","tags_url":"https://api.github.com/repos/nusmodifications/nusmods/tags","blobs_url":"https://api.github.com/repos/nusmodifications/nusmods/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/nusmodifications/nusmods/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/nusmodifications/nusmods/git/refs{/sha}","trees_url":"https://api.github.com/repos/nusmodifications/nusmods/git/trees{/sha}","statuses_url":"https://api.github.com/repos/nusmodifications/nusmods/statuses/{sha}","languages_url":"https://api.github.com/repos/nusmodifications/nusmods/languages","stargazers_url":"https://api.github.com/repos/nusmodifications/nusmods/stargazers","contributors_url":"https://api.github.com/repos/nusmodifications/nusmods/contributors","subscribers_url":"https://api.github.com/repos/nusmodifications/nusmods/subscribers","subscription_url":"https://api.github.com/repos/nusmodifications/nusmods/subscription","commits_url":"https://api.github.com/repos/nusmodifications/nusmods/commits{/sha}","git_commits_url":"https://api.github.com/repos/nusmodifications/nusmods/git/commits{/sha}","comments_url":"https://api.github.com/repos/nusmodifications/nusmods/comments{/number}","issue_comment_url":"https://api.github.com/repos/nusmodifications/nusmods/issues/comments{/number}","contents_url":"https://api.github.com/repos/nusmodifications/nusmods/contents/{+path}","compare_url":"https://api.github.com/repos/nusmodifications/nusmods/compare/{base}...{head}","merges_url":"https://api.github.com/repos/nusmodifications/nusmods/merges","archive_url":"https://api.github.com/repos/nusmodifications/nusmods/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/nusmodifications/nusmods/downloads","issues_url":"https://api.github.com/repos/nusmodifications/nusmods/issues{/number}","pulls_url":"https://api.github.com/repos/nusmodifications/nusmods/pulls{/number}","milestones_url":"https://api.github.com/repos/nusmodifications/nusmods/milestones{/number}","notifications_url":"https://api.github.com/repos/nusmodifications/nusmods/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/nusmodifications/nusmods/labels{/name}","releases_url":"https://api.github.com/repos/nusmodifications/nusmods/releases{/id}","deployments_url":"https://api.github.com/repos/nusmodifications/nusmods/deployments","created_at":"2012-08-20T13:58:35Z","updated_at":"2018-08-13T00:31:37Z","pushed_at":"2018-08-13T09:08:44Z","git_url":"git://github.com/nusmodifications/nusmods.git","ssh_url":"git@github.com:nusmodifications/nusmods.git","clone_url":"https://github.com/nusmodifications/nusmods.git","svn_url":"https://github.com/nusmodifications/nusmods","homepage":"https://nusmods.com","size":11049,"stargazers_count":204,"watchers_count":204,"language":"JavaScript","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":97,"mirror_url":null,"archived":false,"open_issues_count":74,"license":{"key":"mit","name":"MIT License","spdx_id":"MIT","url":"https://api.github.com/licenses/mit","node_id":"MDc6TGljZW5zZTEz"},"forks":97,"open_issues":74,"watchers":204,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/nusmodifications/nusmods/pulls/1092"},"html":{"href":"https://github.com/nusmodifications/nusmods/pull/1092"},"issue":{"href":"https://api.github.com/repos/nusmodifications/nusmods/issues/1092"},"comments":{"href":"https://api.github.com/repos/nusmodifications/nusmods/issues/1092/comments"},"review_comments":{"href":"https://api.github.com/repos/nusmodifications/nusmods/pulls/1092/comments"},"review_comment":{"href":"https://api.github.com/repos/nusmodifications/nusmods/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/nusmodifications/nusmods/pulls/1092/commits"},"statuses":{"href":"https://api.github.com/repos/nusmodifications/nusmods/statuses/a4dfae58ce12a05835c2914ee789fd503003ae62"}},"author_association":"CONTRIBUTOR","merged":true,"mergeable":null,"rebaseable":null,"mergeable_state":"unknown","merged_by":{"login":"ZhangYiJiang","id":445650,"node_id":"MDQ6VXNlcjQ0NTY1MA==","avatar_url":"https://avatars0.githubusercontent.com/u/445650?v=4","gravatar_id":"","url":"https://api.github.com/users/ZhangYiJiang","html_url":"https://github.com/ZhangYiJiang","followers_url":"https://api.github.com/users/ZhangYiJiang/followers","following_url":"https://api.github.com/users/ZhangYiJiang/following{/other_user}","gists_url":"https://api.github.com/users/ZhangYiJiang/gists{/gist_id}","starred_url":"https://api.github.com/users/ZhangYiJiang/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ZhangYiJiang/subscriptions","organizations_url":"https://api.github.com/users/ZhangYiJiang/orgs","repos_url":"https://api.github.com/users/ZhangYiJiang/repos","events_url":"https://api.github.com/users/ZhangYiJiang/events{/privacy}","received_events_url":"https://api.github.com/users/ZhangYiJiang/received_events","type":"User","site_admin":false},"comments":2,"review_comments":0,"maintainer_can_modify":false,"commits":11,"additions":586,"deletions":273,"changed_files":22}}
{"actor":{"display_login":"ZhangYiJiang"}}