Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 2.0.1
    • Fix Version/s: 2.0.4
    • Component/s: Wiki (2.x)
    • Testing Instructions:
      Hide

      To test:
      1. Adding a new file using 'Files' tab in wiki
      2. Create a page using nwiki or creole format
      3. Select the file you uploaded in step 1
      4. save the page, you will be able to see the image in wik

      Please note:
      You need to test it using Collaborative wiki and Individual Wiki with group mode on/off.

      Thanks.

      Show
      To test: 1. Adding a new file using 'Files' tab in wiki 2. Create a page using nwiki or creole format 3. Select the file you uploaded in step 1 4. save the page, you will be able to see the image in wik Please note: You need to test it using Collaborative wiki and Individual Wiki with group mode on/off. Thanks.
    • Affected Branches:
      MOODLE_20_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE
    • Pull Master Branch:
      s9_MDL-26392_wiki_files_master
    • Rank:
      16347

      Description

      As reported by Gisela Hillenbrand in http://moodle.org/mod/forum/discuss.php?d=168577

      I use the NWiki format. Clicking on the image icon in the wiki page edit mode results in the piece of code: [[image:Image|alt]] - so where can I upload a image.jpg file and link it with that Image in my wiki?

        Issue Links

          Activity

          Helen Foster created issue -
          Helen Foster made changes -
          Field Original Value New Value
          Link This issue will be resolved by MDL-20814 [ MDL-20814 ]
          Martin Dougiamas made changes -
          Fix Version/s STABLE Sprint 6 [ 10535 ]
          Fix Version/s STABLE backlog [ 10463 ]
          Dongsheng Cai made changes -
          Assignee moodle.com [ moodle.com ] Dongsheng Cai [ dongsheng ]
          Dongsheng Cai made changes -
          Status Open [ 1 ] In Progress [ 3 ]
          Hide
          Dongsheng Cai added a comment -

          Helen, we never support images for nwiki format, this is more like a new feature, not a blocker, I'm not sure if we should implement this in this sprint, I will check with Martin.

          Show
          Dongsheng Cai added a comment - Helen, we never support images for nwiki format, this is more like a new feature, not a blocker, I'm not sure if we should implement this in this sprint, I will check with Martin.
          Hide
          Martin Dougiamas added a comment -

          Can't be added in an existing sprint ...

          Show
          Martin Dougiamas added a comment - Can't be added in an existing sprint ...
          Martin Dougiamas made changes -
          Fix Version/s STABLE backlog [ 10463 ]
          Fix Version/s STABLE Sprint 6 [ 10535 ]
          Hide
          Martin Dougiamas added a comment -

          Sorry, I was confused. It was in the sprint already

          I guess image support is a whole new feature, yes. But the fact that the bogus image button is there is a bug, so that should be fixed.

          Show
          Martin Dougiamas added a comment - Sorry, I was confused. It was in the sprint already I guess image support is a whole new feature, yes. But the fact that the bogus image button is there is a bug, so that should be fixed.
          Martin Dougiamas made changes -
          Fix Version/s STABLE Sprint 6 [ 10535 ]
          Fix Version/s STABLE backlog [ 10463 ]
          Dongsheng Cai made changes -
          Status In Progress [ 3 ] Open [ 1 ]
          Hide
          Dongsheng Cai added a comment -

          Martin, the image button is not completely not working, it is just not working with the new file system, if you put a image url there (I means external image file), it will be displayed.

          Show
          Dongsheng Cai added a comment - Martin, the image button is not completely not working, it is just not working with the new file system, if you put a image url there (I means external image file), it will be displayed.
          Hide
          Martin Dougiamas added a comment -

          OK, so after discussion I think it's worth it to do this in stable:

          1) Add a filemanager form element to the nwiki editing form so that images (only, for now) can be uploaded
          2) Make sure the nwiki parser can securely convert [image:somefile.jpg] into a HTML img tag via pluginfile.php

          The larger problem about other file types needs to be thought about some other time. Potential here for people to start adding huge file collections and perhaps avoid quotas etc.

          Show
          Martin Dougiamas added a comment - OK, so after discussion I think it's worth it to do this in stable: 1) Add a filemanager form element to the nwiki editing form so that images (only, for now) can be uploaded 2) Make sure the nwiki parser can securely convert [image:somefile.jpg] into a HTML img tag via pluginfile.php The larger problem about other file types needs to be thought about some other time. Potential here for people to start adding huge file collections and perhaps avoid quotas etc.
          David Mudrak made changes -
          Link This issue has a non-specific relationship to MDL-25629 [ MDL-25629 ]
          Hide
          David Mudrak added a comment -

          I just added a link to MDL-25629 that is kind of similar issue, affecting all Moodle (not just single module).

          Show
          David Mudrak added a comment - I just added a link to MDL-25629 that is kind of similar issue, affecting all Moodle (not just single module).
          Dongsheng Cai made changes -
          Status Open [ 1 ] In Progress [ 3 ]
          Hide
          Dongsheng Cai added a comment -
          Show
          Dongsheng Cai added a comment - Adding Aparup here for peer review: https://github.com/dongsheng/moodle/commit/119e7d4884bfc13ef234280a8e18764b9628facc Thanks!
          Dongsheng Cai made changes -
          Link This issue will be resolved by PULL-377 [ PULL-377 ]
          Hide
          Aparup Banerjee added a comment -

          code looks good, saving works fine .

          need to fix the preview pages though as images aren't showing up there (ie: draft files)

          Show
          Aparup Banerjee added a comment - code looks good, saving works fine . need to fix the preview pages though as images aren't showing up there (ie: draft files)
          Hide
          Aparup Banerjee added a comment -

          Hi DS,
          the updated fix now works, draft files and saved files all showing up nice and well! it works for me!

          Show
          Aparup Banerjee added a comment - Hi DS, the updated fix now works, draft files and saved files all showing up nice and well! it works for me!
          Hide
          Dongsheng Cai added a comment -

          PULL request submitted.

          Show
          Dongsheng Cai added a comment - PULL request submitted.
          Dongsheng Cai made changes -
          Status In Progress [ 3 ] Ready for review [ 10010 ]
          Resolution Fixed [ 1 ]
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Hi Dong, plz, take a look to my comment (when testing) at:

          http://tracker.moodle.org/browse/PULL-377?focusedCommentId=105851&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-105851

          Feel free to reply there or here... TIA and ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Hi Dong, plz, take a look to my comment (when testing) at: http://tracker.moodle.org/browse/PULL-377?focusedCommentId=105851&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-105851 Feel free to reply there or here... TIA and ciao
          Martin Dougiamas made changes -
          Resolution Fixed [ 1 ]
          Status Ready for review [ 10010 ] Reopened [ 4 ]
          Martin Dougiamas made changes -
          Fix Version/s STABLE Sprint 7 [ 10540 ]
          Fix Version/s STABLE Sprint 6 [ 10535 ]
          Martin Dougiamas made changes -
          Link This issue is blocked by MDL-26739 [ MDL-26739 ]
          Martin Dougiamas made changes -
          Fix Version/s STABLE Sprint 7 [ 10540 ]
          Martin Dougiamas made changes -
          Fix Version/s STABLE backlog [ 10463 ]
          Hide
          Helen Foster added a comment -

          Linking to MDL-26782 as both issues are about images in a wiki.

          Show
          Helen Foster added a comment - Linking to MDL-26782 as both issues are about images in a wiki.
          Helen Foster made changes -
          Link This issue has been marked as being related by MDL-26782 [ MDL-26782 ]
          Martin Dougiamas made changes -
          Fix Version/s STABLE Sprint 8 [ 10541 ]
          Fix Version/s STABLE backlog [ 10463 ]
          Dongsheng Cai made changes -
          Parent MDL-26739 [ 42833 ]
          Issue Type Bug [ 1 ] Sub-task [ 5 ]
          Hide
          Ludo ( Marc Alier) added a comment -

          Hi,
          you guys did not like the way it was code the file uploading so you commented all the lines. If you provide some insight on how to do it we may add the feature again.
          L.

          Show
          Ludo ( Marc Alier) added a comment - Hi, you guys did not like the way it was code the file uploading so you commented all the lines. If you provide some insight on how to do it we may add the feature again. L.
          Martin Dougiamas made changes -
          Fix Version/s STABLE Sprint 9 [ 10550 ]
          Fix Version/s STABLE Sprint 8 [ 10541 ]
          Hide
          Dongsheng Cai added a comment -

          Martin,

          Can you please review my changes?

          repo: git@github.com:dongsheng/moodle.git
          Branch: s8_MDL-26392_wiki_files
          diff: https://github.com/dongsheng/moodle/compare/master...s8_MDL-26392_wiki_files

          Show
          Dongsheng Cai added a comment - Martin, Can you please review my changes? repo: git@github.com:dongsheng/moodle.git Branch: s8_ MDL-26392 _wiki_files diff: https://github.com/dongsheng/moodle/compare/master...s8_MDL-26392_wiki_files
          Dongsheng Cai made changes -
          Status Reopened [ 4 ] In Progress [ 3 ]
          Dongsheng Cai made changes -
          Link This issue will be resolved by PULL-613 [ PULL-613 ]
          Dongsheng Cai made changes -
          Link This issue will be resolved by PULL-612 [ PULL-612 ]
          Hide
          Dongsheng Cai added a comment -

          PULL requests submitted

          Show
          Dongsheng Cai added a comment - PULL requests submitted
          Dongsheng Cai made changes -
          Status In Progress [ 3 ] Ready for review [ 10010 ]
          Resolution Fixed [ 1 ]
          Hide
          Petr Škoda added a comment -

          Reopening, see PULL for details.

          Show
          Petr Škoda added a comment - Reopening, see PULL for details.
          Petr Škoda made changes -
          Resolution Fixed [ 1 ]
          Status Ready for review [ 10010 ] Reopened [ 4 ]
          Martin Dougiamas made changes -
          Fix Version/s STABLE Sprint 10 [ 10551 ]
          Fix Version/s STABLE Sprint 9 [ 10550 ]
          Martin Dougiamas made changes -
          Workflow MDL Workflow [ 67733 ] MDL Full Workflow [ 76866 ]
          Dongsheng Cai made changes -
          Status Reopened [ 4 ] Development in progress [ 3 ]
          Hide
          Dongsheng Cai added a comment -

          1/ require_course_login() is not enough when modifying any text or file - luckily since 2.0 guests and not-logged-in do not have any write caps, but I think we should use the require_login() x require_course_login() correctly

          2/ you can not bump version above the later branch 2011040500, it has to use single +1 increments now that we branched

          3/ each file is using different "@package mod-wiki-2.0"

          4/ why can not students manage files by default?

          5/ please use s() when adding any html element attributes - such as in: $html .= "<img src=\"$CFG->wwwroot/mod/wiki/editors/wiki/images/$button[0]\" alt=\"".$button[1]."\" title=\"".$button[1]."\" />";

          6/ do not make empty line after initial "<?php" - it helps some IDEs a lot

          7/ $groupanduser = optional_param('groupanduser', 0, PARAM_TEXT); is definitely very bad coding style

          8/ mod/wiki/filesedit.php accepts both pageid and subwiki id, why? you can derive the subwiki from the pageid, but if you have both you need to verify they match - you must not use them separately without this validation; the same problem might be in other files

          9/ mod/wiki/filesedit.php why are all parameters optional? That does not make much sense to me.

          Show
          Dongsheng Cai added a comment - 1/ require_course_login() is not enough when modifying any text or file - luckily since 2.0 guests and not-logged-in do not have any write caps, but I think we should use the require_login() x require_course_login() correctly 2/ you can not bump version above the later branch 2011040500, it has to use single +1 increments now that we branched 3/ each file is using different "@package mod-wiki-2.0" 4/ why can not students manage files by default? 5/ please use s() when adding any html element attributes - such as in: $html .= "<img src=\"$CFG->wwwroot/mod/wiki/editors/wiki/images/$button [0] \" alt=\"".$button [1] ."\" title=\"".$button [1] ."\" />"; 6/ do not make empty line after initial "<?php" - it helps some IDEs a lot 7/ $groupanduser = optional_param('groupanduser', 0, PARAM_TEXT); is definitely very bad coding style 8/ mod/wiki/filesedit.php accepts both pageid and subwiki id, why? you can derive the subwiki from the pageid, but if you have both you need to verify they match - you must not use them separately without this validation; the same problem might be in other files 9/ mod/wiki/filesedit.php why are all parameters optional? That does not make much sense to me.
          Hide
          Dongsheng Cai added a comment -

          1. replaced all require_course_login with reqire_login
          2. fixed
          3. all files use '@package mod-wiki-2.0' now
          4. Martin agreed to not allowing student manage files by default
          5. fixed by using html_writer:empty_tag
          6. What IDEs are you talking about? And what's the benefits? I removed empty lines from the files I newly created, but most moodle files have that empty line.
          7. It's actually an existing problem of wiki 2, there is a subwiki selector created in render.php, it use groupid-userid as parameter for individual wiki in group mode, it affects other places as well, I am not sure if we should fix this render function in this issue, I added clean_param for now
          8. the page id only used for navigation, it's not for file management, if I don't pass it, users won't be able to go back to the page they came from.
          9. Fixed.

          Show
          Dongsheng Cai added a comment - 1. replaced all require_course_login with reqire_login 2. fixed 3. all files use '@package mod-wiki-2.0' now 4. Martin agreed to not allowing student manage files by default 5. fixed by using html_writer:empty_tag 6. What IDEs are you talking about? And what's the benefits? I removed empty lines from the files I newly created, but most moodle files have that empty line. 7. It's actually an existing problem of wiki 2, there is a subwiki selector created in render.php, it use groupid-userid as parameter for individual wiki in group mode, it affects other places as well, I am not sure if we should fix this render function in this issue, I added clean_param for now 8. the page id only used for navigation, it's not for file management, if I don't pass it, users won't be able to go back to the page they came from. 9. Fixed.
          Dongsheng Cai made changes -
          Status Development in progress [ 3 ] Waiting for peer review [ 10012 ]
          Pull Master Diff URL https://github.com/dongsheng/moodle/compare/master...s9_MDL-26392_wiki_files_master
          Pull Master Branch s9_MDL-26392_wiki_files_master
          Pull 2.0 Diff URL https://github.com/dongsheng/moodle/compare/MOODLE_20_STABLE...s9_MDL-26392_wiki_files_20
          Pull 2.0 Branch s9_MDL-26392_wiki_files_20
          Pull from Repository git://github.com/dongsheng/moodle.git
          Peer reviewer rwijaya
          Testing Instructions To test:
          1. Adding a new file using 'Files' tab in wiki
          2. Create a page using nwiki or creole format
          3. Select the file you uploaded in step 1
          4. save the page, you will be able to see the image in wik

          Please note:
          You need to test it using Collaborative wiki and Individual Wiki with group mode on/off.

          Thanks.
          Hide
          Andrew Davis added a comment -

          Is it worth retaining the commented out code in mod/wiki/filesedit_form.php starting at line 45?

          Show
          Andrew Davis added a comment - Is it worth retaining the commented out code in mod/wiki/filesedit_form.php starting at line 45?
          Hide
          Dongsheng Cai added a comment -

          @Andrew

          Maybe not, I just removed it, two branches got updated.

          Show
          Dongsheng Cai added a comment - @Andrew Maybe not, I just removed it, two branches got updated.
          Hide
          Jérôme Mouneyrac added a comment -

          My peer review:
          It works. +1 for integration. I understood that some part of this code will be refactored later, so it's why I suggest to integrate it and to create issues for things to do later.

          I would suggest to write these issues:

          • some help information should explain clearly how to change the code [[image:Image|alt]]. Specially as clicking on the image button or selecting 'Insert an image...' would generate this code (a little bit buggy).
          • imo, for later, the button image should call the file picker and directly write the correct wiki code (as you would expect of anything related to file from a user point of view)
          Show
          Jérôme Mouneyrac added a comment - My peer review: It works. +1 for integration. I understood that some part of this code will be refactored later, so it's why I suggest to integrate it and to create issues for things to do later. I would suggest to write these issues: some help information should explain clearly how to change the code [ [image:Image|alt] ]. Specially as clicking on the image button or selecting 'Insert an image...' would generate this code (a little bit buggy). imo, for later, the button image should call the file picker and directly write the correct wiki code (as you would expect of anything related to file from a user point of view)
          Jérôme Mouneyrac made changes -
          Original Estimate 0 minutes [ 0 ]
          Remaining Estimate 0 minutes [ 0 ]
          Status Waiting for peer review [ 10012 ] Development in progress [ 3 ]
          Jérôme Mouneyrac made changes -
          Status Development in progress [ 3 ] Waiting for integration review [ 10010 ]
          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
          Dongsheng Cai added a comment -

          Rebased

          Regards,
          Dongsheng

          Show
          Dongsheng Cai added a comment - Rebased Regards, Dongsheng
          Eloy Lafuente (stronk7) made changes -
          Labels triaged ci pullweek-2011-19
          Sam Hemelryk made changes -
          Status Waiting for integration review [ 10010 ] Integration review in progress [ 10004 ]
          Integrator samhemelryk
          Hide
          Sam Hemelryk added a comment -

          Hmmmm I've just been reviewing this now and there are a couple of things I am not 100% sure about.

          1. The files link/tab is shown to everyone regardless of whether they can actually view the page.
          2. You are able to delete files that are still in use leading to broken images on pages where they were used.
          3. There is no way to access files from the HTML editor, I understand this isn't easy but it confused me while I was learning these changes.
          4. files.php - shouldn't pageid be a required param rather than optional given that the default 0 results in an exception anyway.
          5. filesedit.php displays all the tabs regardless of what the user can see/access (e.g. as a guest I am shown the edit tab).

          I also noted the following (although I wouldn't expect them to be fixed as part of this issue).

          1. The navigation is VERY confused for the wiki, especially how wiki_extend_navigation attempts to collect pageid and/or id from the URL... I think a better navigation solution could be found for the wiki module.
          2. The wiki still has inline JS calls (onchange) - these certainly need upgrading to make use of the page requirements API.
          3. There is a bug with the navigation when logged in as a guest and viewing a front page course module.
          4. The package phpdoc tag needs to be standardised and used consistently - there are still a couple of variations of the @package tag within the wiki module.

          Most of the points I have spotted are very minor or may be things that have already discussed with other reviewers, I'll wait to discuss this with Eloy before making a decision about integration.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hmmmm I've just been reviewing this now and there are a couple of things I am not 100% sure about. The files link/tab is shown to everyone regardless of whether they can actually view the page. You are able to delete files that are still in use leading to broken images on pages where they were used. There is no way to access files from the HTML editor, I understand this isn't easy but it confused me while I was learning these changes. files.php - shouldn't pageid be a required param rather than optional given that the default 0 results in an exception anyway. filesedit.php displays all the tabs regardless of what the user can see/access (e.g. as a guest I am shown the edit tab). I also noted the following (although I wouldn't expect them to be fixed as part of this issue). The navigation is VERY confused for the wiki, especially how wiki_extend_navigation attempts to collect pageid and/or id from the URL... I think a better navigation solution could be found for the wiki module. The wiki still has inline JS calls (onchange) - these certainly need upgrading to make use of the page requirements API. There is a bug with the navigation when logged in as a guest and viewing a front page course module. The package phpdoc tag needs to be standardised and used consistently - there are still a couple of variations of the @package tag within the wiki module. Most of the points I have spotted are very minor or may be things that have already discussed with other reviewers, I'll wait to discuss this with Eloy before making a decision about integration. Cheers Sam
          Hide
          Martin Dougiamas added a comment -

          A1, A4 and A5 should be fixed for sure. +1 to reject based on those. The others are probably other bugs.

          Show
          Martin Dougiamas added a comment - A1, A4 and A5 should be fixed for sure. +1 to reject based on those. The others are probably other bugs.
          Hide
          Sam Hemelryk added a comment -

          Hi DS,

          I've just been talking this issue over with Martin and Eloy, its being rejected this week so that we can clean a couple of those things up and discuss the HTML editor and file deletion points if need be.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi DS, I've just been talking this issue over with Martin and Eloy, its being rejected this week so that we can clean a couple of those things up and discuss the HTML editor and file deletion points if need be. Cheers Sam
          Sam Hemelryk made changes -
          Status Integration review in progress [ 10004 ] Open [ 1 ]
          Eloy Lafuente (stronk7) made changes -
          Status Open [ 1 ] Closed [ 6 ]
          Resolution Fixed [ 1 ]
          Eloy Lafuente (stronk7) made changes -
          Resolution Fixed [ 1 ]
          Status Closed [ 6 ] Reopened [ 4 ]
          Martin Dougiamas made changes -
          Fix Version/s Sprint to 2.1 [ 10650 ]
          Fix Version/s STABLE Sprint 10 [ 10551 ]
          Eloy Lafuente (stronk7) made changes -
          Labels ci pullweek-2011-19 pullweek-2011-19
          Hide
          Dongsheng Cai added a comment -

          Sam, Thanks for reviewing.

          A1. Fixed, user will need viewpage capability to view files
          A4. Fixed, pageid now being required
          A5. you are saying files.php right? filesedit.php doesn't display tabs, I added capability checks to tabs render.

          Regards,
          Dongsheng

          Show
          Dongsheng Cai added a comment - Sam, Thanks for reviewing. A1. Fixed, user will need viewpage capability to view files A4. Fixed, pageid now being required A5. you are saying files.php right? filesedit.php doesn't display tabs, I added capability checks to tabs render. Regards, Dongsheng
          Dongsheng Cai made changes -
          Status Reopened [ 4 ] Development in progress [ 3 ]
          Dongsheng Cai made changes -
          Status Development in progress [ 3 ] Waiting for peer review [ 10012 ]
          Hide
          Rossiani Wijaya added a comment -

          Hi Dongsheng,

          I tested both patches and received the following messages:

          Notice: Undefined variable: newcontent in /m20/moodle/mod/wiki/edit.php on line 88

          Notice: Undefined variable: context in /m20/moodle/mod/wiki/pagelib.php on line 1916

          Notice: Trying to get property of non-object in /m20/moodle/mod/wiki/pagelib.php on line 1916

          To reproduce:
          1. create new wiki (either creole or nwiki)
          2. upload img file
          3. add uploaded image on edit page and save.

          also notice mod/wiki/edit.php on line 48:
          [code]
          $newconent = '';
          if (!empty($newcontent) && is_array($newcontent))

          { $newcontent = $newcontent['text']; }

          [/code]
          Variable $newconent, is this a typo?
          if it is, then what is the use of if statement for?

          Show
          Rossiani Wijaya added a comment - Hi Dongsheng, I tested both patches and received the following messages: Notice: Undefined variable: newcontent in /m20/moodle/mod/wiki/edit.php on line 88 Notice: Undefined variable: context in /m20/moodle/mod/wiki/pagelib.php on line 1916 Notice: Trying to get property of non-object in /m20/moodle/mod/wiki/pagelib.php on line 1916 To reproduce: 1. create new wiki (either creole or nwiki) 2. upload img file 3. add uploaded image on edit page and save. also notice mod/wiki/edit.php on line 48: [code] $newconent = ''; if (!empty($newcontent) && is_array($newcontent)) { $newcontent = $newcontent['text']; } [/code] Variable $newconent, is this a typo? if it is, then what is the use of if statement for?
          Hide
          Dongsheng Cai added a comment -

          Rossiani
          Those errors are not from this patch, can you please file a new bug for it?

          Show
          Dongsheng Cai added a comment - Rossiani Those errors are not from this patch, can you please file a new bug for it?
          Hide
          Rossiani Wijaya added a comment -

          File a new issue for the errors I described above: MDL-27525

          Show
          Rossiani Wijaya added a comment - File a new issue for the errors I described above: MDL-27525
          Hide
          Rossiani Wijaya added a comment -

          Hi DS,

          Apart from the errors I mentioned above, adding images in nwiki and creole works great.

          Note: to test this issue, I set 'debug messages' to none.

          Show
          Rossiani Wijaya added a comment - Hi DS, Apart from the errors I mentioned above, adding images in nwiki and creole works great. Note: to test this issue, I set 'debug messages' to none.
          Rossiani Wijaya made changes -
          Status Waiting for peer review [ 10012 ] Development in progress [ 3 ]
          Hide
          Dongsheng Cai added a comment -

          Thanks Rossiani

          I just assigned MDL-27525 myself.

          Show
          Dongsheng Cai added a comment - Thanks Rossiani I just assigned MDL-27525 myself.
          Eloy Lafuente (stronk7) made changes -
          Integration date 11/May/11
          Hide
          Dongsheng Cai added a comment -

          Rossiani did you clicked "looks great to me" after peer review, this issue rolled back to development in progress

          Show
          Dongsheng Cai added a comment - Rossiani did you clicked "looks great to me" after peer review, this issue rolled back to development in progress
          Dongsheng Cai made changes -
          Status Development in progress [ 3 ] Waiting for peer review [ 10012 ]
          Hide
          Rossiani Wijaya added a comment -

          Ds,

          I did click on "looks great to me" button after I reviewed it.

          Show
          Rossiani Wijaya added a comment - Ds, I did click on "looks great to me" button after I reviewed it.
          Hide
          Dongsheng Cai added a comment -

          Hi Rossiani

          Aparup just explained to me, either button will send issue back to in progress status, we need to click "send for integration".

          Show
          Dongsheng Cai added a comment - Hi Rossiani Aparup just explained to me, either button will send issue back to in progress status, we need to click "send for integration".
          Eloy Lafuente (stronk7) made changes -
          Integration date 11/May/11
          Hide
          Rossiani Wijaya added a comment -

          Hi DS,

          This is look great. I'm sending this to integration.

          Show
          Rossiani Wijaya added a comment - Hi DS, This is look great. I'm sending this to integration.
          Rossiani Wijaya made changes -
          Status Waiting for peer review [ 10012 ] Waiting for integration review [ 10010 ]
          Hide
          Dongsheng Cai added a comment -

          rebased

          Show
          Dongsheng Cai added a comment - rebased
          Sam Hemelryk made changes -
          Currently in integration Yes
          Hide
          Sam Hemelryk added a comment -

          Hi Dongsheng,

          I've been reviewing this now for integration.
          All in all it looks good, I did spot several minor things but I think these could be cleaned up during integration and I've attached a patch to see them cleaned up.

          Once I've checked with Eloy that it is OK for things to be sent upstream this can go in.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi Dongsheng, I've been reviewing this now for integration. All in all it looks good, I did spot several minor things but I think these could be cleaned up during integration and I've attached a patch to see them cleaned up. Once I've checked with Eloy that it is OK for things to be sent upstream this can go in. Cheers Sam
          Sam Hemelryk made changes -
          Attachment MDL.26739.integration.patch [ 24023 ]
          Hide
          Sam Hemelryk added a comment -

          Forgot to mention what I spotted!

          Minor things to be corrected:

          • Two new strings insertimage and insertimage_help need to be added in alphabetically.
          • wiki_print_subwiki_selector $pagetype should default to 'view' and $baseurl is assumed a moodle_url and should never default to null.
          • wiki_files_tree should be declared public as per coding style
          • wiki_parser_real_path arguments have changes but PHPdoc hasn't been updated.

          Questions I have for you DS that I needed answered before this can be integrated:

          • Is the new string cannotmanagefiles used any where/needed?
          • Is the new string insertimage_help used any where/needed?

          And general notes:

          • Test on frontpage wiki module and check access when not logged in.
          • HTML generation through $OUTPUT should be moved out of loops where possible to aid page generation time, see mod_wiki_renderer::htmllize_tree (the pix_icon calls).
          Show
          Sam Hemelryk added a comment - Forgot to mention what I spotted! Minor things to be corrected: Two new strings insertimage and insertimage_help need to be added in alphabetically. wiki_print_subwiki_selector $pagetype should default to 'view' and $baseurl is assumed a moodle_url and should never default to null. wiki_files_tree should be declared public as per coding style wiki_parser_real_path arguments have changes but PHPdoc hasn't been updated. Questions I have for you DS that I needed answered before this can be integrated: Is the new string cannotmanagefiles used any where/needed? Is the new string insertimage_help used any where/needed? And general notes: Test on frontpage wiki module and check access when not logged in. HTML generation through $OUTPUT should be moved out of loops where possible to aid page generation time, see mod_wiki_renderer::htmllize_tree (the pix_icon calls).
          Sam Hemelryk made changes -
          Status Waiting for integration review [ 10010 ] Integration review in progress [ 10004 ]
          Hide
          Sam Hemelryk added a comment -

          Thanks Dongsheng this has been integrated now... Hoorah

          Show
          Sam Hemelryk added a comment - Thanks Dongsheng this has been integrated now... Hoorah
          Sam Hemelryk made changes -
          Status Integration review in progress [ 10004 ] Waiting for testing [ 10005 ]
          Eloy Lafuente (stronk7) made changes -
          Status Waiting for testing [ 10005 ] Testing in progress [ 10011 ]
          Tester stronk7
          Hide
          Eloy Lafuente (stronk7) added a comment -

          passing based one Rossiani's comments.

          Show
          Eloy Lafuente (stronk7) added a comment - passing based one Rossiani's comments.
          Eloy Lafuente (stronk7) made changes -
          Status Testing in progress [ 10011 ] Tested [ 10006 ]
          Hide
          Eloy Lafuente (stronk7) added a comment -

          This is now upstream, yay! Many thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - This is now upstream, yay! Many thanks!
          Eloy Lafuente (stronk7) made changes -
          Status Tested [ 10006 ] Closed [ 6 ]
          Fix Version/s 2.0.4 [ 10652 ]
          Resolution Fixed [ 1 ]
          Currently in integration Yes
          Integration date 9/Jun/11
          Eloy Lafuente (stronk7) made changes -
          Fix Version/s DEV Sprint 2.1 [ 10650 ]

            People

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

              Dates

              • Created:
                Updated:
                Resolved: