Add Grading API calls #238

remo5000 · 2018-07-26T06:05:07Z

Awaiting source-academy/backend#167.

Features

Add sagas to fetch Grading from backend
Use XP instead of grade
Remove graded option (to be added once backend endpoint is available)

TODO

Add GradingOverview calls and type changes
Add Grading calls and type changes
Test with an updated staging server
- Adjustments in grading overview (awating backend)
- Assessment name and category

coveralls · 2018-07-26T06:09:07Z

Pull Request Test Coverage Report for Build 527

14 of 93 (15.05%) changed or added relevant lines in 8 files are covered.
3 unchanged lines in 2 files lost coverage.
Overall coverage decreased (-0.7%) to 24.799%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/actions/session.ts	1	2	50.0%
src/reducers/workspaces.ts	0	1	0.0%
src/utils/castBackend.ts	1	4	25.0%
src/mocks/backend.ts	1	10	10.0%
src/components/academy/grading/GradingEditor.tsx	4	16	25.0%
src/sagas/backend.ts	2	55	3.64%

Files with Coverage Reduction	New Missed Lines	%
src/reducers/workspaces.ts	1	7.94%
src/components/academy/grading/GradingEditor.tsx	2	14.1%

Totals
Change from base Build 515:	-0.7%
Covered Lines:	806
Relevant Lines:	2766

💛 - Coveralls

ning-y · 2018-07-29T12:22:09Z

@remo5000 You gotta rebase off #240 before you use API call in ./src/sagas/backend.ts because the function request has been changed completely in that PR.

remo5000 · 2018-07-30T05:21:02Z

@ningyuansg will do

ning-y · 2018-07-30T09:19:18Z

src/components/academy/grading/GradingEditor.tsx

+    g: number,
+    s: string,
+    i: number | undefined
+  ) => void


Full param names (since prettier put them on separate lines already). I think abbreviated param names is acceptable just for stuff like the tests.

ning-y · 2018-07-30T09:20:17Z

src/components/academy/grading/GradingEditor.tsx

 type State = {
  mdeState: ReactMdeTypes.MdeState
-  XPInput: number | undefined
+  adjustmentInput: number | undefined


Can this be number | null, like that time with the editorValue

ning-y · 2018-07-30T09:22:44Z

src/containers/academy/grading/GradingEditorContainer.ts

-    gradingXP: state.workspaces.grading.gradingXP
-  }
-}
+const mapStateToProps: MapStateToProps<{}, {}, IState> = state => ({})


You can pass null to connect in lieu of an empty mapStateToProps.

ning-y · 2018-07-30T09:30:41Z

src/sagas/backend.ts

+          errorMessage = 'Invalid or missing parameter(s) or submission and/or question not found'
+          break
+        case 401:
+          errorMessage = 'Got 403 response. Only staff can save gradings.'


Typo here (should be '403'). Also, can you help me change this line to case 401?

Anyways I think this case clause can be omitted. Anyone using the site properly would be a staff by the point they reach here, so it doesn't make sense to remind them that only a staff can save gradings. This should fall in the default clause of 'oops we don't know what happened, here's the error code, go open an issue please'.

ning-y · 2018-07-30T09:32:48Z

src/sagas/backend.ts

+    refreshToken: tokens.refreshToken,
+    shouldRefresh: true
+  })
+  if (response && response.ok) {


if (response) is sufficient. Non-null response implies response.ok. Reference.

ning-y · 2018-07-30T09:33:09Z

src/sagas/backend.ts

+    refreshToken: tokens.refreshToken,
+    shouldRefresh: true
+  })
+  if (response && response.ok) {


if (response)

ning-y · 2018-07-30T09:34:43Z

src/utils/castBackend.ts

@@ -0,0 +1,15 @@
+import { ExternalLibraryName, Library } from '../components/assessment/assessmentShape'
+
+export const castLibrary = (lib: any): Library => ({


Good to document differences in frontend and backend data, like here.

remo5000 · 2018-08-02T08:25:22Z

There was a problem with typing, as the input as a number was stored. I've stored the input as a string, and parse it whenever this data leaves the component (submission)

And a helper file castBackend.ts to help cast thigs like the Library

It does not need to be stored in the state anymore

this is undefined in the constructor for some reason, but props can be used directly from paramters.

Param names, and using null

ning-y · 2018-08-02T10:26:50Z

src/sagas/backend.ts

+      refreshToken: state.session.refreshToken
+    }))
+    const resp = yield postGrading(submissionId, questionId, grade, comment, adjustment, tokens)
+    if (resp !== null && resp.ok) {


Use resp && resp.ok for consistency

ning-y · 2018-08-02T10:31:06Z

src/sagas/backend.ts

+      let errorMessage: string
+      switch (resp.status) {
+        case 400:
+          errorMessage = 'Invalid or missing parameter(s) or submission and/or question not found'


These error messages are for the end user. If they're not providing useful end-user-type information I think just let them be caught in the default case.

ning-y · 2018-08-02T10:32:12Z

src/sagas/backend.ts

+          errorMessage = 'Invalid or missing parameter(s) or submission and/or question not found'
+          break
+        case 403:
+          errorMessage = 'Got 403 response. Only staff can save gradings.'


Check role before sending the POST instead of based on the POST response (see SUBMIT_ANSWER)

Sure, but I think it's good to note that in a normal case the student would not be able to reach the UI to submit the grading anyway

Updated the codes to match the new ones too

Bumps [distillery](https://github.com/bitwalker/distillery) from 2.0.3 to 2.0.4. - [Release notes](https://github.com/bitwalker/distillery/releases) - [Changelog](https://github.com/bitwalker/distillery/blob/master/docs/changelog.md) - [Commits](bitwalker/distillery@2.0.3...2.0.4) Signed-off-by: dependabot[bot] <support@dependabot.com>

remo5000 force-pushed the grading-api branch from 9b1c605 to d0a5d52 Compare July 27, 2018 04:49

remo5000 changed the title ~~[WIP] Add Grading API calls~~ Add Grading API calls Jul 27, 2018

remo5000 force-pushed the grading-api branch from 6f07ca8 to 572d9de Compare July 30, 2018 06:49

ning-y requested changes Jul 30, 2018

View reviewed changes

ning-y added the waiting label Jul 31, 2018

remo5000 removed the waiting label Aug 2, 2018

remo5000 added 20 commits August 2, 2018 18:13

Scaffold GradingOverview function

6772416

Change GradingOverview to fit backend call

95ed081

Modify mock data to fit new GradingOverview

2971518

Modify link in table

4ee7a74

Edit table rendering

11b4b73

Add scaffold function getGrading

9cd6e5d

Add conversion for grading

76c7d7b

And a helper file castBackend.ts to help cast thigs like the Library

Modify gradingShape to fit backend

2489f56

Fix syntax and import errors for saga

d160197

Fix grading usage in component

4bb7f93

Modify mock gradings

398f209

Fix imports for castBackend

168f6f2

Scaffold semantic changes to GradingEditor

c6aa1bb

Fix a minor naming error for GradingEditor

c127eaa

Modify props for GradingEditor

56a7a72

Remove StateProps for GradingEditor

bef826b

Change props passed to GradingEditor

21c5873

Remove actions related to updating xp and comments

eebb6da

It does not need to be stored in the state anymore

Remove gradingCommentsValue & gradingXP from state

f984702

Rename action arguments and handler

9f95f5b

remo5000 added 20 commits August 2, 2018 18:15

Move postGrading

11268ab

Refactor postGrading

52d514c

Add getGradingOverviews again

6b96677

Add getGrading method again

f7714c0

Fix imports

3f439a6

Fix response param parsing for getGradingOverviews

799895f

Fix response parsing for getGrading

3c0ef15

Fix constructor for GradingEditor

fb686db

this is undefined in the constructor for some reason, but props can be used directly from paramters.

Format file

056efee

Use string for adjustmentInput

b7785d8

Format file

8f0eef0

Remove failsafe for adjustment

9f9a4b3

Make some requested changes

b6c9007

Param names, and using null

Remove redundant mapStateToProps

77abac3

Add review changes for saga

517c411

Add documentation for castLibrary

8b50f92

Format files

81d2df2

Add warning for out of bound grading

7c4d25f

Add documentation

9b3fa99

Format file

7a6bfde

remo5000 force-pushed the grading-api branch from 22b9da9 to 7a6bfde Compare August 2, 2018 10:27

ning-y requested changes Aug 2, 2018

View reviewed changes

remo5000 added 4 commits August 3, 2018 13:43

Update request body for postGrading

ce0e929

Remove grade parameter from submitGrading

33ec404

Remove grade for saga function

7ba31a2

Make review changes

0e475f4

ning-y approved these changes Aug 3, 2018

View reviewed changes

ning-y merged commit d27e7b9 into master Aug 3, 2018

ning-y deleted the grading-api branch August 3, 2018 10:30

		@@ -0,0 +1,15 @@
		import { ExternalLibraryName, Library } from '../components/assessment/assessmentShape'

		export const castLibrary = (lib: any): Library => ({

Add Grading API calls #238

Add Grading API calls #238

Uh oh!

Conversation

remo5000 commented Jul 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Features

TODO

Uh oh!

coveralls commented Jul 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 527

💛 - Coveralls

Uh oh!

ning-y commented Jul 29, 2018

Uh oh!

remo5000 commented Jul 30, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

remo5000 commented Aug 2, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

remo5000 commented Jul 26, 2018 •

edited

Loading

coveralls commented Jul 26, 2018 •

edited

Loading