In InstCourseOfferingExercise, there are two closely-named (apparently closely related) methods: find_or_create and find_or_create_resource. Both methods take parameters and look up an existing object, creating a new one if no object is found.
The find_or_create method also updates the retrieved object, if an existing object is found, which makes sense. However, the find_or_create_resource method in contrast never updates the specified object if settings have changed, and only records settings on initial creation of the object. This leads to situations where the settings may have changed, but whether or not those new settings values are stored depends on which of the two methods are used.
This pull request fixes find_or_create_resource so that the settings updates are applied the same way between newly created or existing retrieved objects.
Actually, I'm unsure where these two methods are used, since the statements that extract the relevant settings for storage in the instance are actually different between the two methods. It would be better to refactor the code so there's only one method here, and it used the same logic consistently. Having two closely related, highly inter-duplicated but not identical methods is a maintenance risk. Also, this pull request is a partial response to fixing a bug arising from this duplication. But I'd rather have some additional eyes on the change to make sure it won't have unintended consequences before making a change that is larger.
In InstCourseOfferingExercise, there are two closely-named (apparently closely related) methods: find_or_create and find_or_create_resource. Both methods take parameters and look up an existing object, creating a new one if no object is found.
The find_or_create method also updates the retrieved object, if an existing object is found, which makes sense. However, the find_or_create_resource method in contrast never updates the specified object if settings have changed, and only records settings on initial creation of the object. This leads to situations where the settings may have changed, but whether or not those new settings values are stored depends on which of the two methods are used.
This pull request fixes find_or_create_resource so that the settings updates are applied the same way between newly created or existing retrieved objects.
Actually, I'm unsure where these two methods are used, since the statements that extract the relevant settings for storage in the instance are actually different between the two methods. It would be better to refactor the code so there's only one method here, and it used the same logic consistently. Having two closely related, highly inter-duplicated but not identical methods is a maintenance risk. Also, this pull request is a partial response to fixing a bug arising from this duplication. But I'd rather have some additional eyes on the change to make sure it won't have unintended consequences before making a change that is larger.