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

Database: Date field not autoselecting month when editing

    Details

    • Testing Instructions:
      Hide

      1) Login in as teacher (admin, manager)
      2) Create an instance of a database activity module
      3) Add a date field to the form
      4) Add an entry with a date which is not in January
      6) Edit an entry: Make sure that date shows the same you entered (previously month reset to January)

      Show
      1) Login in as teacher (admin, manager) 2) Create an instance of a database activity module 3) Add a date field to the form 4) Add an entry with a date which is not in January 6) Edit an entry: Make sure that date shows the same you entered (previously month reset to January)
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Pull from Repository:
      git@github.com:marinaglancy/moodle.git
    • Pull Master Branch:
      wip-MDL-28352-master

      Description

      When adding a record to the database using a date field, the default is to have the selected items set for today's date. This is not working if the user's timezone is set (not using system timezone).

        Gliffy Diagrams

          Activity

          Hide
          aborrow Anthony Borrow added a comment -

          I've made this a database issue since that is where it was observed; however, in looking into the issue I found that it is actually an issue with lib/moodlelib.php's getuserdate function.

          If the server time is being used, PHP's getdate function is used to generate the array. This array returns the 'mon' item as an integer so for July it return 7. If $timezone is not 99 (servertime), then gmstrftime is used with the %m parameter which is the two digit representation of the month which is created as a string (i.e. 07).

          When lib/outputcomponents.php select_option method uses in_array to check if the selected value is in the array of available options, it is currently using the strict comparison and thus 07 (string) is not found since 7 (int) is what it is looking for.

          While we could simply remove the strictness and the function would work, my concern is that the usergetdate function should return a consistent array and currently it is not.

          One solution I contemplated was to change %m (two digits) to %n (single digit); however, the gmstrftime does not accept the %n option. Thus I think the best solution is to simply cast the value as an int before returning it.

          One concern that Rajesh raised was that if there were other places in the code that were expecting a string that those will break with this solution. My response is that those would already break if the timezone is using the server time (i.e. 99). So this makes it consistent.

          Peace - Anthony

          Show
          aborrow Anthony Borrow added a comment - I've made this a database issue since that is where it was observed; however, in looking into the issue I found that it is actually an issue with lib/moodlelib.php's getuserdate function. If the server time is being used, PHP's getdate function is used to generate the array. This array returns the 'mon' item as an integer so for July it return 7. If $timezone is not 99 (servertime), then gmstrftime is used with the %m parameter which is the two digit representation of the month which is created as a string (i.e. 07). When lib/outputcomponents.php select_option method uses in_array to check if the selected value is in the array of available options, it is currently using the strict comparison and thus 07 (string) is not found since 7 (int) is what it is looking for. While we could simply remove the strictness and the function would work, my concern is that the usergetdate function should return a consistent array and currently it is not. One solution I contemplated was to change %m (two digits) to %n (single digit); however, the gmstrftime does not accept the %n option. Thus I think the best solution is to simply cast the value as an int before returning it. One concern that Rajesh raised was that if there were other places in the code that were expecting a string that those will break with this solution. My response is that those would already break if the timezone is using the server time (i.e. 99). So this makes it consistent. Peace - Anthony
          Hide
          aborrow Anthony Borrow added a comment -

          Grepping for select_time it appears that the html_writer:select_time function also is used by forum/search.php so I will add forum to the list of components. Peace - Anthony

          Show
          aborrow Anthony Borrow added a comment - Grepping for select_time it appears that the html_writer:select_time function also is used by forum/search.php so I will add forum to the list of components. Peace - Anthony
          Hide
          skodak Petr Skoda added a comment -

          I am arid it is hard to decide what is the expected result here, it might be easier to fix the problematic use instead.

          Show
          skodak Petr Skoda added a comment - I am arid it is hard to decide what is the expected result here, it might be easier to fix the problematic use instead.
          Hide
          aborrow Anthony Borrow added a comment -

          I think the expected result is that the mon would be a number like the day and year. It just makes sense to me that it would be consistent. The in_array is expecting that. Also, I was testing something else today and when I was applying a filter to browse users I found:

          Notice: Undefined index: month in /home/arborrow/NetBeansProjects/m21/lib/form/dateselector.php on line 229

          When I applied the patch to cast mon as an integer the notice went away. I think we just pick one, document it and fix up anywhere that may not have been doing that. At least that way there is a clear path forward.

          Peace - Anthony

          Show
          aborrow Anthony Borrow added a comment - I think the expected result is that the mon would be a number like the day and year. It just makes sense to me that it would be consistent. The in_array is expecting that. Also, I was testing something else today and when I was applying a filter to browse users I found: Notice: Undefined index: month in /home/arborrow/NetBeansProjects/m21/lib/form/dateselector.php on line 229 When I applied the patch to cast mon as an integer the notice went away. I think we just pick one, document it and fix up anywhere that may not have been doing that. At least that way there is a clear path forward. Peace - Anthony
          Hide
          nebgor Aparup Banerjee added a comment -

          This has been integrated.

          Tester: please test that the issue has been resolved.

          Show
          nebgor Aparup Banerjee added a comment - This has been integrated. Tester: please test that the issue has been resolved.
          Hide
          ankit_frenz Ankit Agarwal added a comment -

          This is working as expected.
          Great Fix.
          Passing!
          Thanks

          Show
          ankit_frenz Ankit Agarwal added a comment - This is working as expected. Great Fix. Passing! Thanks
          Hide
          stronk7 Eloy Lafuente (stronk7) added a comment -

          Your nice code represents only 1/46 of the issues that have been sent upstream this week, so thanks, but not many.

          Nah, joking, many thanks! Closing this a fixed, ciao

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - Your nice code represents only 1/46 of the issues that have been sent upstream this week, so thanks, but not many. Nah, joking, many thanks! Closing this a fixed, ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved:
                Fix Release Date:
                12/Mar/12