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
    • Rank:
      17996

      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).

        Activity

        Hide
        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
        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
        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
        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
        Petr Škoda 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
        Petr Škoda 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
        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
        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
        Aparup Banerjee added a comment -

        This has been integrated.

        Tester: please test that the issue has been resolved.

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

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

        Show
        Ankit Agarwal added a comment - This is working as expected. Great Fix. Passing! Thanks
        Hide
        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
        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: