jonathan-davies/create-string-interpolated-links

[1strangequark]

This adds link string interpolation. However, since user accounts are not currently added to a class, fetching the class_id from GET /classes/me yields a null value. As such, I have string constants that take the place of the class_id and assignment_id while we implement the functionality to add a student to a class and get that data back.

[krashanoff]

Hey Jonathan, this is looking good but there's a few changes that need to be made to help the performance of the page.

On another note, please delete package-lock.json. This is the reason that the code delta for this PR is almost 30k lines. Removing it will reduce it to the actual number, maybe 100 or so. You can use npm install to add dependencies, but don't commit the lockfile for it. The package manager we're using is yarn. To add dependencies with yarn, use yarn add PACKAGE_NAME.

[krashanoff]

You can statically type this and then make the call with something like: fetchClassInfo(cookies?.jwt).

Suggested change

	function fetchClassInfo(token: any) {
	fetch("http://localhost:8080/class/me", {
	method: "GET",
	mode: "cors",
	headers: {
	"Content-Type": "application/json",
	Authorization: String(token),
	},
	})
	.then((response) => {
	if (response.status === 401) throw new Error("Unauthorized");
	return response.json();
	})
	.then((json) => {
	if (json?.classes[0].id) {
	setClassID(json?.classes[0].id);
	}
	})
	.catch((e) => {
	setError(
	`Failed to get class info! Server responded with: ${String(e).replace(
	"TypeError: ",
	""
	)}`
	);
	});
	function fetchClassInfo(token: string) {
	fetch("http://localhost:8080/class/me", {
	method: "GET",
	mode: "cors",
	headers: {
	"Content-Type": "application/json",
	Authorization: token,
	},
	})
	.then((response) => {
	if (response.status === 401) throw new Error("Unauthorized");
	return response.json();
	})
	.then((json) => {
	if (json?.classes[0].id) {
	setClassID(json?.classes[0].id);
	}
	})
	.catch((e) => {
	setError(
	`Failed to get class info! Server responded with: ${String(e).replace(
	"TypeError: ",
	""
	)}`
	);
	});

[krashanoff]

This will kill the performance of this page. You fire the function getClassInfo when the component draws. However, if you modify the state of the component, it will redraw. This is an infinite loop which will crash the app.

Wrap it in an useEffect() call, then store the state of the response so you can use it for future navigation, rather than making a call every time.

Suggested change

	getClassInfo(cookies.jwt);
	useEffect(() => {
	getClassInfo(cookies.jwt);
	// TODO: set the state of the component.
	}, [cookies.jwt]);

[krashanoff]

Please refer to the other comment in this file. Making a single request on component-mount will save a lot of trouble.

Suggested change

	fetchClassInfo(cookies.jwt);
	nav("/class/" + classID + "/classstats");
	nav("/class/" + classID + "/classstats");

[krashanoff]

Redundant as per L69:93.

Suggested change

	function fetchClassInfo(token: any) {
	fetch("http://localhost:8080/class/me", {
	method: "GET",
	mode: "cors",
	headers: {
	"Content-Type": "application/json",
	Authorization: String(token),
	},
	})
	.then((response) => {
	if (response.status === 401) throw new Error("Unauthorized");
	return response.json();
	})
	.then((json) => {
	setClassID(json.id);
	})
	.catch((e) => {
	setError(
	`Failed to get class info! Server responded with: ${String(e).replace(
	"TypeError: ",
	""
	)}`
	);
	});
	}