Moodle
  1. Moodle
  2. MDL-31708

wiki 2.0 non-group member can see edit option in navigation block.

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.0.7, 2.1.4, 2.2.1, 2.2.5, 2.3.2, 2.4
    • Fix Version/s: 2.2.6, 2.3.3
    • Component/s: Wiki (2.x)
    • Labels:
    • Rank:
      38295

      Description

      NOTE to integrators: This cannot be integrated until we get its blockers fixed (MDL-30478), so groups are working and this can be tested.

      Gilles commented and attached a screenshot in MDL-27037 - it fixed a regression by taking away the 'edit' tab for wiki when a non-group member didn't have edit access to the wiki.

      Giless mentioned that it seems to have left out removing the 'edit' link @ block:navigation->...>wiki>edit .

        Issue Links

          Activity

          Hide
          Aparup Banerjee added a comment -

          Assigning , maybe Mayank can quickly patch up his old patch

          Show
          Aparup Banerjee added a comment - Assigning , maybe Mayank can quickly patch up his old patch
          Show
          Gilles-Philippe Leblanc added a comment - Here are the fix for the following branches: Master: https://github.com/StudiUM/moodle/compare/master...MDL-31708_nongroup_member_edit_navigation_block MOODLE_21_STABLE: https://github.com/StudiUM/moodle/compare/MOODLE_21_STABLE...MDL-31708_nongroup_member_edit_navigation_block_moodle21 MOODLE_22_STABLE: https://github.com/StudiUM/moodle/compare/MOODLE_22_STABLE...MDL-31708_nongroup_member_edit_navigation_block_moodle22 I add in this fix a some hardcoded string fix too.
          Hide
          Mayank Gupta added a comment -

          Looks like you have already fixed it. I was going to give it a shot over the weekend, had a busy week at school.
          @Aparup: Can I peer review it (am I allowed to)?

          Show
          Mayank Gupta added a comment - Looks like you have already fixed it. I was going to give it a shot over the weekend, had a busy week at school. @Aparup: Can I peer review it (am I allowed to)?
          Hide
          Mayank Gupta added a comment - - edited

          @Gilles: Just had a quick look - do we really need the strings (Not sure!)?

          [edit] -
          We actually need to. It was a TODO.
          Thanks!

          Show
          Mayank Gupta added a comment - - edited @Gilles: Just had a quick look - do we really need the strings (Not sure!)? [edit] - We actually need to. It was a TODO. Thanks!
          Hide
          Gilles-Philippe Leblanc added a comment - - edited

          Well... no we don't really need it specifically for the fix of this task but our University is not a English one so we need the internationalize the most possible so we said...why not ?

          We feel that this kind of fix is never treated separately, because this would most likely overkill to create a task just for that. We took advantage of the occasion of this task to include this correction.

          This allows our code to be as much as possible in harmony with one of the Moodle community.

          However, Aparup can choose to include it or not. its was indeed a @TODO.

          Show
          Gilles-Philippe Leblanc added a comment - - edited Well... no we don't really need it specifically for the fix of this task but our University is not a English one so we need the internationalize the most possible so we said...why not ? We feel that this kind of fix is never treated separately, because this would most likely overkill to create a task just for that. We took advantage of the occasion of this task to include this correction. This allows our code to be as much as possible in harmony with one of the Moodle community. However, Aparup can choose to include it or not. its was indeed a @TODO.
          Hide
          Aparup Banerjee added a comment - - edited

          pushing up for integration , filled in git repo info, setting peer-reviewer and assignee (oh Gilles, i couldn't find you on the developer list) here too.

          Mayank: Peer reviews are always welcome , here is a recent growing checklist.
          http://docs.moodle.org/dev/Peer_reviewing_checklist

          Show
          Aparup Banerjee added a comment - - edited pushing up for integration , filled in git repo info, setting peer-reviewer and assignee (oh Gilles, i couldn't find you on the developer list) here too. Mayank: Peer reviews are always welcome , here is a recent growing checklist. http://docs.moodle.org/dev/Peer_reviewing_checklist
          Hide
          Gilles-Philippe Leblanc added a comment -

          Glad to see this work progress.
          Aparup: I do not think this list officially. How could I be added?

          Show
          Gilles-Philippe Leblanc added a comment - Glad to see this work progress. Aparup: I do not think this list officially. How could I be added?
          Hide
          Aparup Banerjee added a comment -

          That's a good question Gilles, I've added MD and Helen too - maybe they can help add you. (or feel free to contact them)

          I know there was a cvs process previously which was kicked off by a 'CVS access request form' but since we've moved to git i can't find a doc on how to kick off that process (for tracker especially).

          surely CVS_for_developers doc is so obsolete that we need one for developers using git.

          ah wow, Git_for_developers doesn't have a section "1 Joining the project as a developer" like the old cvs doc has.

          Show
          Aparup Banerjee added a comment - That's a good question Gilles, I've added MD and Helen too - maybe they can help add you. (or feel free to contact them) I know there was a cvs process previously which was kicked off by a 'CVS access request form' but since we've moved to git i can't find a doc on how to kick off that process (for tracker especially). surely CVS_for_developers doc is so obsolete that we need one for developers using git. ah wow, Git_for_developers doesn't have a section "1 Joining the project as a developer" like the old cvs doc has.
          Hide
          Michael de Raadt added a comment -

          Hi, Gilles and all.

          It used to be necessary for all CONTRIB developers to have CVS access, but with the new Plugins repository, we no longer need to offer CVS access or equivalent access as plugins are submitted as zip files. Developers can have their own Git repositories and work with this if they wish to, but this is not absolutely necessary.

          In terms of access to the tracker as a "jira-developer" (someone with the ability to edit and resolve an issue), we are willing to offer this to all CONTRIB developers and people who have demonstrated an ability to put forward patches for CORE issues. I have offered this level to a few people and some people have asked for it. We don't have a formal process in place for this, nor any documentation that describes this level of access (as far as I know). We don't want to hand out this access too freely, but we don't want to prevent people from becoming involved.

          Anyway, the short version is that I have added you to the jira-developers group and set you as the assignee for this issue, so you can get credit for the work you have done.

          Show
          Michael de Raadt added a comment - Hi, Gilles and all. It used to be necessary for all CONTRIB developers to have CVS access, but with the new Plugins repository, we no longer need to offer CVS access or equivalent access as plugins are submitted as zip files. Developers can have their own Git repositories and work with this if they wish to, but this is not absolutely necessary. In terms of access to the tracker as a "jira-developer" (someone with the ability to edit and resolve an issue), we are willing to offer this to all CONTRIB developers and people who have demonstrated an ability to put forward patches for CORE issues. I have offered this level to a few people and some people have asked for it. We don't have a formal process in place for this, nor any documentation that describes this level of access (as far as I know). We don't want to hand out this access too freely, but we don't want to prevent people from becoming involved. Anyway, the short version is that I have added you to the jira-developers group and set you as the assignee for this issue, so you can get credit for the work you have done.
          Hide
          Gilles-Philippe Leblanc added a comment -

          Thank you Michael.

          Show
          Gilles-Philippe Leblanc added a comment - Thank you Michael.
          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
          Gilles-Philippe Leblanc added a comment -

          The pull branches have been updated.

          Show
          Gilles-Philippe Leblanc added a comment - The pull branches have been updated.
          Hide
          Aparup Banerjee added a comment -

          Thanks for the nice work here! This is up in integration.git (master, 22, 21) and is ready for testing there.

          Show
          Aparup Banerjee added a comment - Thanks for the nice work here! This is up in integration.git (master, 22, 21) and is ready for testing there.
          Hide
          Adrian Greeve added a comment -

          I can't test this at the moment. MDL-30478 is preventing me from following the test instructions, so I can't tell if this patch works or not.

          Show
          Adrian Greeve added a comment - I can't test this at the moment. MDL-30478 is preventing me from following the test instructions, so I can't tell if this patch works or not.
          Hide
          Adrian Greeve added a comment -

          I tried testing this with master, 2.2, 2.1, a fresh install, and on another computer. I still have an issue where I can't change groups and see other contributions. Until this issue is fixed I can't test this patch.

          Sorry.

          Show
          Adrian Greeve added a comment - I tried testing this with master, 2.2, 2.1, a fresh install, and on another computer. I still have an issue where I can't change groups and see other contributions. Until this issue is fixed I can't test this patch. Sorry.
          Hide
          Gilles-Philippe Leblanc added a comment -

          Are the problem mentioned is related to MDL-30478 or its the current patch who cause the problem?

          Show
          Gilles-Philippe Leblanc added a comment - Are the problem mentioned is related to MDL-30478 or its the current patch who cause the problem?
          Hide
          Adrian Greeve added a comment -

          Yes, I still have the same problem before this patch, so it isn't to do with this code.

          Show
          Adrian Greeve added a comment - Yes, I still have the same problem before this patch, so it isn't to do with this code.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Hi Gilles (et all),

          I'm going to revert this fix in the git repository and reopen it. While the fix looks ok, we haven't been able to test this nor determine if the problems mentioned are caused by this or no. That and the imminent release of new stable versions cause us to take the safer way, knowing that the bug will be still present.

          I hope that the issue blocking this (MDL-30478, and their linked ones) will be added to some stable sprint soon, so this issue will be able to land safely.

          Many thanks for your support and patience, ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Hi Gilles (et all), I'm going to revert this fix in the git repository and reopen it. While the fix looks ok, we haven't been able to test this nor determine if the problems mentioned are caused by this or no. That and the imminent release of new stable versions cause us to take the safer way, knowing that the bug will be still present. I hope that the issue blocking this ( MDL-30478 , and their linked ones) will be added to some stable sprint soon, so this issue will be able to land safely. Many thanks for your support and patience, ciao
          Hide
          Gilles-Philippe Leblanc added a comment -

          I've see that the MDL-30478 is now fixed. Can we continue on the integration of the current fix ?
          Thanks!

          Show
          Gilles-Philippe Leblanc added a comment - I've see that the MDL-30478 is now fixed. Can we continue on the integration of the current fix ? Thanks!
          Hide
          Aparup Banerjee added a comment -

          thanks for the heads up. sending this to integration.

          Show
          Aparup Banerjee added a comment - thanks for the heads up. sending this to integration.
          Hide
          Gilles-Philippe Leblanc added a comment - - edited

          I created new branch for code clarity and to fix some tabulation mistakes.

          Show
          Gilles-Philippe Leblanc added a comment - - edited I created new branch for code clarity and to fix some tabulation mistakes.
          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
          Gilles-Philippe Leblanc added a comment -

          The branches were rebased

          Show
          Gilles-Philippe Leblanc added a comment - The branches were rebased
          Hide
          Aparup Banerjee added a comment -

          Thanks thats been integrated now into 22, 23 and master.

          Show
          Aparup Banerjee added a comment - Thanks thats been integrated now into 22, 23 and master.
          Hide
          Adrian Greeve added a comment -

          Hi Apu,

          As mentioned before, once you include headings, a table of contents is created and edit tags appear next to the text. I hope that it is straight forward to fix.

          Show
          Adrian Greeve added a comment - Hi Apu, As mentioned before, once you include headings, a table of contents is created and edit tags appear next to the text. I hope that it is straight forward to fix.
          Hide
          Dan Poltawski added a comment -

          Hurray!

          You did it, congratulations! You have on Mojito credit to redeem after the release of Moodle 2.4

          Show
          Dan Poltawski added a comment - Hurray! You did it, congratulations! You have on Mojito credit to redeem after the release of Moodle 2.4

            People

            • Votes:
              4 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: