Moodle
  1. Moodle
  2. MDL-36958

use FORUM_CHOOSESUBSCRIBE instead of 0, etc. in mod/forum/subscribe.php

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.1.6, 2.1.9, 2.2.3, 2.2.6, 2.3.3
    • Fix Version/s: 2.3.4, 2.4.1
    • Component/s: Forum
    • Labels:
    • Testing Instructions:
      Hide

      It probably don't need any testing instructions.
      Some instructions to check existing behaviour

      1. Log in as admin
      2. Select forum in a course
      3. Change forum subscription and make sure it is retained without any error/warning. (Settings -> Forum administration -> Subscription mode -> Optional/forced/auto/disable )
      Show
      It probably don't need any testing instructions. Some instructions to check existing behaviour Log in as admin Select forum in a course Change forum subscription and make sure it is retained without any error/warning. (Settings -> Forum administration -> Subscription mode -> Optional/forced/auto/disable )
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Pull 2.4 Branch:
      wip-mdl-36958-m24
    • Pull Master Branch:
      wip-mdl-36958
    • Rank:
      46498

      Description

      This four-line patch changes hard-coded numbers into the define()d words that should be used instead.

      Patch is here: https://github.com/moquist/moodle/commit/8cef53e89b6afa8aa2fc854177006a4e1cc1124b

        Activity

        Hide
        Michael de Raadt added a comment -

        Thanks for reporting that and providing a patch.

        Show
        Michael de Raadt added a comment - Thanks for reporting that and providing a patch.
        Hide
        Rajesh Taneja added a comment -

        Thanks for providing patch Matt,

        Patch looks Good. I have created local branches with your commit.
        Pushing it for peer-review.

        Show
        Rajesh Taneja added a comment - Thanks for providing patch Matt, Patch looks Good. I have created local branches with your commit. Pushing it for peer-review.
        Hide
        Mark Nelson added a comment -

        Hi Raj, looks good. However you may want to rebase for 2.4/master or change the diff URL to point directly to the commit rather than diff.

        eg.
        2.4. https://github.com/rajeshtaneja/moodle/commit/92e16636d71b5e9e907884df3340e3ab50c88da6
        2.5. https://github.com/rajeshtaneja/moodle/commit/3cfe11b99ca433e07dc34458eeedbc0c63dce6e5

        Also the commit message contains '()' which looks odd.

        Thanks!

        Show
        Mark Nelson added a comment - Hi Raj, looks good. However you may want to rebase for 2.4/master or change the diff URL to point directly to the commit rather than diff. eg. 2.4. https://github.com/rajeshtaneja/moodle/commit/92e16636d71b5e9e907884df3340e3ab50c88da6 2.5. https://github.com/rajeshtaneja/moodle/commit/3cfe11b99ca433e07dc34458eeedbc0c63dce6e5 Also the commit message contains '()' which looks odd. Thanks!
        Hide
        Rajesh Taneja added a comment -

        Thanks Mark,
        Rebased branches and updated commit message

        Show
        Rajesh Taneja added a comment - Thanks Mark, Rebased branches and updated commit message
        Hide
        Sam Hemelryk added a comment -

        Thanks Raj, this has been integrated.

        Show
        Sam Hemelryk added a comment - Thanks Raj, this has been integrated.
        Hide
        Rossiani Wijaya added a comment -

        This is working as expected.

        Test passed.

        Show
        Rossiani Wijaya added a comment - This is working as expected. Test passed.
        Hide
        Eloy Lafuente (stronk7) added a comment -

        And your fantastic code has met core, hope they become good friends for a long period.

        Closing, thanks!

        Show
        Eloy Lafuente (stronk7) added a comment - And your fantastic code has met core, hope they become good friends for a long period. Closing, thanks!

          People

          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: