Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New course instance copies the SIS id from the old instance if no new SIS id is set #1177

Closed
markkuriekkinen opened this issue May 3, 2023 · 3 comments
Assignees
Labels
area: enrollment Related to course enrollments effort: hours Estimated to take less than one day, from the creation of a new branch to the merging experience: beginner required knowledge estimate requester: team The issue is raised by a person inside the A+ developer team type: bug This is a bug
Milestone

Comments

@markkuriekkinen
Copy link
Contributor

When a teacher creates a new course instance under the Edit course - Instances page, it is possible that the new course instance copies the SIS id and enroll settings from the old course instance. This means that the students registered to the old course in SISU are also added to the new A+ course instance, which is absolutely not wanted. The old SIS id is copied to the new course if the teacher does not select anything in the SIS fields on the CloneInstanceForm. This is actually common since the SIS course ids are usually not yet available, e.g., in May for the coming autumn courses.

The new A+ course instance should have an empty SIS id after creation if the teacher has not selected any new SIS id in the CloneInstanceForm.

Currently, the code sets a new SIS id only when the teacher has selected something in the form:

if siskey and siskey != 'none':
set_sis(instance, siskey, sisenroll)

@markkuriekkinen markkuriekkinen added type: bug This is a bug effort: hours Estimated to take less than one day, from the creation of a new branch to the merging experience: beginner required knowledge estimate requester: team The issue is raised by a person inside the A+ developer team area: enrollment Related to course enrollments labels May 3, 2023
@markkuriekkinen markkuriekkinen added this to the v1.19 milestone May 3, 2023
@markkuriekkinen markkuriekkinen moved this to Todo in A+ sprints May 3, 2023
@PasiSa PasiSa self-assigned this May 9, 2023
@ihalaij1
Copy link
Contributor

ihalaij1 commented May 9, 2023

I'm pretty sure the fix is to set instance.sis_id = '' and instance.sis_enroll = False in clone.py before instance.save() is called.

@PasiSa
Copy link
Contributor

PasiSa commented May 11, 2023

I'm pretty sure the fix is to set instance.sis_id = '' and instance.sis_enroll = False in clone.py before instance.save() is called.

Yep, correct. I can push the fix shortly.

@PasiSa PasiSa closed this as completed in 8ab057c May 11, 2023
@github-project-automation github-project-automation bot moved this from Todo to Done in A+ sprints May 11, 2023
@markkuriekkinen
Copy link
Contributor Author

@PasiSa
8ab057c

    instance.sis_id =''
    instance.sis_enroll = False

Unfortunate whitespace mistake: missing whitespace after =
sis_id =''

markkuriekkinen added a commit to markkuriekkinen/a-plus that referenced this issue May 29, 2023
Commit 8ab057c did not use whitespace
correctly aroung the assignment operator.

apluslms#1177 (comment)
markkuriekkinen pushed a commit to markkuriekkinen/a-plus that referenced this issue Jun 2, 2023
markkuriekkinen added a commit to markkuriekkinen/a-plus that referenced this issue Jun 2, 2023
Commit 8ab057c did not use whitespace
correctly aroung the assignment operator.

apluslms#1177 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: enrollment Related to course enrollments effort: hours Estimated to take less than one day, from the creation of a new branch to the merging experience: beginner required knowledge estimate requester: team The issue is raised by a person inside the A+ developer team type: bug This is a bug
Projects
Status: Done
Development

No branches or pull requests

3 participants