Moodle
  1. Moodle
  2. MDL-35694

Course log report export to excel looses information

        Details

        • Type: Bug Bug
        • Status: Closed
        • Priority: Minor Minor
        • Resolution: Fixed
        • Affects Version/s: 2.3.2, 2.4
        • Fix Version/s: 2.3.3
        • Component/s: Reports
        • Labels:
        • Testing Instructions:
          Hide

          1. find an existing course with some random content on your install (or create some)
          2. browse around the course, maybe add a new activity - basically generate a few log entries.
          3. Go to the log course report (from the navigation block)
          4. Click get these logs - today's logs should be displayed onscreen. This is unchanged.
          5. Change the Download in dropdown to excel format, press get these logs.
          6. Check the excel file opens and contains additional link information in the action column which matches the link for that item in the onscreen display.
          7. Repeat steps 5 and 6 for ODS and text formats.

          Show
          1. find an existing course with some random content on your install (or create some) 2. browse around the course, maybe add a new activity - basically generate a few log entries. 3. Go to the log course report (from the navigation block) 4. Click get these logs - today's logs should be displayed onscreen. This is unchanged. 5. Change the Download in dropdown to excel format, press get these logs. 6. Check the excel file opens and contains additional link information in the action column which matches the link for that item in the onscreen display. 7. Repeat steps 5 and 6 for ODS and text formats.
        • Affected Branches:
          MOODLE_23_STABLE, MOODLE_24_STABLE
        • Fixed Branches:
          MOODLE_23_STABLE
        • Pull from Repository:
          git@github.com:jennymgray/moodle.git
        • Pull Master Branch:
          wip-MDL-35694
        • Pull Master Diff URL:
        • Rank:
          44435

          Description

          When you run the log report for a course and display results to screen the action column includes a link so you can repeat the action.

          When you display the results as excel the url for the action is lost and hence so is some of the detail of what the user actually did. It would be good if the excel export could include the action as well.

          (ODS and text exports not yet checked, probably suffer from same problem)

            Activity

            Ascending order - Click to sort in descending order
            Hide
            Permalink
            Jenny Gray added a comment - - edited

            ODS and text (csv) exports do have the same problem.

            Two choices: add extra column with url, or add url within the current action column. I've chosen the latter in case people are importing output into other reporting systems (like crystal) where adding an extra column would probably cause them more problems. Though arguably adding extra data in existing columns may cause problems too, depending on how their tools work. If integrators feel a separate column would be better, then I am happy to rework it that way (though would we then change the onscreen display too?)

            I tried using write_url from the excel library, but this produced a corrupt spreadsheet It also didn't let me pass link text and link address together to match the onscreen display, which is what I really wanted to do. So instead I have added the action url in brackets after the existing text and used the same approach for all three download types.

            Since this does not include any database changes or API changes, I will submit a branch for integration to 2.3.x as well as master (though whether the branch is duff or not I'm not sure because git seems to think I have a detached head when I checkout 23_STABLE). To note also that the commit from master does not cherry-pick cleanly to 23 because the cvsexporter is used in master but not in 23 within print_log_csv.

            Show
            Jenny Gray added a comment - - edited ODS and text (csv) exports do have the same problem. Two choices: add extra column with url, or add url within the current action column. I've chosen the latter in case people are importing output into other reporting systems (like crystal) where adding an extra column would probably cause them more problems. Though arguably adding extra data in existing columns may cause problems too, depending on how their tools work. If integrators feel a separate column would be better, then I am happy to rework it that way (though would we then change the onscreen display too?) I tried using write_url from the excel library, but this produced a corrupt spreadsheet It also didn't let me pass link text and link address together to match the onscreen display, which is what I really wanted to do. So instead I have added the action url in brackets after the existing text and used the same approach for all three download types. Since this does not include any database changes or API changes, I will submit a branch for integration to 2.3.x as well as master (though whether the branch is duff or not I'm not sure because git seems to think I have a detached head when I checkout 23_STABLE). To note also that the commit from master does not cherry-pick cleanly to 23 because the cvsexporter is used in master but not in 23 within print_log_csv.
            Hide
            Permalink
            Michael de Raadt added a comment -

            Hi, Jenny.

            I've reopened this issue so it gets an opportunity for peer review.

            Show
            Michael de Raadt added a comment - Hi, Jenny. I've reopened this issue so it gets an opportunity for peer review.
            Hide
            Permalink
            Adrian Greeve added a comment -

            [Y] Syntax
            [Y] Output
            [Y] Whitespace
            [N/A] Language
            [N/A] Databases
            [Y] Testing
            [N/A] Security
            [N/A] Documentation
            [N] Git - The commit message is missing the component information (In this case 'logs')
            [Y] Sanity check

            I think that the changes make sense and it's nice to have the url information downloaded as well.
            After reading the comments that Jenny made I see no reason not to go with this solution.

            Please fix up the git commit message and if needed I would be happy to submit this for integration review.

            Show
            Adrian Greeve added a comment - [Y] Syntax [Y] Output [Y] Whitespace [N/A] Language [N/A] Databases [Y] Testing [N/A] Security [N/A] Documentation [N] Git - The commit message is missing the component information (In this case 'logs') [Y] Sanity check I think that the changes make sense and it's nice to have the url information downloaded as well. After reading the comments that Jenny made I see no reason not to go with this solution. Please fix up the git commit message and if needed I would be happy to submit this for integration review.
            Hide
            Permalink
            Jenny Gray added a comment -

            Commit message fixed, but for some reason git got very unhappy about pushing the commit back and I had to --force (even though master hasn't changed, my remote branch hasn't changed... don't understand that). Hopefully I haven't done anything tragic for integration!!

            Show
            Jenny Gray added a comment - Commit message fixed, but for some reason git got very unhappy about pushing the commit back and I had to --force (even though master hasn't changed, my remote branch hasn't changed... don't understand that). Hopefully I haven't done anything tragic for integration!!
            Hide
            Permalink
            Aparup Banerjee added a comment -

            integrated with no problems
            (into 23 and master)
            Thanks!

            Show
            Aparup Banerjee added a comment - integrated with no problems (into 23 and master) Thanks!
            Hide
            Permalink
            Andrew Davis added a comment -

            Works as described in 2.3 and master. Passing.

            Show
            Andrew Davis added a comment - Works as described in 2.3 and master. Passing.
            Hide
            Permalink
            Eloy Lafuente (stronk7) added a comment -

            From somewhere within the clouds...

            Congrats, this has been sent upstream and is now part of Moodle (your favorite LMS platform). Many thanks for your awesome collaboration!

            Ciao

            Show
            Eloy Lafuente (stronk7) added a comment - From somewhere within the clouds... Congrats, this has been sent upstream and is now part of Moodle (your favorite LMS platform). Many thanks for your awesome collaboration! Ciao

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved: