Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-31708

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

    Details

    • Type: Bug
    • Status: Closed
    • Priority: 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:

      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 .

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            nebgor Aparup Banerjee added a comment -

            Assigning , maybe Mayank can quickly patch up his old patch

            Show
            nebgor Aparup Banerjee added a comment - Assigning , maybe Mayank can quickly patch up his old patch
            Show
            leblangi 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_gupta2005 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_gupta2005 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_gupta2005 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_gupta2005 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
            leblangi 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
            leblangi 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
            nebgor 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
            nebgor 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
            leblangi 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
            leblangi 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
            nebgor 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
            nebgor 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
            salvetore 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
            salvetore 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
            leblangi Gilles-Philippe Leblanc added a comment -

            Thank you Michael.

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

            The pull branches have been updated.

            Show
            leblangi Gilles-Philippe Leblanc added a comment - The pull branches have been updated.
            Hide
            nebgor 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
            nebgor 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
            abgreeve 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
            abgreeve 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
            abgreeve 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
            abgreeve 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
            leblangi 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
            leblangi 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
            abgreeve 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
            abgreeve 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
            stronk7 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
            stronk7 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
            leblangi 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
            leblangi 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
            nebgor Aparup Banerjee added a comment -

            thanks for the heads up. sending this to integration.

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

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

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

            The branches were rebased

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

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

            Show
            nebgor Aparup Banerjee added a comment - Thanks thats been integrated now into 22, 23 and master.
            Hide
            abgreeve 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
            abgreeve 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
            poltawski 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
            poltawski 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:
                  Fix Release Date:
                  12/Nov/12