Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hookup create sections FE/BE #164

Merged
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
44f3d51
Wire up get course sections w/ create sections FE
chrisrrowland Jul 20, 2021
04bcb1a
Wire up adding sections
chrisrrowland Jul 22, 2021
64071b7
Handle error display
chrisrrowland Jul 27, 2021
76a1419
File name alignment fix
chrisrrowland Jul 28, 2021
0af6590
File name alignment fix
chrisrrowland Jul 28, 2021
69c4ecd
Refactor flow.
chrisrrowland Aug 2, 2021
4dd1d9e
Uncoment a thus far unused property
chrisrrowland Aug 4, 2021
586ffdb
Update ccm_web/client/src/api.ts
chrisrrowland Aug 4, 2021
477e551
Merge branch '88_get_sections_fe_be' of https://github.com/chrisrrowl…
chrisrrowland Aug 4, 2021
dfdbbf0
Non functional tweaking of getPost getPut
chrisrrowland Aug 4, 2021
c551ff2
Delete the forbidden comment
chrisrrowland Aug 4, 2021
75c9da5
Reorder some imports ~asthetics~
chrisrrowland Aug 4, 2021
2ab72f6
Remove an unused css style
chrisrrowland Aug 4, 2021
6df77d6
rename a variable
chrisrrowland Aug 4, 2021
f03449b
Only change to LoadingExistingSectionNamesFailed if getCanvasSectionD…
chrisrrowland Aug 5, 2021
d951d16
Prevent multiple submits
chrisrrowland Aug 5, 2021
6a60c85
Update ccm_web/client/src/pages/BulkSectionCreate.tsx
chrisrrowland Aug 5, 2021
5ef8b47
Update ccm_web/client/src/pages/BulkSectionCreate.tsx
chrisrrowland Aug 5, 2021
4503bbb
Update ccm_web/client/src/pages/BulkSectionCreate.tsx
chrisrrowland Aug 5, 2021
4bd2796
Escape quotes in text
chrisrrowland Aug 5, 2021
e70ddfb
Learning to spell basic words
chrisrrowland Aug 5, 2021
27b081d
Get rid of redundant code. Need to get better at javascript
chrisrrowland Aug 5, 2021
06dfdfd
Add Canvas settings URL (#3)
ssciolla Aug 5, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion ccm_web/client/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ function App (props: AppProps): JSX.Element {
<Switch>
<Route exact={true} path="/" render={() => (<Home globals={globals} ltiKey={props.ltiKey} course={course} setCourse={setCourse} getCourseError={getCourseError} />)} />
{features.map(feature => {
return <Route key={feature.data.id} path={feature.route} component={feature.component} />
return <Route key={feature.data.id} path={feature.route} component={() => <feature.component globals={globals} ltiKey={props.ltiKey} />}/>
})}
<Route render={() => (<div><em>Under Construction</em></div>)} />
</Switch>
Expand Down
41 changes: 28 additions & 13 deletions ccm_web/client/src/api.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { CanvasCourseBase } from './models/canvas'
import { CanvasCourseBase, CanvasCourseSection } from './models/canvas'
import { Globals } from './models/models'
import handleErrors from './utils/handleErrors'

Expand All @@ -23,6 +23,16 @@ const getGet = (key: string | undefined): RequestInit => {
return request
}

const getPost = (key: string | undefined, body: string): RequestInit => {
const headers: string[][] = []
headers.push(['Content-Type', 'application/json'])
headers.push(['Accept', 'application/json'])
chrisrrowland marked this conversation as resolved.
Show resolved Hide resolved
const request = initRequest(key, headers)
request.method = 'POST'
request.body = body
return request
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit-pick: There may be some benefit to keeping them separate, but other than the method, this is identical to getPut. We could parameterize this, make it getPutOrPost.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good suggestion. My IDE complains that some of the code I'm using for section enrollment dupes the section creation. I was thinking of seeing whether that could be combined.

// This currently assumes all put requests have a JSON payload and receive a JSON response.
const getPut = (key: string | undefined, body: string): RequestInit => {
const headers: string[][] = []
Expand Down Expand Up @@ -55,18 +65,23 @@ export const getGlobals = async (key: string | undefined): Promise<Globals> => {
return await resp.json()
}

const delay = async (ms: number): Promise<void> => {
await new Promise<void>(resolve => setTimeout(() => resolve(), ms))
// usage:
// const data = await delay(nnnn).then(() => {return something})
// const delay = async (ms: number): Promise<void> => {
// await new Promise<void>(resolve => setTimeout(() => resolve(), ms))
// }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// usage:
// const data = await delay(nnnn).then(() => {return something})
// const delay = async (ms: number): Promise<void> => {
// await new Promise<void>(resolve => setTimeout(() => resolve(), ms))
// }

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are code comments considered poor practice now? I feel like any time I leave commented code it's asked to be removed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally I won't leave code that is not used in the repo, But you can ask other developers what is the best practice here

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess, for me, I don't see the value of keeping this. Are we going to use it again? Even if we did, you could find it in the commit history.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are code comments considered poor practice now? I feel like any time I leave commented code it's asked to be removed.

Yeah. It's because they probably need to be removed in the future, but nobody ever remembers to go back and remove them. Or in the future, someone stumbles across it and doesn't understand the code or doesn't know whether should it be removed. So, out of fear of breaking something, it's never touched again.

On the flip side, don't worry about losing code forever when it's deleted. It will still be in the repo history.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't they just get minified out in production anyways?

I might remember that once upon a time I left myself a comment somewhere (Narrator: he won't) and look it up in the history, but nobody else is going to know that once upon a time there was a comment that might have been helpful when they are looking at some code for the first time.

tldr; get off my lawn ;-)


export const getCourseSections = async (key: string | undefined, courseId: number): Promise<CanvasCourseSection[]> => {
const request = getGet(key)
const resp = await fetch('/api/course/' + courseId.toString() + '/sections', request)
await handleErrors(resp)
return await resp.json()
}

// This is a placeholder for a real implementation (I mean, obviously :D)
export const getCourseSections = async (key: string | undefined, courseId: string): Promise<string[]> => {
const sections = await delay(2000).then(() => {
if (Math.random() * 3 > 1) {
return (['AAAA', 'BBBB'])
} else {
return new Promise<string[]>((resolve, reject) => { reject(new Error('Error retrieving course section information.')) })
}
})
return sections
export const addCourseSections = async (key: string | undefined, courseId: number, sectionNames: string[]): Promise<CanvasCourseSection[]> => {
const body = JSON.stringify({ sections: sectionNames })
const request = getPost(key, body)
const resp = await fetch('/api/course/' + courseId.toString() + '/sections', request)
await handleErrors(resp)
return await resp.json()
ssciolla marked this conversation as resolved.
Show resolved Hide resolved
}
6 changes: 4 additions & 2 deletions ccm_web/client/src/models/FeatureUIData.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,11 @@ import ConvertCanvasGradebook from '../pages/GradebookCanvas'
import MergeSections from '../pages/MergeSections'
import BulkSectionCreate from '../pages/BulkSectionCreate'
import { LtiProps } from '../api'
import { RoleEnum } from './models'
import { Globals, RoleEnum } from './models'

export interface CCMComponentProps extends LtiProps {}
export interface CCMComponentProps extends LtiProps {
globals: Globals
}

interface FeatureUIGroup {
id: string
Expand Down
6 changes: 6 additions & 0 deletions ccm_web/client/src/models/canvas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,9 @@ export interface CanvasCourseBase {
id: number
name: string
}

export interface CanvasCourseSection {
id: number
name: string
// total_students?: number
ssciolla marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it doesn't interfere with your current code, you can probably uncomment this. If it's not being used, your IDE may give you some warnings about it. If eslint warns about it, I think you can add a linting directive in a comment following the field to tell eslint to ignore it.

It'd be nice if the client Canvas models could use interfaces from the server Canvas models.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be nice if the client Canvas models could use interfaces from the server Canvas models.

💯

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if we ever created an issue for this. I did look into it, hopefully we'll get to it at some point.

}
4 changes: 4 additions & 0 deletions ccm_web/client/src/models/models.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,7 @@ export interface APIErrorData {
statusCode: number
errors: APIErrorPayload[]
}

export interface IDefaultError {
ssciolla marked this conversation as resolved.
Show resolved Hide resolved
errors: APIErrorPayload[]
}