Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-42085

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

    Details

    • Story Points (Obsolete):
      5
    • Sprint:
      FRONTEND Sprint 7

      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

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            skodak Petr Skoda added a comment -

            Thanks for the report.

            Show
            skodak Petr Skoda added a comment - Thanks for the report.
            Hide
            salvetore Michael de Raadt added a comment -

            This issue has been duplicated. I'm giving it a nudge.

            Show
            salvetore Michael de Raadt added a comment - This issue has been duplicated. I'm giving it a nudge.
            Hide
            heather.williams Heather Williams added a comment -

            I can replicate this behavior in 2.5.2 as well.

            Show
            heather.williams Heather Williams added a comment - I can replicate this behavior in 2.5.2 as well.
            Hide
            jerome Jérôme Mouneyrac added a comment -

            Thanks for the great step Kevin. Sent to peer-review.

            Show
            jerome Jérôme Mouneyrac added a comment - Thanks for the great step Kevin. Sent to peer-review.
            Hide
            dobedobedoh Andrew Nicols added a comment -

            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.

            Show
            dobedobedoh Andrew Nicols added a comment - 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.
            Hide
            jerome Jérôme Mouneyrac added a comment - - 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.

            Show
            jerome Jérôme Mouneyrac added a comment - - 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.
            Hide
            jerome Jérôme Mouneyrac added a comment - - 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.

            Show
            jerome Jérôme Mouneyrac added a comment - - 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.
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Thanks Jerome - this has been integrated now.

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

            1. 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].
            2. 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

            Show
            samhemelryk Sam Hemelryk added a comment - 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
            Hide
            salvetore Michael de Raadt added a comment -

            Test result: Success!

            Tested in 2.4, 2.5, 2.6 and master.

            Show
            salvetore Michael de Raadt added a comment - Test result: Success! Tested in 2.4, 2.5, 2.6 and master.
            Hide
            damyon Damyon Wiese added a comment -

            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!

            Show
            damyon Damyon Wiese added a comment - 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!

              People

              • Votes:
                2 Vote for this issue
                Watchers:
                8 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  13/Jan/14

                  Agile