Allow course sharing name to be changed

[Michael-Holmes-SDE]

This pull request allows the 'Course Sharing Name' to be changed, as long as the course has no publicly shared questions and no questions have been imported from the current course

implements part of #7895

[github-actions]

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

[Michael-Holmes-SDE]

I have reviewed and hereby sign the CLA

[Michael-Holmes-SDE]

recheck

[nwalters512]

One UI comment; other than that, you'll need to run our formatter on this code. You can either rely on the extensions/settings in the .vscode directory if you're using VS Code, or manually run make format-js if you're not.

[nwalters512]

We should be able to figure out whether or not renaming is allowed at the instant the page is loaded and update the UI to say concretely "you can/cannot rename this" and disable buttons/etc. appropriately.

[SethPoulsen]

@Michael-Holmes-SDE we talked about this in our developer meeting and decided that the easiest thing to do is this: The 'Choose sharing name' button on the page should always be enabled.

If you click on it and you are allowed to change it, the dialog you see should say that you will be allowed to edit it until you share questions.

If you click on it and you are not allowed to change it, the dialog should say that you cannot change the sharing name, because you already have shared questions--and then the dialog should not have the 'choose sharing name' button on it, just the close button to close the dialog.

[SethPoulsen]

Thanks for your hard work @Michael-Holmes-SDE this is a great start!

As Nathan said, you'll need to get the code formatters working, and you should have the button enabled/disabled based on whether they are allowed to edit.

The other thing you'll need to do (I can't remember if I told you this before or not, sorry) is to add some automated tests verifying that the functionality works.

The automated tests for this page are in apps/prairielearn/src/tests/questionSharing.test.ts.

You'll want to have one test where you can successfully change the name if nothing has been shared yet, probably around here:

PrairieLearn/apps/prairielearn/src/tests/questionSharing.test.ts

Lines 189 to 210 in 1b3268b

	step('Fail if trying to set an invalid sharing name', async () => {
	let res = await setSharingName(sharingCourse.id, 'invalid@sharingname');
	assert(res.status === 400);
	res = await setSharingName(sharingCourse.id, 'invalid / sharingname');
	assert(res.status === 400);
	res = await setSharingName(sharingCourse.id, '');
	assert(res.status === 400);
	});
	step('Set consuming course sharing name', async () => {
	await setSharingName(consumingCourse.id, CONSUMING_COURSE_SHARING_NAME);
	const sharingPage = await fetchCheerio(sharingPageUrl(consumingCourse.id));
	assert(sharingPage.ok);
	assert.include(
	sharingPage.$('[data-testid="sharing-name"]').text(),
	CONSUMING_COURSE_SHARING_NAME,
	);
	});

and they you'll want another test where it fails to change the name if they have already shared something, probably after this code:

PrairieLearn/apps/prairielearn/src/tests/questionSharing.test.ts

Lines 350 to 376 in 1b3268b

	step(`Add question "${SHARING_QUESTION_QID}" to sharing set`, async () => {
	const result = await sqldb.queryOneRowAsync(sql.get_question_id, {
	course_id: sharingCourse.id,
	qid: SHARING_QUESTION_QID,
	});
	const questionSettingsUrl = `${baseUrl}/course_instance/${sharingCourse.id}/instructor/question/${result.rows[0].id}/settings`;
	const resGet = await fetchCheerio(questionSettingsUrl);
	assert(resGet.ok);
	const token = resGet.$('#test_csrf_token').text();
	const resPost = await fetch(questionSettingsUrl, {
	method: 'POST',
	body: new URLSearchParams({
	__action: 'sharing_set_add',
	__csrf_token: token,
	unsafe_sharing_set_id: '1',
	}),
	});
	assert(resPost.ok);
	const settingsPageResponse = await fetchCheerio(questionSettingsUrl);
	assert.include(
	settingsPageResponse.$('[data-testid="shared-with"]').text(),
	SHARING_SET_NAME,
	);
	});
	});

see the docs about running tests here: https://prairielearn.readthedocs.io/en/latest/installingLocal/#running-the-test-suite

You only need to worry about running this one test file (yarn mocha src/tests/questionSharing.test.ts), you don't need to worry about running the whole test suite locally

[codecov]

Codecov Report

Attention: Patch coverage is 98.90110% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 66.97%. Comparing base (37f5638) to head (bc3bbd3).
Report is 2 commits behind head on master.

Files	Patch %	Lines
...eAdminSharing/instructorCourseAdminSharing.html.ts	98.38%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #9860      +/-   ##
==========================================
+ Coverage   66.78%   66.97%   +0.19%     
==========================================
  Files         454      456       +2     
  Lines       71046    71307     +261     
  Branches     5679     5714      +35     
==========================================
+ Hits        47446    47760     +314     
+ Misses      23175    23120      -55     
- Partials      425      427       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[nwalters512]

We could use some more types here, and I'd rather use our Modal component instead of handcrafting one, but this isn't directly related to this PR. I'll open a followup to fix these things.

[nwalters512]

This input lacks an accessible label. I know this is not a change in this PR, but I'd very much appreciate adding one. IMO we also don't need to use form-control-sm here.

I also don't think this should go in the footer, or be right-aligned. Let's move this input under the "Enter the sharing name you would like..." text.

[Michael-Holmes-SDE]

Added a label and moved the input:

• 

[SethPoulsen]

@nwalters512 is there an easy way to auto-approve the CI build for all PRs from @Michael-Holmes-SDE? I don't want to have to keep hitting 'approve' over and over again.

[nwalters512]

@SethPoulsen once he lands any PR, including this one, all future CI runs will happen automatically.