Description

Issue: Enrollment duration is not applied when manually enrolling a user if enrollment option is not expanded.

Steps to Replicate:
-Go into a course
-Course Admin>Enrollment Methods>Manual Enrollments>Settings
-Set and Enable a Default Enrollment Duration
-Go to Enrolled users
-Enroll Users
-Just enrol a user
Result: No end date is applied to the user enrollment

To get the user enrollment duration applied:
-Go to Enrolled Users
-Click the Enrollment Options arrow to drop that menu down
-Make no changes to it (you will see the default is populated)
-Enrol User
-Finish Enrolling users
-End Date is populated

Andrew Nicols
added a comment - 12/Dec/13 3:03 AM Hi Jerome,
[Y] Syntax
[Y] Whitespace
[Y] Output
[-] Language
[-] Databases
[Y] Testing (instructions and automated tests)
[Y] Security
[N] Documentation
[N] Git
[-] Third party code
[N] Sanity check
Documentation:
There isn't any to explain this change - can you add into the issue why we're doing this so that in 6 months time when I come to blame you I understand why
Git:
The commit message could do with a few tweaks:
We code do Australian English, where Enrolment has a single l;
The commit message summary is a touch long
If you could explain the rationale for the change in the commit message that would be grand
Sanity check:
I must admit, possibly because of the lack of comments and explanation, I have no idea what this is doing so I'm unable to sanity check this.

Basically before the fix, the startdates and duration fields were integrated to HTML code when the user clicked on the collapsible region to see them. It was wrong because the value were not sent by the form if ever the user didn't expand their region - the Enrolment options.

Jérôme Mouneyrac
added a comment - 12/Dec/13 7:40 AM - edited Basically before the fix, the startdates and duration fields were integrated to HTML code when the user clicked on the collapsible region to see them. It was wrong because the value were not sent by the form if ever the user didn't expand their region - the Enrolment options.

I talked with Andrew, I reworded the commit and added some explanation in the commit message lines below the first line (I should use them more often). Thanks for the peer-review Andrew. Sending to integration.

Jérôme Mouneyrac
added a comment - 12/Dec/13 7:56 AM - edited I talked with Andrew, I reworded the commit and added some explanation in the commit message lines below the first line (I should use them more often). Thanks for the peer-review Andrew. Sending to integration.

Just noting there were a couple of things that struck me about this code.

The php processing the AJAX request is not using the defaults either, as they are always applied that is not really a bug [right now].

As we always populate the duration there is no longer a need for the default 0 to be added during the construction of the select (in the init method above your changes). Again it has no impact on functionality so not an issue.

Both points would be nice to clean up, however as your changes work perfectly getting it in now is better than delaying it.

Sam Hemelryk
added a comment - 15/Dec/13 10:14 PM Thanks Jerome - this has been integrated now.
Just noting there were a couple of things that struck me about this code.
The php processing the AJAX request is not using the defaults either, as they are always applied that is not really a bug [right now] .
As we always populate the duration there is no longer a need for the default 0 to be added during the construction of the select (in the init method above your changes). Again it has no impact on functionality so not an issue.
Both points would be nice to clean up, however as your changes work perfectly getting it in now is better than delaying it.
Many thanks
Sam

Damyon Wiese
added a comment - 20/Dec/13 7:31 AM Twas the week before Christmas,
And all though HQ
Devs were scrambling to finish peer review.
They sent all their issues,
and rushed out the door -
"To the beach!" someone heard them roar!
This issue has been released upstream. Thanks!