lti: add launching basic #70

Merged

bwc9876.dev 6d ago

targeting ben/lti-deep-link from ben/lti-launch

Merged opened by and assigned to

bwc9876.dev 6d ago targeting ben/lti-deep-link from ben/lti-launch

Assignments can now be launched by LMS users after an insertion
Some fixes to deep linking
Added AGS stuff but we won't use that quite yet
It's redirecting to a 404 right now, which will be where the student can download the ZIP, launch and environment, or submit

Commits 0

Could not load commits.

Comments

v0 6d ago

ellis.rocks 6c08701b 3d ago

LGTM. Some small things:

pkgs/rs-top/main.rs:40 subpath, I assume
pkgs/rs-database/models.rs:139 context an ...?
pkgs/rs-lti/authorization.rs:65 perhaps add a small comment about why, along the line of our conversation yesterday

v1 3d ago

huy55465.tngl.sh 2d ago

pkgs/rs-database/models.rs:160 ehe

huy55465.tngl.sh 2d ago

pkgs/rs-database/models.rs:170 wait does Rust have auto-deduced function return type?

huy55465.tngl.sh 2d ago

pkgs/rs-database/models.rs:227 hmm why do you need the extra level of nesting her?

huy55465.tngl.sh 2d ago

pkgs/rs-lti/routes.rs:519 probably won't really affect anything, but would something like this work as well?

	let redir = lti_state
		.base_uri
		.join(&format!(
			"/{}/work/{}",
			lti_state.assignment_subpath,
			work.id()
		))
		.context("Failed to join URL");

huy55465.tngl.sh 2d ago

pkgs/rs-lti/ags.rs:71 would it be nicer to create yet another type just for passing arguments to here? 🐱

huy55465.tngl.sh 2d ago

well, none of those I commented on are blockers, so LGTM

bwc9876.dev 1d ago

Rust does not but the diesel::dsl::autotype macro on that function is what determines it.
That might work, Url::join is very temperamental and honestly just a bad API. I might go through and get rid of all usages of it at some point.
Yeah I will probably redo that function at some point

Versions