[MDL-30828] Feedback module - error when sorting by date - Moodle Tracker

Details

Type: Bug
Status: Closed
Priority: Minor
Resolution: Fixed
Affects Version/s: 2.0.1, 2.1.4, 2.2.1, 2.3
Fix Version/s: 2.1.5, 2.2.2
Component/s: Feedback
Labels:
- patch
- triaged

Database:

MySQL
Testing Instructions:

Hide

enable the feedback module in "manage activities" in your site settings

create a feedback activity. In activity settings set "record user names" to NOT be anonymous

Add at least one question then log in as a student and provide feedback.

Log back in as a teacher, also provide feedback so you have 2 responses.

Click Show Responses.

Click the date column header and check that it sorts appropriately.

Click on the name column headers and check that they sort appropriately.

Show
enable the feedback module in "manage activities" in your site settings create a feedback activity. In activity settings set "record user names" to NOT be anonymous Add at least one question then log in as a student and provide feedback. Log back in as a teacher, also provide feedback so you have 2 responses. Click Show Responses. Click the date column header and check that it sorts appropriately. Click on the name column headers and check that they sort appropriately.
Affected Branches:
MOODLE_20_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE
Fixed Branches:
MOODLE_21_STABLE, MOODLE_22_STABLE
Pull from Repository:
git://github.com/andyjdavis/moodle.git
Pull Master Branch:
~~MDL-30828~~_feedback_sort
Pull Master Diff URL:
https://github.com/andyjdavis/moodle/compare/master...MDL-30828_feedback_sort

Description

Module feedback - when sorting by date shows the error reading from the database.

Gliffy Diagrams

Attachments

Options
- Sort By Name
- Sort By Date
- Ascending
- Descending
- Thumbnails
- List
- Download All

Attachments

feedback_show.jpg
18 kB
21/Dec/11 11:49 PM
patch.diff
0.6 kB
20/Dec/11 11:08 PM
patch.diff
0.6 kB
20/Dec/11 9:18 PM

Activity

Ascending order - Click to sort in descending order

Hide

Permalink

Michael de Raadt added a comment - 21/Dec/11 9:23 AM

Thanks for reporting this.

It would be good if you could add more information, such as screenshots, replication instructions and the output of the error.

Show

Michael de Raadt added a comment - 21/Dec/11 9:23 AM Thanks for reporting this. It would be good if you could add more information, such as screenshots, replication instructions and the output of the error.

Hide

Permalink

Cristiano Boell added a comment - 21/Dec/11 11:49 PM

when sorting by date

Show

Cristiano Boell added a comment - 21/Dec/11 11:49 PM when sorting by date

Hide

Permalink

Cristiano Boell added a comment - 21/Dec/11 11:55 PM

presents the field duplicate timemodified, adding the field in the session select the function works normally

Show

Cristiano Boell added a comment - 21/Dec/11 11:55 PM presents the field duplicate timemodified, adding the field in the session select the function works normally

Hide

Permalink

Andrew Davis added a comment - 27/Dec/11 12:35 PM

It took quite a lot of experimenting but I was eventually able to find the problem. I'm not sure I've ever used the feedback module before. Internally its quite different from the rest of Moodle. I've added testing instructions to record the process to get to the bug.

I haven't yet encountered the but in this report but I also haven't managed to get sorting to work at all yet. The sorting seems entirely broken and always sorts by participant lastname no matter what you click on.

Show

Andrew Davis added a comment - 27/Dec/11 12:35 PM It took quite a lot of experimenting but I was eventually able to find the problem. I'm not sure I've ever used the feedback module before. Internally its quite different from the rest of Moodle. I've added testing instructions to record the process to get to the bug. I haven't yet encountered the but in this report but I also haven't managed to get sorting to work at all yet. The sorting seems entirely broken and always sorts by participant lastname no matter what you click on.

Hide

Permalink

Andrew Davis added a comment - 28/Dec/11 4:34 PM

I'm putting up a potential fix for peer review. Its a modified version of the original patch. Sorting was broken because the param type PARAM_ALPHANUMEXT, used by optional_param(), removes full stops. This caused c.timemodified to be received as ctimemodified. That column doesnt exist so the sort always defaulted to last name.

Show

Andrew Davis added a comment - 28/Dec/11 4:34 PM I'm putting up a potential fix for peer review. Its a modified version of the original patch. Sorting was broken because the param type PARAM_ALPHANUMEXT, used by optional_param(), removes full stops. This caused c.timemodified to be received as ctimemodified. That column doesnt exist so the sort always defaulted to last name.

Hide

Permalink

Adrian Greeve added a comment - 10/Jan/12 12:02 PM

I also couldn't replicate the error message.
I think that the code is a nice fix for sorting via the date heading.
Thanks Andrew and Cristiano.

Show

Adrian Greeve added a comment - 10/Jan/12 12:02 PM I also couldn't replicate the error message. I think that the code is a nice fix for sorting via the date heading. Thanks Andrew and Cristiano.

Hide

Permalink

Eloy Lafuente (stronk7) added a comment - 27/Jan/12 11:11 AM

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.

TIA and ciao

Show

Eloy Lafuente (stronk7) added a comment - 27/Jan/12 11:11 AM 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. TIA and ciao

Hide

Permalink

Aparup Banerjee added a comment - 31/Jan/12 4:55 PM

cool, thats integrated into 21(noted new lines), 22 and master now for testing on all branches.

Show

Aparup Banerjee added a comment - 31/Jan/12 4:55 PM cool, thats integrated into 21(noted new lines), 22 and master now for testing on all branches.

Hide

Permalink

Rajesh Taneja added a comment - 01/Feb/12 5:27 PM

Thanks Andrew
Works Great ...

Show

Rajesh Taneja added a comment - 01/Feb/12 5:27 PM Thanks Andrew Works Great ...

Hide

Permalink

Eloy Lafuente (stronk7) added a comment - 02/Feb/12 8:20 PM

Many thanks for your collaboration, this code has been integrated upstream and it's available in all the repositories.

Closing, ciao

Show

Eloy Lafuente (stronk7) added a comment - 02/Feb/12 8:20 PM Many thanks for your collaboration, this code has been integrated upstream and it's available in all the repositories. Closing, ciao

People

Assignee:

Andrew Davis

Reporter:

Cristiano Boell

Peer reviewer:

Adrian Greeve

Integrator:

Aparup Banerjee

Tester:

Rajesh Taneja

Participants:

Adrian Greeve, Andrew Davis, Aparup Banerjee, Cristiano Boell, Eloy Lafuente (stronk7), Michael de Raadt, Rajesh Taneja

Votes:

0 Vote for this issue

Watchers:

1 Start watching this issue

Dates

Created:

20/Dec/11 9:18 PM

Updated:

30/Sep/13 10:14 AM

Resolved:

02/Feb/12 8:20 PM

Fix Release Date:

12/Mar/12