Copy set level proctors when adding a course and copying sets. - #2672
Merged
somiaj merged 1 commit intoFeb 18, 2025
Merged
Conversation
drgrice1
force-pushed
the
copy-set-level-proctors-with-sets
branch
2 times, most recently
from
February 14, 2025 02:42
e7713e8 to
d9e3038
Compare
If sets are copied then everything for that set should be copied. These set level proctor users belong to the set. This fixes issue openwebwork#2652. Also add a missing `maketext` call in the related template. Also fix some database list/get usage that I missed when these copy options were implemented. Never list all database records if you are going to subsequently get all records anyway. Listing records is the equivalent of calling `SELECT id from table` and getting all records is the equivalent of calling `SELECT * from table`. Why would you inneficiently first list to get id's and then select everything separately? Note that you can get always get all records by using the webwork2 db `Where` methods with no where clause. Note that the copying of sets that is done here is still highly inneficient. The current approach gets all sets, and then loops through those sets, then gets the problems for that set and loops through those adding one at a time, then gets set locations and loops through adding one of those at a time, and now adds the set level proctor (of which there can be only one). Instead since all sets are being copyied all sets could be fetched and added at once, then all problems for all sets fetched and added at once, then all set level proctors for all sets fetched and added at once. However, adding all at once like that takes for effort because there aren't convenience database methods for doing that. This is now done when assigning sets to multiple users (I helped @taniwallach implement that) and the same could be done here. Note that for a course with a large number of sets and a lot of problems the current approach is quite slow.
drgrice1
force-pushed
the
copy-set-level-proctors-with-sets
branch
from
February 14, 2025 02:43
d9e3038 to
fc7f210
Compare
Alex-Jordan
approved these changes
Feb 18, 2025
somiaj
approved these changes
Feb 18, 2025
somiaj
left a comment
Contributor
There was a problem hiding this comment.
Tested the issue is fixed and set proctors are copied.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
If sets are copied then everything for that set should be copied. These set level proctor users belong to the set.
This fixes issue #2652.
Also add a missing
maketextcall in the related template.Also fix some database list/get usage that I missed when these copy options were implemented. Never list all database records if you are going to subsequently get all records anyway. Listing records is the equivalent of calling
SELECT id from tableand getting all records is the equivalent of callingSELECT * from table. Why would you inneficiently first list to get id's and then select everything separately? Note that you can get always get all records by using the webwork2 dbWheremethods with no where clause.Note that the copying of sets that is done here is still highly inneficient. The current approach gets all sets, and then loops through those sets, then gets the problems for that set and loops through those adding one at a time, then gets set locations and loops through adding one of those at a time, and now adds the set level proctor (of which there can be only one). Instead since all sets are being copyied all sets could be fetched and added at once, then all problems for all sets fetched and added at once, then all set level proctors for all sets fetched and added at once. However, adding all at once like that takes for effort because there aren't convenience database methods for doing that. This is now done when assigning sets to multiple users (I helped @taniwallach implement that) and the same could be done here. Note that for a course with a large number of sets and a lot of problems the current approach is quite slow.