Project

General

Profile

Support #13312

Site Header Issue Using Teaching Template

Added by scott voth about 1 year ago. Updated 5 months ago.

Status:
Resolved
Priority name:
Normal
Assignee:
Category name:
WordPress (Permissions)
Target version:
Start date:
2020-09-11
Due date:
% Done:

0%

Estimated time:

Description

Steve Brier reported that he was unable to change the header image for his teaching site - https://f20urbedhistory.commons.gc.cuny.edu/. Matt stepped in and was able to change it with his super admin permissions. Marilyn asked him to use the Customizr route, but he responded "I assumed that was how to do it, tried it, and was told I didn't have sufficient permission as the site administrator to have access to that feature." I know that other professors have been able to change the header and I just created a site with teaching template and had no trouble changing the header, so this problem seems to be an isolated case.


Related issues

Related to CUNY Academic Commons - Bug #13331: Combine Site Template and Clone operationsNew2020-09-15

History

#1 Updated by Boone Gorges about 1 year ago

  • Status changed from New to Reporter Feedback

When I went to test this by visiting https://f20urbedhistory.commons.gc.cuny.edu/wp-admin/user-new.php as a super admin (to add a non-super-admin user to the blog for testing) I noticed that the 'Role' dropdown was empty. This strongly indicated that the wp_[blogid]_user_roles option in the database was corrupted/missing, which would also explain why a regular site admin was not able to perform the action in question. I looked in the database and my suspicion was confirmed:

mysql> select distinct option_name from wp_13091_options where option_name like '%role%';
+--------------------+
| option_name        |
+--------------------+
| default_role       |
| wp_3859_user_roles |
| wp_8358_user_roles |
+--------------------+
3 rows in set (0.00 sec)

I changed the 8358 option to have the proper blog ID, and the Role field is now present. I would wager that Steve will now be able to perform all Administrator functions.

There is a broader question of what happened here. 3859 is the teaching-template site. 8358 is f19urbedhistory, a previous incarnation of this course. Presumably, Steve used our clone feature to create this new course, with f19urbedhistory as the source. Scott, can you confirm?

I see two possible sources for the bug. One is that there was a problem with 8358 specifically, which made the clone process break when 13091 was cloned. See #13134 for some possible backstory. If 8358 was corrupt - say, by having more than one wp_*_user_roles entry - then perhaps it would trigger the kind of behavior we see here. The other possibility is that there's a broader bug in the cloning process that causes user roles not to be synced properly in all cases. Jeremy, could you please have a look at the relevant portion of the codebase and see whether you can hypothesize on either of these? What happens if you try to clone a site with more than one user_roles entry? Are there codepaths that lead to the option not being copied properly, or not having its option name changed?

#2 Updated by scott voth about 1 year ago

Here is Steve's reply:
"Yes, I used the clone feature to create the f20 site from the f19 site and this is the site that Boone worked out the fix so he could clone my Library from F19 to the F20 new site. I’m wondering if this has to do with using the Teaching template to make the original site but that’s just a guess. Matt fixed my graphic issue as a super admin, which was the original reason I tried to gain access to the Appearance tab on the Dashboard."

#3 Updated by Boone Gorges about 1 year ago

Thanks for confirming, Marilyn.

#4 Updated by Jeremy Felt about 1 year ago

Working through this locally:

  • I created a new site with the teaching template. The teaching template is assigned to site ID 216 locally.
  • The new site (ID 291) has entries for `wp_1_user_roles`, `wp_216_user_roles`, and `wp_291_user_roles`.
  • I cloned that new site (ID 292) and it has entries for `wp_1_user_roles`, `wp_216_user_roles`, `wp_291_user_roles`, and `wp_292_user_roles`.
  • The roles dropdown on the newly cloned site is populated as expected.

This makes sense because the clone process (1) creates a new site, (2) replaces the new site's options with the previous site's options, but (3) skips several options, including the new `wp_#_user_roles`. The `wp_#_user_roles` option is unique in its... uniqueness, so multiple unused entries end up in storage.

Initial thoughts:
  • We can adjust the site cloning routine so old `wp_#_user_roles` data is not stored.
  • We can also adjust the site template cloning routine so that old `wp_#_user_roles` data is not stored.
  • Maybe it's worth thinking about combining these two clone routines? (This may have been less obvious to me before)

This does not address the actual issue, in which a new roles entry was not created for the cloned site. I'm going to keep poking around at site creation logic to see how it may have been skipped.

#5 Updated by Boone Gorges about 1 year ago

  • Related to Bug #13331: Combine Site Template and Clone operations added

#6 Updated by Boone Gorges about 1 year ago

  • Target version set to 1.17.4

Awesome, thanks for having a first look and explaining it so clearly.

We can adjust the site cloning routine so old `wp_#_user_roles` data is not stored.
We can also adjust the site template cloning routine so that old `wp_#_user_roles` data is not stored.

Definite yes to both of these. Let's do this under the current ticket.

Maybe it's worth thinking about combining these two clone routines? (This may have been less obvious to me before)

Yes, this is a good idea, though it might be complex. I've opened #13331 to explore.

This does not address the actual issue, in which a new roles entry was not created for the cloned site. I'm going to keep poking around at site creation logic to see how it may have been skipped.

Agreed. Please keep this ticket updated as you poke.

#7 Updated by Boone Gorges about 1 year ago

To help us with our debugging, #11482 is the ticket where we changed template content authorship to commonsadmin.

#8 Updated by Boone Gorges 12 months ago

  • Assignee changed from Boone Gorges to Jeremy Felt
  • Target version changed from 1.17.4 to 1.17.5

#9 Updated by Boone Gorges 11 months ago

  • Target version changed from 1.17.5 to 1.17.6

#10 Updated by Boone Gorges 11 months ago

  • Target version changed from 1.17.6 to 1.17.7

#11 Updated by Boone Gorges 10 months ago

  • Target version changed from 1.17.7 to 1.17.8

#12 Updated by Boone Gorges 10 months ago

  • Target version changed from 1.17.8 to 1.18.1

#13 Updated by Boone Gorges 9 months ago

  • Target version changed from 1.18.1 to 1.18.2

#14 Updated by Boone Gorges 8 months ago

  • Target version changed from 1.18.2 to 1.18.3

#15 Updated by Boone Gorges 8 months ago

  • Target version changed from 1.18.3 to 1.18.4

#16 Updated by Boone Gorges 7 months ago

  • Target version changed from 1.18.4 to 1.18.5

#17 Updated by Jeremy Felt 7 months ago

I have PRs up on CAC and cac-site-templates that stop the cloning processes from storing original user role data under its old option key on the new site.

This did make me wonder if it would make sense to instead transform the data so that the source site user roles were actually stored under the new site's user role option key. I don't recall if we talked through that separately or if I just used the existing decision from the template cloning process.

Okay, those PRs help keep the database cleaner.

This does not address the actual issue, in which a new roles entry was not created for the cloned site. I'm going to keep poking around at site creation logic to see how it may have been skipped.

The source of the issue that started the ticket was resolved in the fix for #13134. Before that fix, the entire source site's option table was cloned to the new site. This would preserve the template site's user roles key and the source site's user roles key, but would not generate a new user roles key for the new site.

This could mean there are a handful of sites out there that were cloned at the same time and had the same issue, but I would think the issue reported in #13134 would have come up for each of them.

#18 Updated by Boone Gorges 7 months ago

  • Status changed from Reporter Feedback to Assigned

This did make me wonder if it would make sense to instead transform the data so that the source site user roles were actually stored under the new site's user role option key. I don't recall if we talked through that separately or if I just used the existing decision from the template cloning process.

I guess this would cover the situation where a plugin on the source site has modified the default roles, but where the roles were not properly reinitialized after the site is cloned. Can you think of any risks if we copy the roles instead of letting them be regenerated? If not, I think we should go with your suggestion instead of simply skipping them.

#19 Updated by Boone Gorges 7 months ago

  • Target version changed from 1.18.5 to 1.18.6

#20 Updated by Boone Gorges 6 months ago

  • Target version changed from 1.18.6 to 1.18.7

#21 Updated by Boone Gorges 6 months ago

  • Status changed from Assigned to Resolved

#22 Updated by Jeremy Felt 5 months ago

I (finally) wrapped up both of both of the PRs on this.

In CAC: https://github.com/cuny-academic-commons/cac/pull/10
In cac-site-templates: https://github.com/cuny-academic-commons/cac-site-templates/pull/3

Both prevent the source and destination options from being cloned initially AND then override the new user role options with the value from the source site.

The result: only one roles option is stored and it contains the roles as they existed on the source site.

#23 Updated by Boone Gorges 5 months ago

  • Status changed from Resolved to Staged for Production Release
  • Target version changed from 1.18.7 to 1.18.8

Thank you sir!

#24 Updated by Boone Gorges 5 months ago

  • Status changed from Staged for Production Release to Resolved

Also available in: Atom PDF