CUNY Academic Commons

Yes, we can add something like this. Currently, we just set the 'instructor' to the course creator, and the only way this can be changed is if a network admin later goes and changes it in the dashboard.

Questions about the workflow:
1. Do we prompt for Instructor during the creation workflow? If so, where? Perhaps this would be one of the fields that appears if you select 'Teaching' as the site purpose, with the default value set to the current user.
2. Do we offer this setting for Teaching groups as well, or only sites?
3. Can anyone make anyone else an Instructor? I can imagine a case where a user makes someone else an "instructor" just for a laugh. We could (a) limit who can select Instructors, (b) limit who can be selected as an Instructor, (c) introduce some some sort of verification step (an email to the designated instructor, with a link that must be clicked?), or (d) do nothing and just hope that it's not abused

Thanks for this feedback Boone. Some thoughts below...

Questions about the workflow:
1. Do we prompt for Instructor during the creation workflow? If so, where? Perhaps this would be one of the fields that appears if you select 'Teaching' as the site purpose, with the default value set to the current user.

Related to question 3 below, i think the addition of this feature in the creation process could cause more confusion than if it's implemented as a setting that is available once the group or site is created and running. If implemented within the group/site settings only admins on the teaching group or site could be listed as instructor.

2. Do we offer this setting for Teaching groups as well, or only sites?

Yes, I think we want to be able to indicate a different admin as instructor for both groups and sites.

3. Can anyone make anyone else an Instructor? I can imagine a case where a user makes someone else an "instructor" just for a laugh.

I can imagine this maybe happening too, which is why i suggested that in order to be listed as instructor the person must be listed as admin on the teaching group or site. This would mean that the instructor cannot be set during the creation stage, and would need to be field in group>manage or Site>settings.

We could (a) limit who can select Instructors, (b) limit who can be selected as an Instructor, (c) introduce some some sort of verification step (an email to the designated instructor, with a link that must be clicked?),

Yes to (a) and (b) I would think we want to limit it to only group/site admins can be listed as instructor. I think an email would create a level of complexity that could be avoided if it is limited to admins.

Thanks, Ray! I've tested this and it looks great.

Hi Laurie,

Only group and site administrators will be populated in the Instructors field. So if you wish to add a new instructor, you will need to add that person as a group admin first or if your course is connected to a site, that person will need to be added as a site administrator.

To view other sites on cdev, you will need to modify your computer's HOSTS file to add the site you wish to view on cdev.

For example:

# cdev sites
146.96.128.252 commons.gc.cuny.edu
146.96.128.252 lauriehudson.commons.gc.cuny.edu
146.96.128.252 anothersite.commons.gc.cuny.edu

If you already added those entries to your HOSTS file, you might need to create a new site on cdev. Afterwards, you will need to add that site to your HOSTS file.

Ray's instructions on configuring your computer to view sites on cdev are correct.

Regarding your group video, the second group you tried uncovered a bug where a fatal PHP error could be triggered in groups that do not have the 'Teaching' purpose. I've fixed this bug.

Oh man, sorry for this silly question! I always forget that I need to add extra sites into the hosts file. Sorry about that.

But glad I could help surface that bug, though.

Thanks all!

Regarding your group video, the second group you tried uncovered a bug where a fatal PHP error could be triggered in groups that do not have the 'Teaching' purpose. I've fixed this bug.

Good catch, Boone! Thanks for fixing it. And thank you to Laurie for finding the bug.

Tested adding an insturctor on a cdev site this morning and it worked as intended.

Also worked in groups now that the bug is fixed. Thanks Boone and Ray!

Sorry if I'm late to this or missing something, but I'm having trouble finding the instructor field at all in the settings locations described for this connected group and site:

https://commons.gc.cuny.edu/groups/dec-new-group-colin
https://decnewsite.commons.gc.cuny.edu/

I have Scott added as an admin in both, and both are public (if that matters) and teaching purpose. Does it matter that they're connected? Like would changing the instructor in one place (if I could find it) change it in both?

Thanks, Colin. There was a bug that was preventing the 'Instructors' interface from being unhidden when 'Teaching' was selected as purpose. This should be fixed in https://github.com/cuny-academic-commons/cac/commit/88996f81e1572832bcf71ac3d1dc540958872992.

Great, I'm seeing the Instructor field in both places now. Minor thing, I'm pretty sure that for a Connected Group + Site, if I update Instructor from one to two people on the Site first, it doesn't display that in the item on the Courses directory. It still shows only the first person there externally, despite updating to two people on both the Site and Group (carried over automatically, which is cool) internal settings pages.

If I remove and then re-add the second person on the Group settings, then it shows both.

The issues with site-course syncing are related to the switch_to_blog() mechanism. This is also the cause of some of the inconsistencies in #14457. I'll outline the issue here for posterity before describing my solution.

When creating or updating a site with the Teaching purpose, we sync certain pieces of information to a cac_course post on the root blog, site #1. The underlying mechanism in the cac-courses blog, which is active and creates its post types only on site #1. Both site creation and site editing happen in a different site context (ie, not site #1), which is why we previously used switch_to_blog() to switch to the root blog before creating/updating the cac_course content. However, while a switch is necessary in both the creation and editing contexts, the details are different:
a. Site creation happens inside of a switch_to_blog() on the main site. WP switches to the new site, and then inside of that switch, we switch back to the root blog in order to create the cac_course object. For hypothetical site 456, the double-switch looks like this: 1 - [ 456 - 1 - 456 ] - 1.
b. Site editing happens on the individual site Dashboard, which means that the switch looks like this: 456 - 1 - 456.

These switches work in terms of saving to the wp_1_posts database table, but they don't work in two other key ways:
1. WP loads plugins based on the originally requested site. So cac-courses is only available natively in situation (a). I'd previously worked around this by manually loading the necessary cac-courses libraries and registering the post types, which kinda sorta worked but is very treacherous.
2. WP has some underlying cache issues that affect the taxonomy and postmeta systems inside of switch_to_blog(). The details are complicated and I didn't try to dig to the very bottom of them, but it appears that the get_metadata() cache (maybe around here? https://core.trac.wordpress.org/browser/tags/5.8.2/src/wp-includes/meta.php#L579 - could have something to do with the way the key is built) isn't responsive to blog-switching, with the result that postmeta calls return empty in an inappropriate way. This breaks certain syncing mechanisms in cac-courses, which translate postmeta into taxonomy terms, and is the direct cause of various bugs we've experienced here on the Commons.

So, briefly, switch_to_blog() is not viable for course-site syncing, and we need a new mechanism. I toyed with the idea of a REST API endpoint, but this is a major headache in a bunch of ways (calls must be made client side, causing concurrency/latency/race-condition issues; we have to account for CORS headers; there are security implications related to privileging, etc).

As such, I went with a cron-based routine. See https://github.com/cuny-academic-commons/cac/compare/5e181dc94be6934617e4c5993b0cbf7f8ef42901...16c03abd7fd343930a5173a5cf47bb6b852f03e6
1. When a site is created or updated, a cron job is scheduled on the main site
a. If the site has the Teaching purpose, run a 'sync' routine
b. If the site does not have the Teaching purpose, run a 'delete' routine
2. In the cron-job callbacks, no switch_to_blog() is necessary

This new routine means that, during testing, you may find that the very first pageload of the Courses directory after the change on the site Dashboard will not reflect the changes. It might take a few seconds to take effect.

As part of these changes, I needed to rework the way that the Instructors data is stored. When Ray built the feature in this ticket, he used blogmeta as a more-efficient mirror of the cac_course data, rather than as the canonical source of that data for the site. As such, he needed to pass the instructor data along to cac_create_course_object_from_site(). In an asynchronous cron job, this technique doesn't work very well. It also doesn't parallel the other course-related metadata, which is stored primarily with the site, and synced to the course only secondarily. So I switched the Instructors data model to mirror the other metadata types more closely. There's a small amount of magic in there so that if your Course doesn't have an instructor listed at all, it'll use the administrators from the site.

despite updating to two people on both the Site and Group (carried over automatically, which is cool) internal settings pages.

I don't believe that there is now, nor was there previously, a mechanism that syncs Site and Group instructors, or other sitemeta, even when the Site and Group are linked. If a Site and Group are linked to a Course, then information from the Site will generally take precedence (this is something of a change from previously, where Group information generally was preferred; the fact that Site-syncing happens in a delayed cron job means that it is the "winner"). But if you go and edit the Group afterward, I believe that the Group's metadata will overwrite what's in the Course. If we want forced syncing of course-related metadata between linked sites and groups, we should discuss it in a separate ticket.

Thanks for the rundown, Boone.

1. WP loads plugins based on the originally requested site. So cac-courses is only available natively in situation (a). I'd previously worked around this by manually loading the necessary cac-courses libraries and registering the post types, which kinda sorta worked but is very treacherous.

Yeah, when I initially worked on this feature, I tried to adhere to the existing way we were already doing things.

A prior workaround could have involved activating cac-courses network-wide and hiding the admin interfaces on secondary sites. See https://wordpress.stackexchange.com/questions/143437/querying-posts-by-taxonomy-from-alternate-network-site. But that doesn't solve the postmeta cache clash you mentioned.

In general, the cron approach sounds more sane. I see one issue with the cronjob, which I'll outline below.

So I switched the Instructors data model to mirror the other metadata types more closely. There's a small amount of magic in there so that if your Course doesn't have an instructor listed at all, it'll use the administrators from the site.

In this change: https://github.com/cuny-academic-commons/cac/compare/5e181dc94be6934617e4c5993b0cbf7f8ef42901...16c03abd7fd343930a5173a5cf47bb6b852f03e6#diff-e199d5f24d983e8d01b0190e1d59fc65c9cd3659c2841dd6664a47fdd4469f00R2934, this will undo the manual instructor curation from the sub-site's admin dashboard since the selected instructor IDs would be overriden back to the default administrators during the cron job sync.

Perhaps we should check if the instructor IDs are empty here before resetting them with the default?

Also, I have a question about this change: https://github.com/cuny-academic-commons/cac/compare/5e181dc94be6934617e4c5993b0cbf7f8ef42901...16c03abd7fd343930a5173a5cf47bb6b852f03e6#diff-e199d5f24d983e8d01b0190e1d59fc65c9cd3659c2841dd6664a47fdd4469f00R2386, the cac_get_site_instructors() function now uses cac_get_instructor_administrators() if the instructor IDs have never been mirrored to blogmeta before.

cac_get_instructor_administrators() is meant to return a list of available administrator IDs to be used for the Instructor field, which isn't the same as the canonical course instructor IDs data. This function actually pulls up all the admins of the site and the group (if the course is connected to a group). (Sorry for the poorly-named fucntion!)

Is there a scenario where a course isn't populated with an instructor? Is this primarily for older courses before the Instructor field was made available?

Thanks for having a close look at this, Ray!

In this change: https://github.com/cuny-academic-commons/cac/compare/5e181dc94be6934617e4c5993b0cbf7f8ef42901...16c03abd7fd343930a5173a5cf47bb6b852f03e6#diff-e199d5f24d983e8d01b0190e1d59fc65c9cd3659c2841dd6664a47fdd4469f00R2934, this will undo the manual instructor curation from the sub-site's admin dashboard since the selected instructor IDs would be overriden back to the default administrators during the cron job sync.

Perhaps we should check if the instructor IDs are empty here before resetting them with the default?

Can you talk me through this? I didn't see any such resetting in my tests, but it's possible that I'm testing in the wrong way. Here's the way it's intended to work:

1. You're looking at options-general.php on the subsite, and you change instructor ID settings
2. On submit (to subsite.commons/options.php), the changes are saved using cac_site_metadata_save(). As part of that routine, the selected instructors are saved to blogmeta: https://github.com/cuny-academic-commons/cac/blob/0b7f7388a44065d57ac1fbe2fb8c669dbde2593e/wp-content/mu-plugins/cac-functions.php#L2680, https://github.com/cuny-academic-commons/cac/blob/0b7f7388a44065d57ac1fbe2fb8c669dbde2593e/wp-content/mu-plugins/cac-functions.php#L2453. At the end of the save routine, the cron job is scheduled. https://github.com/cuny-academic-commons/cac/blob/0b7f7388a44065d57ac1fbe2fb8c669dbde2593e/wp-content/mu-plugins/cac-functions.php#L2713
3. When the cron job is run, the instructors are pulled from blogmeta https://github.com/cuny-academic-commons/cac/blob/0b7f7388a44065d57ac1fbe2fb8c669dbde2593e/wp-content/mu-plugins/cac-functions.php#L2934 and stored in postmeta for the cac_course object.

The disconnect between you and me might be that I made the change at https://github.com/cuny-academic-commons/cac/compare/5e181dc94be6934617e4c5993b0cbf7f8ef42901...16c03abd7fd343930a5173a5cf47bb6b852f03e6#diff-e199d5f24d983e8d01b0190e1d59fc65c9cd3659c2841dd6664a47fdd4469f00R2381 and https://github.com/cuny-academic-commons/cac/compare/5e181dc94be6934617e4c5993b0cbf7f8ef42901...16c03abd7fd343930a5173a5cf47bb6b852f03e6#diff-e199d5f24d983e8d01b0190e1d59fc65c9cd3659c2841dd6664a47fdd4469f00R2449 - namely, instructor IDs are stored in blogmeta immediately on options-general.php save, which is a change from how you previously had it.

cac_get_instructor_administrators() is meant to return a list of available administrator IDs to be used for the Instructor field, which isn't the same as the canonical course instructor IDs data. This function actually pulls up all the admins of the site and the group (if the course is connected to a group). (Sorry for the poorly-named fucntion!)

Function name seems OK to me - I can't think of a better one that isn't very long :) I guess I put this block in here just so that the getter would never return an empty array. My thought was that, in this situation, it seems a decent approximation to add all of the site's admins as instructors. But this could be a bad assumption, and it's probably not super important, so I'm happy to pull it if you think it's better to pull it. This does remind me, though, that we will probably need a script to populate instructor IDs for existing courses, right? I guess that, for consistency with the current approach, this script would designate the creator_id for groups; for sites, maybe the owner of admin_email, or the first user in the list of Administrators (I don't think there's a way to determine who created a site after the fact?). Does this seem right? I can whip something up unless you beat me to it.

Can you talk me through this? I didn't see any such resetting in my tests, but it's possible that I'm testing in the wrong way.

I didn't test at all :) I incorrectly interpreted the logic at this line: https://github.com/cuny-academic-commons/cac/blob/0b7f7388a44065d57ac1fbe2fb8c669dbde2593e/wp-content/mu-plugins/cac-functions.php#L2934. (I blame my lack of sleep!) You explain the correct logic here:

namely, instructor IDs are stored in blogmeta immediately on options-general.php save, which is a change from how you previously had it.

My thought was that, in this situation, it seems a decent approximation to add all of the site's admins as instructors. But this could be a bad assumption, and it's probably not super important, so I'm happy to pull it if you think it's better to pull it. This does remind me, though, that we will probably need a script to populate instructor IDs for existing courses, right? I guess that, for consistency with the current approach, this script would designate the creator_id for groups; for sites, maybe the owner of admin_email, or the first user in the list of Administrators (I don't think there's a way to determine who created a site after the fact?). Does this seem right? I can whip something up unless you beat me to it.

If there are no instructors already saved into the course object, I'm not sure if it is better to use all available course admins from the group/site as the course instructor(s) or have nothing displayed at all.

The initial ticket description actually describes a use-case where we might not want to use the creator_id for groups or the owner of admin_email for sites as the initial course instructors. Would it be better to leave the instructor field blank and wait for the correct instructor(s) to be added in later? I'm not sure how many courses are actually created from a user that is not the course instructor. If the percentage is relatively small, then ignore everything I've written and let's go ahead with populating existing courses with no instructors :)

I think we might want to get some feedback from Laurie here.

The initial ticket description actually describes a use-case where we might not want to use the creator_id for groups or the owner of admin_email for sites as the initial course instructors. Would it be better to leave the instructor field blank and wait for the correct instructor(s) to be added in later? I'm not sure how many courses are actually created from a user that is not the course instructor. If the percentage is relatively small, then ignore everything I've written and let's go ahead with populating existing courses with no instructors :)

You make a good point. Maybe we should do nothing. Old courses are not likely to be referenced or used very often anyway.

Laurie, if you could weigh in, that would be helpful. Should existing courses have the creator filled in as the instructor, or should we leave them blank?

Hi Ray and Boone,

Thanks for this feature!

I think it would be best to populate the instructor field with the site/group creator.

The most important aspect of this feature is that the instructor field can now be edited by the user. I don't think it would be good to leave blank because this requires the admin to add themselves as instructor, which results in a more substantial change in the course creation process across the commons.

The most common use case here is Brooklyn college OER team creating course sites for faculty. Previously there was no way to change the instructor which caused issues when students were looking for their courses in the directory. But now this feature allows them to change the instructor listed in the course directory, effectively resolving this issue.

Thanks, Laurie!

New Groups and Sites will have the creator as the sole Instructor.

The immediate question here is what we do with existing (ie old) content.

Ah sorry.

If we leave field blank on old content will this result in the instuctor field presenting as blank in the course directory? If so, I think we should populate the new field with creator/original admin of the site. BK college folks can go change it as desired.

If leaving new instructor field blank on old content will not result in a blank field in the courses directory (aka the creator will still be listed as instructor in the directory), I think it would be fine to not populate this new field.

That seems to make the most sense to me? I just don't want all old courses in the directory to all of sudden have blank fields for instructor