Uploaded image for project: 'Plugins'
  1. Plugins
  2. CONTRIB-3528

Change navigate by numbers to navigate by names in the "View by Response tab"

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 1.9.17
    • Fix Version/s: 2.5
    • Component/s: Module: Questionnaire
    • Labels:
      None
    • Affected Branches:
      MOODLE_19_STABLE
    • Fixed Branches:
      MOODLE_25_STABLE

      Description

      Further to these Questionnaire forum requests:
      http://moodle.org/mod/forum/discuss.php?d=196425
      http://moodle.org/mod/forum/discuss.php?d=160878
      I am developing a new navigation system in the "View by Response tab" based on students' names.

        Gliffy Diagrams

          Activity

          Show
          rezeau Joseph Rézeau added a comment - See https://github.com/remotelearner/moodle-mod_questionnaire/tree/CONTRIB-3528_19_STABLE
          Show
          rezeau Joseph Rézeau added a comment - Added branch: https://github.com/remotelearner/moodle-mod_questionnaire/tree/CONTRIB-3528_23_STABLE
          Hide
          rezeau Joseph Rézeau added a comment -

          I have started working on this issue, for version 2.5.

          Show
          rezeau Joseph Rézeau added a comment - I have started working on this issue, for version 2.5.
          Hide
          rezeau Joseph Rézeau added a comment -

          This new feature has been implemented in current BETA 2.5.2 version, on the Github.

          Show
          rezeau Joseph Rézeau added a comment - This new feature has been implemented in current BETA 2.5.2 version, on the Github.
          Hide
          jenny-gray Jenny Gray added a comment -

          I haven't reviewed every line of code changed, because there were so many (lovely) code checker improvements! But I did notice a few places where you've used <b> rather than <strong>, and personally I'd remove rather than comment out old code.

          On testing this specific feature, I get an error when I choose "all responses" then "list of responses"

          Debug info: ERROR: operator does not exist: bigint = character varying
          LINE 3: ... WHERE R.survey_id=1 AND complete = 'y' AND U.id = R.userna...

          I think the R.username needs to be cast AS Int.

          Once that's fixed I can see the list of respondents, and I wondered whether it would be helpful to include the date & time for when there are multiple submissions by the same user?

          Otherwise, I think its OK

          Show
          jenny-gray Jenny Gray added a comment - I haven't reviewed every line of code changed, because there were so many (lovely) code checker improvements! But I did notice a few places where you've used <b> rather than <strong>, and personally I'd remove rather than comment out old code. On testing this specific feature, I get an error when I choose "all responses" then "list of responses" Debug info: ERROR: operator does not exist: bigint = character varying LINE 3: ... WHERE R.survey_id=1 AND complete = 'y' AND U.id = R.userna... I think the R.username needs to be cast AS Int. Once that's fixed I can see the list of respondents, and I wondered whether it would be helpful to include the date & time for when there are multiple submissions by the same user? Otherwise, I think its OK
          Hide
          rezeau Joseph Rézeau added a comment -

          Hi Jenny,
          In order to test Questionnaire behaviour with PostgreSQL I have installed PostgreSQL on my local machine. Took me a day and a half to install, configure, and understand how it works. Anyway, besides the error you mention in your previous comment, I got a similar error upon creating a new questionnaire.
          I am using Windows 7, PHP Version 5.4.7, PostgreSQL 8.3.3 (indicated by phpInfo, but in fact it's the more recent version PostgreSQL 9.2.4, compiled by Visual C++vuild 1600, 64bits).
          Upon creating a new questionnaire, I get this error message:
          Debug info: ERROR: operator does not exist: character varying = bigint
          LINE 1: ...uestionnaire_survey s ON s.id = q.sid AND s.owner = q.course...
          ^
          HINT: No operator matches the given name and argument type(s). You might need to add explicit type casts.
          SELECT s.id,s.name,s.owner,s.realm,s.status,s.title,q.id as qid FROM mdl_questionnaire q INNER JOIN mdl_questionnaire_survey s ON s.id = q.sid AND s.owner = q.course WHERE (realm = $1 OR owner = $2) ORDER BY realm,name
          [array (
          0 => 'template',
          1 => '2',
          )]
          Error code: dmlreadexception
          Stack trace:

          line 423 of \lib\dml\moodle_database.php: dml_read_exception thrown
          line 248 of \lib\dml\pgsql_native_moodle_database.php: call to moodle_database->query_end()
          line 753 of \lib\dml\pgsql_native_moodle_database.php: call to pgsql_native_moodle_database->query_end()
          line 415 of \mod\questionnaire\locallib.php: call to pgsql_native_moodle_database->get_records_sql()
          line 422 of \mod\questionnaire\locallib.php: call to questionnaire_get_survey_list()
          line 98 of \mod\questionnaire\mod_form.php: call to questionnaire_get_survey_select()
          line 191 of \lib\formslib.php: call to mod_questionnaire_mod_form->definition()
          line 71 of \course\moodleform_mod.php: call to moodleform->moodleform()
          line 248 of \course\modedit.php: call to moodleform_mod->moodleform_mod()

          Can you please explain how you fixed the previous, similar, PostgreSQL error and how to fix the one I am reporting?
          Questionnaire seems to be the only Moodle module to be using sql_cast_char2int, so there must be something obsolete in our sql queries somewhere. Any idea?

          Show
          rezeau Joseph Rézeau added a comment - Hi Jenny, In order to test Questionnaire behaviour with PostgreSQL I have installed PostgreSQL on my local machine. Took me a day and a half to install, configure, and understand how it works. Anyway, besides the error you mention in your previous comment, I got a similar error upon creating a new questionnaire. I am using Windows 7, PHP Version 5.4.7, PostgreSQL 8.3.3 (indicated by phpInfo, but in fact it's the more recent version PostgreSQL 9.2.4, compiled by Visual C++vuild 1600, 64bits). Upon creating a new questionnaire, I get this error message: Debug info: ERROR: operator does not exist: character varying = bigint LINE 1: ...uestionnaire_survey s ON s.id = q.sid AND s.owner = q.course... ^ HINT: No operator matches the given name and argument type(s). You might need to add explicit type casts. SELECT s.id,s.name,s.owner,s.realm,s.status,s.title,q.id as qid FROM mdl_questionnaire q INNER JOIN mdl_questionnaire_survey s ON s.id = q.sid AND s.owner = q.course WHERE (realm = $1 OR owner = $2) ORDER BY realm,name [array ( 0 => 'template', 1 => '2', )] Error code: dmlreadexception Stack trace: line 423 of \lib\dml\moodle_database.php: dml_read_exception thrown line 248 of \lib\dml\pgsql_native_moodle_database.php: call to moodle_database->query_end() line 753 of \lib\dml\pgsql_native_moodle_database.php: call to pgsql_native_moodle_database->query_end() line 415 of \mod\questionnaire\locallib.php: call to pgsql_native_moodle_database->get_records_sql() line 422 of \mod\questionnaire\locallib.php: call to questionnaire_get_survey_list() line 98 of \mod\questionnaire\mod_form.php: call to questionnaire_get_survey_select() line 191 of \lib\formslib.php: call to mod_questionnaire_mod_form->definition() line 71 of \course\moodleform_mod.php: call to moodleform->moodleform() line 248 of \course\modedit.php: call to moodleform_mod->moodleform_mod() Can you please explain how you fixed the previous, similar, PostgreSQL error and how to fix the one I am reporting? Questionnaire seems to be the only Moodle module to be using sql_cast_char2int, so there must be something obsolete in our sql queries somewhere. Any idea?
          Hide
          rezeau Joseph Rézeau added a comment -

          OK, I got it!

          1.- Solution is to "cast" type. In function questionnaire_get_survey_list($courseid=0, $type=''), I do something like $castsowner = $DB->sql_cast_char2int('s.owner');

          2.- However, we should really attack the source of the problem, which is an obvious error in the Questionnaire tables.
          in questionnaire table, field course: bigint
          in questionnaire_survey table, field owner:varchar(16) : should be bigint as well!

          If you agree, I will make the type change in the database to solve all our "type cast" problems:
          a.- questionnaire_survey table, field owner: change varchar(16) to bigint(11)
          b.- questionnaire_response able, field username: change varchar(64) to bigint(10)

          Please answer ASAP on this matter so I can continue my work.
          Joseph

          Show
          rezeau Joseph Rézeau added a comment - OK, I got it! 1.- Solution is to "cast" type. In function questionnaire_get_survey_list($courseid=0, $type=''), I do something like $castsowner = $DB->sql_cast_char2int('s.owner'); 2.- However, we should really attack the source of the problem , which is an obvious error in the Questionnaire tables. in questionnaire table, field course: bigint in questionnaire_survey table, field owner:varchar(16) : should be bigint as well! If you agree, I will make the type change in the database to solve all our "type cast" problems: a.- questionnaire_survey table, field owner: change varchar(16) to bigint(11) b.- questionnaire_response able, field username: change varchar(64) to bigint(10) Please answer ASAP on this matter so I can continue my work. Joseph
          Hide
          jenny-gray Jenny Gray added a comment -

          Hi Joseph,

          Sorry I had a system upgrade yesterday and it didn't go as smoothly as I hoped so I'm still a bit busy sorting out the fall-out.

          Yes, the quick solution is to cast. The reason is usually that username can be (used to be??) "anonymous" or an ID number, so the database field has to cope with text. but most of the time you want to compare it to a number. You've found the same problem on the owner field which I know less about, but I'm guessing it could never be anonymous and your proposal to change the field type would be OK.

          Jenny

          Show
          jenny-gray Jenny Gray added a comment - Hi Joseph, Sorry I had a system upgrade yesterday and it didn't go as smoothly as I hoped so I'm still a bit busy sorting out the fall-out. Yes, the quick solution is to cast. The reason is usually that username can be (used to be??) "anonymous" or an ID number, so the database field has to cope with text. but most of the time you want to compare it to a number. You've found the same problem on the owner field which I know less about, but I'm guessing it could never be anonymous and your proposal to change the field type would be OK. Jenny
          Hide
          rezeau Joseph Rézeau added a comment -

          Changing a field type looks like a bit complicated when upgrading versions. I am not too confident with tampering with the upgrade.php file for the moment. So I will go for the easiest fix, i.e. using the $DB->sql_cast_char2int() now.

          Show
          rezeau Joseph Rézeau added a comment - Changing a field type looks like a bit complicated when upgrading versions. I am not too confident with tampering with the upgrade.php file for the moment. So I will go for the easiest fix, i.e. using the $DB->sql_cast_char2int() now.
          Hide
          rezeau Joseph Rézeau added a comment -

          Fixed CAST bugs and committed to remotelearner.

          Show
          rezeau Joseph Rézeau added a comment - Fixed CAST bugs and committed to remotelearner.
          Hide
          rezeau Joseph Rézeau added a comment -

          Jenny "... I wondered whether it would be helpful to include the date & time for when there are multiple submissions by the same user?"

          At the moment, the submission date & time are displayed upon hovering over the respondent's name, as shown in attached screenshot #1. It would be quite feasible to display that information next to the respondent's name, but this would take up quite some space, and might cause a display problem in the default 3-column display, in longer lists.

          I propose to keep things as they are at the moment, and offer this extra display as an option when (if) we offer a "customizable" List of responses report one day.

          Joseph

          Show
          rezeau Joseph Rézeau added a comment - Jenny "... I wondered whether it would be helpful to include the date & time for when there are multiple submissions by the same user?" At the moment, the submission date & time are displayed upon hovering over the respondent's name, as shown in attached screenshot #1. It would be quite feasible to display that information next to the respondent's name, but this would take up quite some space, and might cause a display problem in the default 3-column display, in longer lists. I propose to keep things as they are at the moment, and offer this extra display as an option when (if) we offer a "customizable" List of responses report one day. Joseph
          Hide
          jenny-gray Jenny Gray added a comment -

          Ah, sorry I missed that! I'm sure that's fine.

          Show
          jenny-gray Jenny Gray added a comment - Ah, sorry I missed that! I'm sure that's fine.
          Hide
          jwhite10 John White added a comment - - edited

          Joseph, Jenny & all,
          I'm attaching this file here because I have installed an instance of the navigate by names solution suggested here, and, without intending any offence, I don't feel it is as compact or as easy to use as the offering I have made in CONTRIB-4260. This is especially so because the dropdown selector of users shown on the right of the image always remains compact no matter how many user records are included. I will try to put up an illustration of this as well.

          Attached image: Screen Shot 2013-05-28 at 15.03.03

          Show
          jwhite10 John White added a comment - - edited Joseph, Jenny & all, I'm attaching this file here because I have installed an instance of the navigate by names solution suggested here, and, without intending any offence, I don't feel it is as compact or as easy to use as the offering I have made in CONTRIB-4260 . This is especially so because the dropdown selector of users shown on the right of the image always remains compact no matter how many user records are included. I will try to put up an illustration of this as well. Attached image: Screen Shot 2013-05-28 at 15.03.03
          Hide
          rezeau Joseph Rézeau added a comment -

          John, thanks for pointing to your earlier contribution. I'll have a look and report there (in CONTRIB-4260).
          Joseph

          Show
          rezeau Joseph Rézeau added a comment - John, thanks for pointing to your earlier contribution. I'll have a look and report there (in CONTRIB-4260 ). Joseph

            People

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

              Dates

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

                Development