Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.4
    • Fix Version/s: 2.4
    • Component/s: Libraries
    • Labels:
    • Testing Instructions:
      Hide

      Database and feedback modules has been modified, test them making sure there are no exceptions thrown

      1. mod/data
        1. Create a database activity adding different types of fields
        2. Export and import data through the preset tab
      2. mod/feedback
        1. Create a feedback activity
        2. Add questions
        3. Complete the activity as a student
        4. Analyse results
        5. Make sure all works as expected, if something doesn't work as expected be sure that is because of this modification
      Show
      Database and feedback modules has been modified, test them making sure there are no exceptions thrown mod/data Create a database activity adding different types of fields Export and import data through the preset tab mod/feedback Create a feedback activity Add questions Complete the activity as a student Analyse results Make sure all works as expected, if something doesn't work as expected be sure that is because of this modification
    • Affected Branches:
      MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-34464_master
    • Rank:
      42879

      Description

      Replace get_context_instance with context_XXXX::instance() in set location (group 11)
      Locations

      • mod/data
      • mod/feedback

        Issue Links

          Activity

          Hide
          Ankit Agarwal added a comment - - edited

          Hi David,

          The patch looks great. A few small things to consider:-

          1. mod/data/export.php
            $context = context_module::instance($cm->id, IGNORE_MISSING);
            should be
            $context = context_module::instance($cm->id);
          2. mod/feedback/item/label/label_form.php
            you deleted $context = get_context_instance(CONTEXT_MODULE, $common['cmid']); but didn't add its equivalent to the code
          3. mod/feedback/item/label/lib.php L235
            I am not sure about practical implication of this file_postupdate_standard_editor() doesn't seem to require $context to be set always. Again am not sure if that is enough to change the strictness to IGNORE_MISSING or not. But worth a deeper look.

          Thanks

          Show
          Ankit Agarwal added a comment - - edited Hi David, The patch looks great. A few small things to consider:- mod/data/export.php $context = context_module::instance($cm->id, IGNORE_MISSING); should be $context = context_module::instance($cm->id); mod/feedback/item/label/label_form.php you deleted $context = get_context_instance(CONTEXT_MODULE, $common ['cmid'] ); but didn't add its equivalent to the code mod/feedback/item/label/lib.php L235 I am not sure about practical implication of this file_postupdate_standard_editor() doesn't seem to require $context to be set always. Again am not sure if that is enough to change the strictness to IGNORE_MISSING or not. But worth a deeper look. Thanks
          Hide
          David Monllaó added a comment -

          Hi Ankit, thanks for your comments

          1. Changed as you proposed
          2. This $context var was never used
          3. file_postupdate_standard_editor() accesses $context attributes and calls trusttext_trusted() which requires $context

          Submitting for peer review

          Show
          David Monllaó added a comment - Hi Ankit, thanks for your comments Changed as you proposed This $context var was never used file_postupdate_standard_editor() accesses $context attributes and calls trusttext_trusted() which requires $context Submitting for peer review
          Hide
          Ankit Agarwal added a comment -

          Well if $options['trusttext'] is not set and $options['maxfiles] = 0 in that case you wont need $context. But that is way too specific, shouldn't be happening anyway.

          Looks good to me. Feel free to submit for integration.

          Show
          Ankit Agarwal added a comment - Well if $options ['trusttext'] is not set and $options ['maxfiles] = 0 in that case you wont need $context. But that is way too specific, shouldn't be happening anyway. Looks good to me. Feel free to submit for integration.
          Hide
          Aparup Banerjee added a comment -

          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
          Aparup Banerjee added a comment - 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
          David Monllaó added a comment -

          Hi Aparup, pull branch rebased, thanks

          Show
          David Monllaó added a comment - Hi Aparup, pull branch rebased, thanks
          Hide
          Ankit Agarwal added a comment -

          Hi David,
          Can you please add some testing instructions.
          Thanks

          Show
          Ankit Agarwal added a comment - Hi David, Can you please add some testing instructions. Thanks
          Hide
          David Monllaó added a comment -

          Thanks for the comments Ankit, adding testing instructions

          Show
          David Monllaó added a comment - Thanks for the comments Ankit, adding testing instructions
          Hide
          Dan Poltawski added a comment -

          Thanks David, i've integrated this now.

          I made one fix to mod/feedback/show_entries_anonym.php, as it seemed you'd put an IGNORE_MISSING where it shouldn't ahve been.

          Show
          Dan Poltawski added a comment - Thanks David, i've integrated this now. I made one fix to mod/feedback/show_entries_anonym.php, as it seemed you'd put an IGNORE_MISSING where it shouldn't ahve been.
          Hide
          Rossiani Wijaya added a comment -

          Test passed.

          Show
          Rossiani Wijaya added a comment - Test passed.
          Hide
          Dan Poltawski added a comment -

          asko, Дзякуй, ধন্যবাদ, Благодаря, Gràcies, 感谢, 謝謝, Hvala, Díky, Tak, Bedankt, Tänan, متشکریم, Salamat, Kiitokset, Merci, Grazas, Danke, Ευχαριστώ, આભાર, תודה, धन्यवाद, Köszönjük, Takk fyrir, Terima Kasih, Grazie, ありがとうございます, Рахмет, សូមអរគុណ, 감사합니다, gratiās, Pateicamies, Ačiū, Благодарам, Tēnā koa, Kia Ora Rawa Atu, आभारी आहोत, Талархал, Takk, Dziękuję, Obrigado, Mulţumesc, Engraziel, Спасибо, Fa'afetai, Хвала, Hvala, ස්තූතියි, Vďaka, Hvala, Mahadsanid, Thanks, Gracias, Tack, Salamat, நன்றி, నెనరులు, ขอบคุณค่ะ!

          Your work has made it into this weeks Moodle release! There are no gold medals available this week - but millions around the world will benefit. Thank you!

          Show
          Dan Poltawski added a comment - asko, Дзякуй, ধন্যবাদ, Благодаря, Gràcies, 感谢, 謝謝, Hvala, Díky, Tak, Bedankt, Tänan, متشکریم, Salamat, Kiitokset, Merci, Grazas, Danke, Ευχαριστώ, આભાર, תודה, धन्यवाद, Köszönjük, Takk fyrir, Terima Kasih, Grazie, ありがとうございます, Рахмет, សូមអរគុណ, 감사합니다, gratiās, Pateicamies, Ačiū, Благодарам, Tēnā koa, Kia Ora Rawa Atu, आभारी आहोत, Талархал, Takk, Dziękuję, Obrigado, Mulţumesc, Engraziel, Спасибо, Fa'afetai, Хвала, Hvala, ස්තූතියි, Vďaka, Hvala, Mahadsanid, Thanks, Gracias, Tack, Salamat, நன்றி, నెనరులు, ขอบคุณค่ะ! Your work has made it into this weeks Moodle release! There are no gold medals available this week - but millions around the world will benefit. Thank you!

            People

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

              Dates

              • Created:
                Updated:
                Resolved: