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

      Description

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

      • mod/data
      • mod/feedback

        Gliffy Diagrams

          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: