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

Process for enabling statistics should be made clearer

    Details

    • Testing Instructions:
      Hide
      1. Go to Settings > Site admin > advanced features
      2. Make sure statistics (enablestats) is disable.
      3. Go to Settings > Site admin > Reports, make sure 'Course overview' is not listed.
      4. Access the report course overview page (eg: http://mymoodletestsite/report/courseoverview/index.php). Make sure the page redirect to search result page that contents the setting for enabling statistics.
      5. Login as Manager, make sure 'course overview' is not listed (Settings > Site admin > Reports).
      Show
      Go to Settings > Site admin > advanced features Make sure statistics (enablestats) is disable. Go to Settings > Site admin > Reports, make sure 'Course overview' is not listed. Access the report course overview page (eg: http://mymoodletestsite/report/courseoverview/index.php ). Make sure the page redirect to search result page that contents the setting for enabling statistics. Login as Manager, make sure 'course overview' is not listed (Settings > Site admin > Reports).
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Pull from Repository:
    • Pull Master Branch:

      Description

      In order to enable statistics, you have to turn on the Advanced features setting (enablestats). However, when Statistics not enabled, the Statistics settings page still appears at Settings > Site admin > Server > Statistics and the Course overview report appears in Settings > Site admin > Reports > Course overview.

      We should consider hiding these two pages while Statistics is disabled.

      Alternately we should consider making the following changes.

      1. When you visit the Course overview report, while statistics is disabled, you are redirected to the Statistics settings page, you should instead be redirected to the Advanced features page, perhaps through a search for "enablestats".
      2. When you visit the Statistics settings page, while statistics is disabled, you should see a notice that Statistics are disabled and provided with a link to the Advanced features page, perhaps through a search for "enablestats".

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            rwijaya Rossiani Wijaya added a comment -

            Created patch to fix the link for redirect.

            In order to keep the setting page consistent with the rest, I didn't add any notification to the statistics setting page.

            I could add it easily if it is really needed/require.

            Show
            rwijaya Rossiani Wijaya added a comment - Created patch to fix the link for redirect. In order to keep the setting page consistent with the rest, I didn't add any notification to the statistics setting page. I could add it easily if it is really needed/require.
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            Thanks Rossie,

            Patch looks spot-on, pushing for integration review.
            [y] Syntax
            [y] Output
            [y] Whitespace
            [-] Language
            [-] Databases
            [y] Testing
            [-] Security
            [-] Documentation
            [y] Git
            [y] Sanity check

            Show
            rajeshtaneja Rajesh Taneja added a comment - Thanks Rossie, Patch looks spot-on, pushing for integration review. [y] Syntax [y] Output [y] Whitespace [-] Language [-] Databases [y] Testing [-] Security [-] Documentation [y] Git [y] Sanity check
            Hide
            damyon Damyon Wiese added a comment -

            The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

            Cheers!

            Show
            damyon Damyon Wiese added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. Cheers!
            Hide
            poltawski Dan Poltawski added a comment -

            Hi Rosie,

            The change is fine, but it doesn't improve the situation that much as we're just again presented with a long list of options. Therefore i'm reopening.

            Why not instead redirect to "admin/search.php?query=enablestats" as Michael suggested?

            thanks
            Dan

            Show
            poltawski Dan Poltawski added a comment - Hi Rosie, The change is fine, but it doesn't improve the situation that much as we're just again presented with a long list of options. Therefore i'm reopening. Why not instead redirect to "admin/search.php?query=enablestats" as Michael suggested? thanks Dan
            Hide
            rwijaya Rossiani Wijaya added a comment -

            Hi Dan,

            Thanks for the feedback.

            I updated the patch to re-direct to admin/search.php?query=enablestats page.

            Re-submitting for integration reivew.

            Show
            rwijaya Rossiani Wijaya added a comment - Hi Dan, Thanks for the feedback. I updated the patch to re-direct to admin/search.php?query=enablestats page. Re-submitting for integration reivew.
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            Hello Dan and Rossie,

            I am not sure if this will work.
            If you are logged in as manager, you can't configure admin settings but can see course overview report.
            So redirecting to a page where user doesn't have permission is probably not helpful.

            We might have to look at this with different roles and there capabilities, before redirecting.

            Show
            rajeshtaneja Rajesh Taneja added a comment - Hello Dan and Rossie, I am not sure if this will work. If you are logged in as manager, you can't configure admin settings but can see course overview report. So redirecting to a page where user doesn't have permission is probably not helpful. We might have to look at this with different roles and there capabilities, before redirecting.
            Hide
            poltawski Dan Poltawski added a comment -

            Good catch, Raj

            Show
            poltawski Dan Poltawski added a comment - Good catch, Raj
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            Thanks to you Dan, for making me think about roles.
            Few recent issues have made me think how different roles behave and have been facing issues.

            Show
            rajeshtaneja Rajesh Taneja added a comment - Thanks to you Dan, for making me think about roles. Few recent issues have made me think how different roles behave and have been facing issues.
            Hide
            rwijaya Rossiani Wijaya added a comment -

            Updating the patch to hide the course overview link when stats is disabled.

            sending for peer review.

            Show
            rwijaya Rossiani Wijaya added a comment - Updating the patch to hide the course overview link when stats is disabled. sending for peer review.
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            Thanks Rossie,

            Patch looks good to me, pushing for integration review.

            Show
            rajeshtaneja Rajesh Taneja added a comment - Thanks Rossie, Patch looks good to me, pushing for integration review.
            Hide
            poltawski Dan Poltawski added a comment -

            Hi Rosie,

            This isn't quite right, it causes problems if the the page isn't added to the admin tree all the time (I made this mistake myself, see the bugfix in MDL-36743).

            The solution is to use the $hidden flag of admin_externalpage which will hide it if necessary

            Show
            poltawski Dan Poltawski added a comment - Hi Rosie, This isn't quite right, it causes problems if the the page isn't added to the admin tree all the time (I made this mistake myself, see the bugfix in MDL-36743 ). The solution is to use the $hidden flag of admin_externalpage which will hide it if necessary
            Hide
            rwijaya Rossiani Wijaya added a comment -

            Hi Dan,

            Thank you for the suggestion.

            I made the changes and re-submitting for integration review.

            Show
            rwijaya Rossiani Wijaya added a comment - Hi Dan, Thank you for the suggestion. I made the changes and re-submitting for integration review.
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            Hello Rossie,

            Not sure, but I think you should use empty($CFG->enablestats), to be on safer side.
            As we have seen $CFG->enablestats might not be set at times.

            Show
            rajeshtaneja Rajesh Taneja added a comment - Hello Rossie, Not sure, but I think you should use empty($CFG->enablestats), to be on safer side. As we have seen $CFG->enablestats might not be set at times.
            Hide
            rwijaya Rossiani Wijaya added a comment -

            Arghh... It was right in front of me to check for empty $CFG and yet I still missed it.

            Thank you Raj for catching that.

            Updated the patch.

            Show
            rwijaya Rossiani Wijaya added a comment - Arghh... It was right in front of me to check for empty $CFG and yet I still missed it. Thank you Raj for catching that. Updated the patch.
            Hide
            damyon Damyon Wiese added a comment -

            The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

            Thanks!

            Show
            damyon Damyon Wiese added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. Thanks!
            Hide
            rwijaya Rossiani Wijaya added a comment -

            Patch rebased.

            Show
            rwijaya Rossiani Wijaya added a comment - Patch rebased.
            Hide
            poltawski Dan Poltawski added a comment -

            Integrated, thanks a lot Rosie!

            Show
            poltawski Dan Poltawski added a comment - Integrated, thanks a lot Rosie!
            Hide
            markn Mark Nelson added a comment -

            Works as expected, passing.

            Show
            markn Mark Nelson added a comment - Works as expected, passing.
            Hide
            damyon Damyon Wiese added a comment -

            Thanks for your hard work - this issue has made it! Moodle is now a little bit better.

            Regards, Damyon

            Show
            damyon Damyon Wiese added a comment - Thanks for your hard work - this issue has made it! Moodle is now a little bit better. Regards, Damyon
            Hide
            salvetore Michael de Raadt added a comment -

            I've added a ui_change and docs_required labels to this issue.

            I suspect the docs might need minor changes after this change.

            Show
            salvetore Michael de Raadt added a comment - I've added a ui_change and docs_required labels to this issue. I suspect the docs might need minor changes after this change.
            Hide
            marycooch Mary Cooch added a comment -

            Removing docs_required label as I have made a note of this in the 2.3 and 2.4 docs http://docs.moodle.org/24/en/Statistics#Enabling_statistics

            Show
            marycooch Mary Cooch added a comment - Removing docs_required label as I have made a note of this in the 2.3 and 2.4 docs http://docs.moodle.org/24/en/Statistics#Enabling_statistics

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  11/Mar/13