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

Fixed the temporarily fixed array elements in Lesson

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.5
    • Fix Version/s: 2.5.1
    • Component/s: Lesson
    • Labels:
    • Testing Instructions:
      Hide
      1. Set debugger to 'developer'
      2. Create new lesson
      3. Enable 'Allow student review
      4. Add a matching question
      5. Add a multiple choices question
      6. Edit the existing questions
      7. Attempt the lesson as student
      8. Review the lesson

      Make sure there is no warning for setType() through out the above process.

      Note: Bug for matching question in review page will be fix through MDL-39546.

      Show
      Set debugger to 'developer' Create new lesson Enable 'Allow student review Add a matching question Add a multiple choices question Edit the existing questions Attempt the lesson as student Review the lesson Make sure there is no warning for setType() through out the above process. Note: Bug for matching question in review page will be fix through MDL-39546.
    • Affected Branches:
      MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_25_STABLE
    • Pull from Repository:
    • Pull Master Branch:

      Description

      Replace the temporary fixed for setType().

      search string:

      // Temporary fixed until MDL-38885 gets integrated
      

        Gliffy Diagrams

          Attachments

            Activity

            Hide
            ankit_frenz Ankit Agarwal added a comment -

            Hi Rosie,
            The patch is nice, a few things you might want to consider:-

            1. $this->_form->addElement('text', 'response_editor['.$count.']', $label, array('size'=>'50')); , will it be better to use PARAM_NOTAGS or we expecting html there?
            2. same with $mform->addElement('hidden', 'answer['.$answer->id.']', $answer->answer);
            3. you missed one

              $mform->addElement('hidden', 'response['.$answer->id.']', htmlspecialchars(trim($answers[$useranswers[$i]]->response)));
                                  // Temporary fixed until MDL-38885 gets integrated
                                  $mform->setType('response', PARAM_TEXT);

              cheers

            Show
            ankit_frenz Ankit Agarwal added a comment - Hi Rosie, The patch is nice, a few things you might want to consider:- $this->_form->addElement('text', 'response_editor ['.$count.'] ', $label, array('size'=>'50')); , will it be better to use PARAM_NOTAGS or we expecting html there? same with $mform->addElement('hidden', 'answer ['.$answer->id.'] ', $answer->answer); you missed one $mform->addElement('hidden', 'response['.$answer->id.']', htmlspecialchars(trim($answers[$useranswers[$i]]->response))); // Temporary fixed until MDL-38885 gets integrated $mform->setType('response', PARAM_TEXT); cheers
            Hide
            rwijaya Rossiani Wijaya added a comment -

            Thanks Ankit for reviewing.

            Updated the patch to your suggestion and removing the temporary fix comment.

            Show
            rwijaya Rossiani Wijaya added a comment - Thanks Ankit for reviewing. Updated the patch to your suggestion and removing the temporary fix comment.
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Thanks Rosie, this has been integrated now.

            Show
            samhemelryk Sam Hemelryk added a comment - Thanks Rosie, this has been integrated now.
            Hide
            dmonllao David Monllaó added a comment - - edited

            It passes, I've seen no warnings

            (Edited: I've seen php warnings/notices in the review page as commented)

            Show
            dmonllao David Monllaó added a comment - - edited It passes, I've seen no warnings (Edited: I've seen php warnings/notices in the review page as commented)
            Hide
            poltawski Dan Poltawski added a comment -

            Thanks for your contributions!

            _main:
            @ BB#0:
                    push    {r7, lr}
                    mov     r7, sp
                    sub     sp, #4
                    movw    r0, :lower16:(L_.str-(LPC0_0+4))
                    movt    r0, :upper16:(L_.str-(LPC0_0+4))
            LPC0_0:
                    add     r0, pc
                    bl      _printf
                    movs    r1, #0
                    movt    r1, #0
                    str     r0, [sp]                @ 4-byte Spill
                    mov     r0, r1
                    add     sp, #4
                    pop     {r7, pc}
             
                    .section        __TEXT,__cstring,cstring_literals
            L_.str:                                 @ @.str
                    .asciz   "This code is now upstream!"
            

            Show
            poltawski Dan Poltawski added a comment - Thanks for your contributions! _main: @ BB#0: push {r7, lr} mov r7, sp sub sp, #4 movw r0, :lower16:(L_.str-(LPC0_0+4)) movt r0, :upper16:(L_.str-(LPC0_0+4)) LPC0_0: add r0, pc bl _printf movs r1, #0 movt r1, #0 str r0, [sp] @ 4-byte Spill mov r0, r1 add sp, #4 pop {r7, pc}   .section __TEXT,__cstring,cstring_literals L_.str: @ @.str .asciz "This code is now upstream!"

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  8/Jul/13