Moodle
  1. Moodle
  2. MDL-31768

it is not possible to add a picture to the thanks page in feedback

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.1.5, 2.2.2
    • Fix Version/s: 2.1.6, 2.2.3
    • Component/s: Feedback
    • Labels:
    • Testing Instructions:
      Hide
      1. create a new feedback instance
      2. insert a picture into the page_after_submit editor field
      3. save the new instance
      4. update the instance
      5. add another picture into the page_after_submit editor field
      6. save the instance
      7. open the feedback instance. The pictures should be shown at the overview page
      8. add one or more questions
      9. complete this feedback. At the end the page_after_submit page should be shown
      10. backup the course including the feedback instance
      11. restore the course including the feedback instance
      12. doublicate the feedback instance inside the course
      Show
      create a new feedback instance insert a picture into the page_after_submit editor field save the new instance update the instance add another picture into the page_after_submit editor field save the instance open the feedback instance. The pictures should be shown at the overview page add one or more questions complete this feedback. At the end the page_after_submit page should be shown backup the course including the feedback instance restore the course including the feedback instance doublicate the feedback instance inside the course
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-31768_master_wip
    • Rank:
      38377

      Description

      It is not possible to upload a picture in the "Page after submit" textarea of the feedback settings page.

        Activity

        Hide
        Andreas Grabs added a comment -

        Hi Danielle,
        thank you for reporting this.
        Andreas

        Show
        Andreas Grabs added a comment - Hi Danielle, thank you for reporting this. Andreas
        Hide
        Aparup Banerjee added a comment -

        I'm not really sure this is a bug - its definitely not been implemented before but I'm happy to put this into master.

        Show
        Aparup Banerjee added a comment - I'm not really sure this is a bug - its definitely not been implemented before but I'm happy to put this into master.
        Hide
        Aparup Banerjee added a comment -

        Hi Andreas,
        i've found some whitespaces and code formatting issues which are minor but i'm short on time today.

        also, please send this issue up for peer-review , it helps that there are more eyes looking at the code going into core

        (still thinking its going into master only tho, will look at this next week)

        Show
        Aparup Banerjee added a comment - Hi Andreas, i've found some whitespaces and code formatting issues which are minor but i'm short on time today. also, please send this issue up for peer-review , it helps that there are more eyes looking at the code going into core (still thinking its going into master only tho, will look at this next week)
        Hide
        Andreas Grabs added a comment -

        Hi Aparup,
        I do not understand, what I have to do. How can I send this issue up for peer-review? What is this meaning?
        Andreas

        Show
        Andreas Grabs added a comment - Hi Aparup, I do not understand, what I have to do. How can I send this issue up for peer-review? What is this meaning? Andreas
        Hide
        Aparup Banerjee added a comment -

        Hi Andreas,
        i think the work done is great but i think its actually a new function right? not really a bug - so its better that this goes only into master (instead of changing things in STABLE versions).

        besides that i noticed this issue wasn't peer-reviewed. we've got a peer-review process described @ http://docs.moodle.org/dev/Process - basically after development it helps to get the issue peer reviewed. so in the end at least 3 eyes have gone through the code (1:developer ,2: peer-reviewer, 3: integrator)

        Ah, i'm guessing you probably can't see the 'peer-review' button ? It might be under the workflow menu for you among the 'send for integration group of buttons. We really should have images/docs for this in the process doc.

        This way we can reduce bugs/regressions and keep moodle quality high

        ps: i can fix this up on friday and get it integrated soon - i ran out of time. Just wanted to let you know about 'peer-review' stage in the workflow - cheers!

        Show
        Aparup Banerjee added a comment - Hi Andreas, i think the work done is great but i think its actually a new function right? not really a bug - so its better that this goes only into master (instead of changing things in STABLE versions). besides that i noticed this issue wasn't peer-reviewed. we've got a peer-review process described @ http://docs.moodle.org/dev/Process - basically after development it helps to get the issue peer reviewed. so in the end at least 3 eyes have gone through the code (1:developer ,2: peer-reviewer, 3: integrator) Ah, i'm guessing you probably can't see the 'peer-review' button ? It might be under the workflow menu for you among the 'send for integration group of buttons. We really should have images/docs for this in the process doc . This way we can reduce bugs/regressions and keep moodle quality high ps: i can fix this up on friday and get it integrated soon - i ran out of time. Just wanted to let you know about 'peer-review' stage in the workflow - cheers!
        Hide
        Andreas Grabs added a comment -

        Hi Aparup,
        sorry, my english is not so good.
        If I understand you right I have to go to a button "peer-review". But I can't see such button. I did never need this before . Where can I do this?
        Andreas

        Show
        Andreas Grabs added a comment - Hi Aparup, sorry, my english is not so good. If I understand you right I have to go to a button "peer-review". But I can't see such button. I did never need this before . Where can I do this? Andreas
        Hide
        Aparup Banerjee added a comment - - edited

        Hi Andreas, please check with Helen or Michael (or MD?) - you should be able to see the peer-review button as a developer.

        edit: ah! the 'peer-review' option is only visible when the issue is in development. i'll push it along now to in-development so that you can see the 'peer-review'.

        Show
        Aparup Banerjee added a comment - - edited Hi Andreas, please check with Helen or Michael (or MD?) - you should be able to see the peer-review button as a developer. edit: ah! the 'peer-review' option is only visible when the issue is in development. i'll push it along now to in-development so that you can see the 'peer-review'.
        Hide
        Aparup Banerjee added a comment - - edited

        Andreas, its now in 'development in progress', you should be able to request 'peer-review' now. i can see it. i'll leave that bit to you to get familiar with

        Show
        Aparup Banerjee added a comment - - edited Andreas, its now in 'development in progress', you should be able to request 'peer-review' now. i can see it. i'll leave that bit to you to get familiar with
        Hide
        Daniele Cordella added a comment -

        Me? Was it a distraction?

        Show
        Daniele Cordella added a comment - Me? Was it a distraction?
        Hide
        Andreas Grabs added a comment -

        Ok, now I start peer review. But I do not know who I should select for "Peer reviewer".
        Andreas

        Show
        Andreas Grabs added a comment - Ok, now I start peer review. But I do not know who I should select for "Peer reviewer". Andreas
        Hide
        Aparup Banerjee added a comment - - edited

        Gal that its working for you Andreas

        it will now appear in a list for HQ to review (its optional to assign a peer-reviewer)

        Show
        Aparup Banerjee added a comment - - edited Gal that its working for you Andreas it will now appear in a list for HQ to review (its optional to assign a peer-reviewer)
        Hide
        Andreas Grabs added a comment -

        Hi Aparup,

        there is still another question. Why do you say it is not a bug? Anybody who use an editor is expecting this function. I think it is a very important thing and should not wait until 2.3 is coming out. At least for 2.2 it should be go in.

        Best regards Andreas

        Show
        Andreas Grabs added a comment - Hi Aparup, there is still another question. Why do you say it is not a bug? Anybody who use an editor is expecting this function. I think it is a very important thing and should not wait until 2.3 is coming out. At least for 2.2 it should be go in. Best regards Andreas
        Hide
        Andreas Grabs added a comment -

        Hi,
        I would like to know whether there are some things I can/have to do so that these commits will be integrated.
        Best regards
        Andreas

        Show
        Andreas Grabs added a comment - Hi, I would like to know whether there are some things I can/have to do so that these commits will be integrated. Best regards Andreas
        Hide
        Aparup Banerjee added a comment -

        Hi Andreas, i'm sending this up for integration now.

        I'm sure its expected to be able to put up pictures in "Page after submit". I just meant it wasn't implemented at all, its great it will be in there soon!

        Show
        Aparup Banerjee added a comment - Hi Andreas, i'm sending this up for integration now. I'm sure its expected to be able to put up pictures in "Page after submit". I just meant it wasn't implemented at all, its great it will be in there soon!
        Hide
        Dan Poltawski added a comment -

        The pre-checker has found a few coding style problems:

        <description>
        This sections shows the coding style problems detected in the code by phpcs
        </description>
        <mess>
        <problem file="mod/feedback/lib.php" linefrom="736" lineto="736" method="" class="" package="" api="" ruleset="Squiz" rule="Commenting.DocCommentAlignment.SpaceBeforeTag" url="http://docs.moodle.org/dev/Coding_style" weight="5">
        <message>Expected 1 space between asterisk and tag; 2 found</message>
        <description/>
        <code/>
        </problem>
        <problem file="mod/feedback/mod_form.php" linefrom="172" lineto="172" method="" class="" package="" api="" ruleset="Squiz" rule="WhiteSpace.SuperfluousWhitespace.EndLine" url="http://docs.moodle.org/dev/Coding_style" weight="5">
        <message>Whitespace found at end of line</message>
        <description/>
        <code/>
        </problem>
        <problem file="mod/feedback/mod_form.php" linefrom="178" lineto="178" method="" class="" package="" api="" ruleset="Squiz" rule="WhiteSpace.SuperfluousWhitespace.EndLine" url="http://docs.moodle.org/dev/Coding_style" weight="5">
        <message>Whitespace found at end of line</message>
        <description/>
        <code/>
        </problem>
        <problem file="mod/feedback/mod_form.php" linefrom="189" lineto="189" method="" class="" package="" api="" ruleset="moodle" rule="Files.LineLength.TooLong" url="http://docs.moodle.org/dev/Coding_style" weight="1">
        <message>
        Line exceeds 132 characters; contains 138 characters
        </message>
        <description/>
        <code/>
        </problem>
        
        Show
        Dan Poltawski added a comment - The pre-checker has found a few coding style problems: <description> This sections shows the coding style problems detected in the code by phpcs </description> <mess> <problem file= "mod/feedback/lib.php" linefrom= "736" lineto= "736" method= "" class=" " package =" " api=" " ruleset=" Squiz " rule=" Commenting.DocCommentAlignment.SpaceBeforeTag " url=" http: //docs.moodle.org/dev/Coding_style " weight=" 5"> <message>Expected 1 space between asterisk and tag; 2 found</message> <description/> <code/> </problem> <problem file= "mod/feedback/mod_form.php" linefrom= "172" lineto= "172" method= "" class=" " package =" " api=" " ruleset=" Squiz " rule=" WhiteSpace.SuperfluousWhitespace.EndLine " url=" http: //docs.moodle.org/dev/Coding_style " weight=" 5"> <message>Whitespace found at end of line</message> <description/> <code/> </problem> <problem file= "mod/feedback/mod_form.php" linefrom= "178" lineto= "178" method= "" class=" " package =" " api=" " ruleset=" Squiz " rule=" WhiteSpace.SuperfluousWhitespace.EndLine " url=" http: //docs.moodle.org/dev/Coding_style " weight=" 5"> <message>Whitespace found at end of line</message> <description/> <code/> </problem> <problem file= "mod/feedback/mod_form.php" linefrom= "189" lineto= "189" method= "" class=" " package =" " api=" " ruleset=" moodle " rule=" Files.LineLength.TooLong " url=" http: //docs.moodle.org/dev/Coding_style " weight=" 1"> <message> Line exceeds 132 characters; contains 138 characters </message> <description/> <code/> </problem>
        Hide
        Andreas Grabs added a comment -

        Hi Dan,

        I have fixed the shown errors at master and 2.2.
        There still are some warnings but I think it makes no sense to shorten the related lines.

        Best regards
        Andreas

        Show
        Andreas Grabs added a comment - Hi Dan, I have fixed the shown errors at master and 2.2. There still are some warnings but I think it makes no sense to shorten the related lines. Best regards Andreas
        Hide
        Dan Poltawski added a comment -

        Hi Andreas,

        I notice that you are using itemid for this with the feedback id. Its not necessary to do this as the context of the filarea will link the item back to the feedback id in the main mod_feedback table.

        It'll be clearer if the itemid is not used as in this case its not necessary (the files are shared throughout the the whole feeback activity). So could you switch to not using the itemid. See the workfshop module for an example of that - with instructauthors.

        Thanks!
        dan

        Show
        Dan Poltawski added a comment - Hi Andreas, I notice that you are using itemid for this with the feedback id. Its not necessary to do this as the context of the filarea will link the item back to the feedback id in the main mod_feedback table. It'll be clearer if the itemid is not used as in this case its not necessary (the files are shared throughout the the whole feeback activity). So could you switch to not using the itemid. See the workfshop module for an example of that - with instructauthors. Thanks! dan
        Hide
        Andreas Grabs added a comment -

        Hi Dan,

        ok, now I understand it better a bit .
        I have fixed these things.

        Best regards
        Andreas

        Show
        Andreas Grabs added a comment - Hi Dan, ok, now I understand it better a bit . I have fixed these things. Best regards Andreas
        Hide
        Eloy Lafuente (stronk7) 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
        Eloy Lafuente (stronk7) 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
        Dan Poltawski added a comment -

        Thanks, that has been integrated now

        Show
        Dan Poltawski added a comment - Thanks, that has been integrated now
        Hide
        Rossiani Wijaya added a comment -

        This looks good

        Test passed.

        Show
        Rossiani Wijaya added a comment - This looks good Test passed.
        Hide
        Aparup Banerjee added a comment -

        The code here has been spread to upstream moodle repositories and mirrors for anyone to use .

        Closing, have a good weekend!

        Show
        Aparup Banerjee added a comment - The code here has been spread to upstream moodle repositories and mirrors for anyone to use . Closing, have a good weekend!

          People

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

            Dates

            • Created:
              Updated:
              Resolved: