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

block_manager::add_blocks() ignores block weight parameter and offset

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.2.6, 2.3.3, 2.4, 2.7.5, 2.8.3, 2.9
    • Fix Version/s: 2.7.6, 2.8.4
    • Component/s: Blocks
    • Labels:
    • Testing Instructions:
      Hide
      1. Check the weight of the admin_bookmarks block (it will be 0):

        SELECT * FROM mdl_block_instances WHERE parentcontextid = 1 AND blockname = 'admin_bookmarks';
        

      2. Apply patch.
      3. Create new Moodle installation.
      4. Check the weight of the admin_bookmarks block again (it should be 2):

        SELECT * FROM mdl_block_instances WHERE parentcontextid = 1 AND blockname = 'admin_bookmarks';
        

      5. Add this line to config.php:

        $CFG->defaultblocks_override = 'participants,calendar_month:activity_modules,news_items,calendar_upcoming';
        

      6. Create new course
        1. Confirm participants block between navigation and settings blocks
      Show
      Check the weight of the admin_bookmarks block (it will be 0): SELECT * FROM mdl_block_instances WHERE parentcontextid = 1 AND blockname = 'admin_bookmarks'; Apply patch. Create new Moodle installation. Check the weight of the admin_bookmarks block again (it should be 2): SELECT * FROM mdl_block_instances WHERE parentcontextid = 1 AND blockname = 'admin_bookmarks'; Add this line to config.php: $CFG->defaultblocks_override = 'participants,calendar_month:activity_modules,news_items,calendar_upcoming'; Create new course Confirm participants block between navigation and settings blocks
    • Affected Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE, MOODLE_24_STABLE, MOODLE_27_STABLE, MOODLE_28_STABLE, MOODLE_29_STABLE
    • Fixed Branches:
      MOODLE_27_STABLE, MOODLE_28_STABLE
    • Pull from Repository:
    • Pull 2.7 Branch:
    • Pull 2.8 Branch:
    • Pull Master Branch:

      Description

      When you use the add_blocks() function, the weight (block positioning) is ignored in the two ways that it can be provided.

        Gliffy Diagrams

          Activity

          Hide
          jrchamp Jonathan Champ added a comment -

          Here is a patch that uses the initial weight from the weight parameter (fixes the admin_bookmarks positioning that Sam H. added in 2009 to blocks_add_default_system_blocks()) and allows you to customize the weighting offset between blocks so that default_block_override and similar lines can avoid conflicting with the navigation and settings default page blocks.

          Currently, when you create a new course with blocks specified for the left column, the order jumbles the first non-default block in with the two default blocks (navigation and settings).

          Show
          jrchamp Jonathan Champ added a comment - Here is a patch that uses the initial weight from the weight parameter (fixes the admin_bookmarks positioning that Sam H. added in 2009 to blocks_add_default_system_blocks()) and allows you to customize the weighting offset between blocks so that default_block_override and similar lines can avoid conflicting with the navigation and settings default page blocks. Currently, when you create a new course with blocks specified for the left column, the order jumbles the first non-default block in with the two default blocks (navigation and settings).
          Hide
          jrchamp Jonathan Champ added a comment -

          Another implementation that I had considered was to set the weight function parameter to null and only if it was set to a non-null value would I specify the weight. Otherwise, if it were possible to simple append the blocks to the page region, that seems like the best bet. Unfortunately, the code from "add_block_at_end_of_default_region()" did not work the same way in the context of populating the initial page blocks during my testing.

          Show
          jrchamp Jonathan Champ added a comment - Another implementation that I had considered was to set the weight function parameter to null and only if it was set to a non-null value would I specify the weight. Otherwise, if it were possible to simple append the blocks to the page region, that seems like the best bet. Unfortunately, the code from "add_block_at_end_of_default_region()" did not work the same way in the context of populating the initial page blocks during my testing.
          Hide
          salvetore Michael de Raadt added a comment -

          Thanks for spotting that and providing a patch.

          It would be really helpful if you could add a set of testing steps.

          Show
          salvetore Michael de Raadt added a comment - Thanks for spotting that and providing a patch. It would be really helpful if you could add a set of testing steps.
          Hide
          salvetore Michael de Raadt added a comment -

          Sam: I've added you as a watcher on this issue as you seem to have worked on this code in the past. Perhaps you could have a look at the patch provided.

          Show
          salvetore Michael de Raadt added a comment - Sam: I've added you as a watcher on this issue as you seem to have worked on this code in the past. Perhaps you could have a look at the patch provided.
          Hide
          mikegrant Michael Grant added a comment -

          I've turned the patch into a branch for integration, tested it and everything appears to be working. Only fixing this as I tried to use add_blocks and setting the weight, and spending half a day tearing hair out to realise it was this. So did something about it. I don't have permissions to put this in the right place, but.

          Repository: https://github.com/mike-grant/moodle
          Pull 2.7 Branch: MDL-37470_27
          Pull 2.7 Diff URL:https://github.com/mike-grant/moodle/compare/MOODLE_27_STABLE...MDL-37470_27
          Pull 2.8 Branch:MDL-49041_28
          Pull 2.8 Diff URL:https://github.com/mike-grant/moodle/compare/MOODLE_28_STABLE...MDL-37470_28
          Pull Master Branch:MDL-49041
          Pull Master Diff URL:https://github.com/mike-grant/moodle/compare/master...MDL-37470

          Hoping that's all the information that's needed..

          Thanks

          Show
          mikegrant Michael Grant added a comment - I've turned the patch into a branch for integration, tested it and everything appears to be working. Only fixing this as I tried to use add_blocks and setting the weight, and spending half a day tearing hair out to realise it was this. So did something about it. I don't have permissions to put this in the right place, but. Repository: https://github.com/mike-grant/moodle Pull 2.7 Branch: MDL-37470 _27 Pull 2.7 Diff URL: https://github.com/mike-grant/moodle/compare/MOODLE_27_STABLE...MDL-37470_27 Pull 2.8 Branch: MDL-49041 _28 Pull 2.8 Diff URL: https://github.com/mike-grant/moodle/compare/MOODLE_28_STABLE...MDL-37470_28 Pull Master Branch: MDL-49041 Pull Master Diff URL: https://github.com/mike-grant/moodle/compare/master...MDL-37470 Hoping that's all the information that's needed.. Thanks
          Hide
          jrchamp Jonathan Champ added a comment -

          Thanks Mike! I'll try and push this up the chain.

          Show
          jrchamp Jonathan Champ added a comment - Thanks Mike! I'll try and push this up the chain.
          Hide
          cibot CiBoT added a comment -

          Code verified against automated checks.

          Checked MDL-37470 using repository: https://github.com/mike-grant/moodle

          More information about this report

          Show
          cibot CiBoT added a comment - Code verified against automated checks. Checked MDL-37470 using repository: https://github.com/mike-grant/moodle MOODLE_27_STABLE (0 errors / 0 warnings) [branch: MDL-37470_27 | CI Job ] MOODLE_28_STABLE (0 errors / 0 warnings) [branch: MDL-37470_28 | CI Job ] master (0 errors / 0 warnings) [branch: MDL-37470 | CI Job ] More information about this report
          Hide
          timhunt Tim Hunt added a comment -

          I am afraid that I can see several problems here:

          blocks_parse_default_blocks_list

          The changes in blocks_parse_default_blocks_list are quite subtle. It took me a while to work out what is going on. The point is that you can define a string like ',,,block_x,,,blocky' to mean block_x has weight 3, and block y has weight 6. However, how is anyone supposed to know that?

          • The format of the strings being parsed needs to be documented somewhere, probably in config-dist.php where this is used, and in the PHPdoc @param string $blocksstr.
          • The PHPdocs for the return-type of blocks_parse_default_blocks_list needs to explain the details of the array that is returned.

          Note that this patch probably introduces an unwanted change to the result. In the past parsing ':block_html' would return array(BLOCK_POS_RIGHT => array('block_html')). Now it will return array(BLOCK_POS_LEFT => array(), BLOCK_POS_RIGHT => array('block_html'))

          • I think we need to undo that unwanted change.
          • blocks_parse_default_blocks_list is exactly the sort of function that should have unit tests, and now would be a really good time to add some.

          add_blocks

          I think the changes to the code are OK. However, existing PHP doc comment is not OK, and now would be a really good time to fix it to explain how the $weight argument works in combination with the $blocks array.

          I hope it is OK for you to resolve these issues before this is pushed forwards for integration.

          Show
          timhunt Tim Hunt added a comment - I am afraid that I can see several problems here: blocks_parse_default_blocks_list The changes in blocks_parse_default_blocks_list are quite subtle. It took me a while to work out what is going on. The point is that you can define a string like ',,,block_x,,,blocky' to mean block_x has weight 3, and block y has weight 6. However, how is anyone supposed to know that? The format of the strings being parsed needs to be documented somewhere, probably in config-dist.php where this is used, and in the PHPdoc @param string $blocksstr. The PHPdocs for the return-type of blocks_parse_default_blocks_list needs to explain the details of the array that is returned. Note that this patch probably introduces an unwanted change to the result. In the past parsing ':block_html' would return array(BLOCK_POS_RIGHT => array('block_html')). Now it will return array(BLOCK_POS_LEFT => array(), BLOCK_POS_RIGHT => array('block_html')) I think we need to undo that unwanted change. blocks_parse_default_blocks_list is exactly the sort of function that should have unit tests, and now would be a really good time to add some. add_blocks I think the changes to the code are OK. However, existing PHP doc comment is not OK, and now would be a really good time to fix it to explain how the $weight argument works in combination with the $blocks array. I hope it is OK for you to resolve these issues before this is pushed forwards for integration.
          Hide
          mikegrant Michael Grant added a comment -

          Thanks for the speedy code review Tim, appreciate it. I'll take a look at them changes tomorrow and see when I could complete them.

          I'm not 100% sure on what the 'final' improvement of the PHP doc comment would look like, but guess that's the point of this.

          Once again, thanks Tim.

          Show
          mikegrant Michael Grant added a comment - Thanks for the speedy code review Tim, appreciate it. I'll take a look at them changes tomorrow and see when I could complete them. I'm not 100% sure on what the 'final' improvement of the PHP doc comment would look like, but guess that's the point of this. Once again, thanks Tim.
          Hide
          mikegrant Michael Grant added a comment -

          I've done everything you suggested from the peer review Tim, except for the documentation in config-dist.php. I can't quite think of a succinct way of explaining it!

          Anyone able to add CI tag again for another pass? Thanks

          Show
          mikegrant Michael Grant added a comment - I've done everything you suggested from the peer review Tim, except for the documentation in config-dist.php. I can't quite think of a succinct way of explaining it! Anyone able to add CI tag again for another pass? Thanks
          Hide
          cibot CiBoT added a comment -
          Show
          cibot CiBoT added a comment - Fails against automated checks. Checked MDL-37470 using repository: https://github.com/mike-grant/moodle MOODLE_27_STABLE (15 errors / 0 warnings) [branch: MDL-37470_27 | CI Job ] phplint (0/0) , php (9/0) , js (0/0) , css (0/0) , phpdoc (5/0) , commit (1/0) , savepoint (0/0) , thirdparty (0/0) , MOODLE_28_STABLE (15 errors / 0 warnings) [branch: MDL-37470_28 | CI Job ] phplint (0/0) , php (9/0) , js (0/0) , css (0/0) , phpdoc (5/0) , commit (1/0) , savepoint (0/0) , thirdparty (0/0) , master (15 errors / 0 warnings) [branch: MDL-37470 | CI Job ] phplint (0/0) , php (9/0) , js (0/0) , css (0/0) , phpdoc (5/0) , commit (1/0) , savepoint (0/0) , thirdparty (0/0) , More information about this report
          Hide
          timhunt Tim Hunt added a comment -

          You need to merge the two commits on the branch so there is only one. Until you do, CiBoT won't be happy.

          Show
          timhunt Tim Hunt added a comment - You need to merge the two commits on the branch so there is only one. Until you do, CiBoT won't be happy.
          Hide
          cibot CiBoT added a comment -
          Show
          cibot CiBoT added a comment - Fails against automated checks. Checked MDL-37470 using repository: https://github.com/mike-grant/moodle MOODLE_27_STABLE (15 errors / 0 warnings) [branch: MDL-37470_27 | CI Job ] phplint (0/0) , php (9/0) , js (0/0) , css (0/0) , phpdoc (5/0) , commit (1/0) , savepoint (0/0) , thirdparty (0/0) , MOODLE_28_STABLE (15 errors / 0 warnings) [branch: MDL-37470_28 | CI Job ] phplint (0/0) , php (9/0) , js (0/0) , css (0/0) , phpdoc (5/0) , commit (1/0) , savepoint (0/0) , thirdparty (0/0) , master (4 errors / 0 warnings) [branch: MDL-37470 | CI Job ] phplint (0/0) , php (0/0) , js (0/0) , css (0/0) , phpdoc (1/0) , commit (3/0) , savepoint (0/0) , thirdparty (0/0) , More information about this report
          Hide
          timhunt Tim Hunt added a comment -

          Sorry, I ran out of time today. If you can try to sort out CiBoT, I will look at this tomorrow. (In case you are wondering, I am in the UK time-zone.)

          Show
          timhunt Tim Hunt added a comment - Sorry, I ran out of time today. If you can try to sort out CiBoT, I will look at this tomorrow. (In case you are wondering, I am in the UK time-zone.)
          Hide
          mikegrant Michael Grant added a comment -

          Hi Tim,

          Whats the best way to squash what will be 3 commits, to one? Merge? Rebase? reset --soft?

          Thanks

          Show
          mikegrant Michael Grant added a comment - Hi Tim, Whats the best way to squash what will be 3 commits, to one? Merge? Rebase? reset --soft? Thanks
          Hide
          timhunt Tim Hunt added a comment -

          This is git, there are many ways you could do it, the important thing is the final result. I would probably use

          git rebase -i master

          Show
          timhunt Tim Hunt added a comment - This is git, there are many ways you could do it, the important thing is the final result. I would probably use git rebase -i master
          Hide
          mikegrant Michael Grant added a comment -

          Here's praying this is the one for CI and you're happy!

          Show
          mikegrant Michael Grant added a comment - Here's praying this is the one for CI and you're happy!
          Hide
          cibot CiBoT added a comment -

          Code verified against automated checks.

          Checked MDL-37470 using repository: https://github.com/mike-grant/moodle

          More information about this report

          Show
          cibot CiBoT added a comment - Code verified against automated checks. Checked MDL-37470 using repository: https://github.com/mike-grant/moodle MOODLE_27_STABLE (0 errors / 0 warnings) [branch: MDL-37470_27 | CI Job ] MOODLE_28_STABLE (0 errors / 0 warnings) [branch: MDL-37470_28 | CI Job ] master (0 errors / 0 warnings) [branch: MDL-37470 | CI Job ] More information about this report
          Hide
          timhunt Tim Hunt added a comment -

          Sorry it took me so long to review this again. it looks spot-on now. Submitting for integration.

          Show
          timhunt Tim Hunt added a comment - Sorry it took me so long to review this again. it looks spot-on now. Submitting for integration.
          Hide
          mikegrant Michael Grant added a comment -

          Thanks for the help Tim! Much appreciated!

          Show
          mikegrant Michael Grant added a comment - Thanks for the help Tim! Much appreciated!
          Hide
          cibot CiBoT added a comment -

          Moving this issue to current integration cycle, will be reviewed soon. Thanks for the hard work!

          Show
          cibot CiBoT added a comment - Moving this issue to current integration cycle, will be reviewed soon. Thanks for the hard work!
          Hide
          cibot CiBoT added a comment -

          Code verified against automated checks.

          Checked MDL-37470 using repository: https://github.com/mike-grant/moodle

          More information about this report

          Show
          cibot CiBoT added a comment - Code verified against automated checks. Checked MDL-37470 using repository: https://github.com/mike-grant/moodle MOODLE_27_STABLE (0 errors / 0 warnings) [branch: MDL-37470_27 | CI Job ] MOODLE_28_STABLE (0 errors / 0 warnings) [branch: MDL-37470_28 | CI Job ] master (0 errors / 0 warnings) [branch: MDL-37470 | CI Job ] More information about this report
          Hide
          stronk7 Eloy Lafuente (stronk7) added a comment -

          Integrated (27, 28 & master), thanks!

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - Integrated (27, 28 & master), thanks!
          Hide
          ankit_frenz Ankit Agarwal added a comment -

          The first part of test works as described.

          For second part am not able to replicate the problem before the patch, which makes me feel I am doing it wrong. There is no error generated when I use the following in config and create a course :-

          $CFG->defaultblocks_override = 'participants,calendar_month:activity_modules,news_items,calendar_upcoming';
          

          Setting this to failed, so this can be looked at.

          Cheers

          Show
          ankit_frenz Ankit Agarwal added a comment - The first part of test works as described. For second part am not able to replicate the problem before the patch, which makes me feel I am doing it wrong. There is no error generated when I use the following in config and create a course :- $CFG->defaultblocks_override = 'participants,calendar_month:activity_modules,news_items,calendar_upcoming'; Setting this to failed, so this can be looked at. Cheers
          Hide
          dobedobedoh Andrew Nicols added a comment -

          Ankit Agarwal,

          I'm able to replicate the previous behaviour. Screenshot attached displaying the error that I'm seeing.

          Show
          dobedobedoh Andrew Nicols added a comment - Ankit Agarwal , I'm able to replicate the previous behaviour. Screenshot attached displaying the error that I'm seeing.
          Hide
          dobedobedoh Andrew Nicols added a comment -

          Sending back to testing as I am able to replicate the section that Ankit can't.

          Show
          dobedobedoh Andrew Nicols added a comment - Sending back to testing as I am able to replicate the section that Ankit can't.
          Hide
          ankit_frenz Ankit Agarwal added a comment - - edited

          Hi Andrew
          A few things :-
          The testing instructions says to set the config to :-

          $CFG->defaultblocks_override = ',,participants,calendar_month:activity_modules,news_items,calendar_upcoming';
          

          Notice the ,, in start and except a "Block does not exist error before but no error after patch"
          However in this case a "block type disabled by admin" error is produced both before and after the patch.

          using

          $CFG->defaultblocks_override = 'participants,calendar_month:activity_modules,news_items,calendar_upcoming';
          

          No error is produced either before or after the patch.

          Show
          ankit_frenz Ankit Agarwal added a comment - - edited Hi Andrew A few things :- The testing instructions says to set the config to :- $CFG->defaultblocks_override = ',,participants,calendar_month:activity_modules,news_items,calendar_upcoming'; Notice the ,, in start and except a "Block does not exist error before but no error after patch" However in this case a "block type disabled by admin" error is produced both before and after the patch. using $CFG->defaultblocks_override = 'participants,calendar_month:activity_modules,news_items,calendar_upcoming'; No error is produced either before or after the patch.
          Hide
          ankit_frenz Ankit Agarwal added a comment -

          Marking it failed again for Andrew to have a look tomorrow.

          Show
          ankit_frenz Ankit Agarwal added a comment - Marking it failed again for Andrew to have a look tomorrow.
          Hide
          ankit_frenz Ankit Agarwal added a comment -

          Also while looking at this I noticed that there was another commit 5052356574813655713ae123204b57a3b45000f4 with the same MDL number that was most likely introduced in MDL-37474 by mistake (haven't investigated much in detail).

          Show
          ankit_frenz Ankit Agarwal added a comment - Also while looking at this I noticed that there was another commit 5052356574813655713ae123204b57a3b45000f4 with the same MDL number that was most likely introduced in MDL-37474 by mistake (haven't investigated much in detail).
          Hide
          stronk7 Eloy Lafuente (stronk7) added a comment -

          Oh, yes. That one was epic (the MDL confusion). Seems that Sam (et all) were unable to type/read/verify MDL-37474 properly, and it all ended being confused many times. Plus we did not have CiBoT yet telling us the truth. Epic! Ignore it, please.

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - Oh, yes. That one was epic (the MDL confusion). Seems that Sam (et all) were unable to type/read/verify MDL-37474 properly, and it all ended being confused many times. Plus we did not have CiBoT yet telling us the truth. Epic! Ignore it, please.
          Hide
          dobedobedoh Andrew Nicols added a comment -

          Thanks Ankit - I misunderstood your original comment. It appeared that you were unable to replicate the failure before this patch were applied.

          Now I've rewritten the instructions to be a bit more legible, it appears that there's a chunk of this patch missing. In fact, there are two chunks missing:

          1. Any documentation to describe the use of the empty list values (the ,, in ,,participants). These appear to translate to participants having a weight of 3 because participants is now the third list element;
          2. Actual handling of this case is just completely missing. From what I can tell, it needs to be carried through to lib/blocklib.php::add_blocks() where it will be used to ascertain the intended weight, but then the actual call to add_block() should be skipped if the blockname is empty.

          Because of these two key missing points, I'm going to revert this patch.

          Thanks,

          Andrew

          Show
          dobedobedoh Andrew Nicols added a comment - Thanks Ankit - I misunderstood your original comment. It appeared that you were unable to replicate the failure before this patch were applied. Now I've rewritten the instructions to be a bit more legible, it appears that there's a chunk of this patch missing. In fact, there are two chunks missing: Any documentation to describe the use of the empty list values (the ,, in ,,participants). These appear to translate to participants having a weight of 3 because participants is now the third list element; Actual handling of this case is just completely missing. From what I can tell, it needs to be carried through to lib/blocklib.php::add_blocks() where it will be used to ascertain the intended weight, but then the actual call to add_block() should be skipped if the blockname is empty. Because of these two key missing points, I'm going to revert this patch. Thanks, Andrew
          Hide
          dobedobedoh Andrew Nicols added a comment -

          Also, this issue seems to be confused as to whether it's a new feature or a bug.
          There seems to be some bug fix going on here, but the testing instructions test a new feature.

          The code change does not seem to relate to the new empty list element syntax at all and seems to be purely bug fix and not relate to the new functionality at all.

          Waiting for other integrator input before reverting.

          Show
          dobedobedoh Andrew Nicols added a comment - Also, this issue seems to be confused as to whether it's a new feature or a bug. There seems to be some bug fix going on here, but the testing instructions test a new feature. The code change does not seem to relate to the new empty list element syntax at all and seems to be purely bug fix and not relate to the new functionality at all. Waiting for other integrator input before reverting.
          Hide
          dobedobedoh Andrew Nicols added a comment -

          After discussion with other integrators we believe that the testing instructions are wrong. This change does indeed seem to belong to a bug, but the final part of the testing instructions appears to relate to a new feature which was not completed in this code.

          I've updated the testing instructions to remove the offending section.

          Michael Grant, if this is something that you guys actually do need (and from the discussion it appears so), then please raise a new issue for this new functionality. That new feature should:

          1. be a "New feature" in the tracker;
          2. only be targetted at master; and
          3. include documentation in config-dist.php on how it is to be used.

          Thanks,

          Andrew

          Show
          dobedobedoh Andrew Nicols added a comment - After discussion with other integrators we believe that the testing instructions are wrong. This change does indeed seem to belong to a bug, but the final part of the testing instructions appears to relate to a new feature which was not completed in this code. I've updated the testing instructions to remove the offending section. Michael Grant , if this is something that you guys actually do need (and from the discussion it appears so), then please raise a new issue for this new functionality. That new feature should: be a "New feature" in the tracker; only be targetted at master; and include documentation in config-dist.php on how it is to be used. Thanks, Andrew
          Hide
          dobedobedoh Andrew Nicols added a comment -

          Now that's all sorted out, sending back to testing.

          Show
          dobedobedoh Andrew Nicols added a comment - Now that's all sorted out, sending back to testing.
          Hide
          ankit_frenz Ankit Agarwal added a comment -

          Thanks Andrew for fixing the instructions.

          Passing the issue as those steps were working.

          Show
          ankit_frenz Ankit Agarwal added a comment - Thanks Andrew for fixing the instructions. Passing the issue as those steps were working.
          Hide
          mikegrant Michael Grant added a comment -

          Hi Andrew,

          What I actually need is add_blocks to respect the weighting applied, which this patch does. So I'm happy with how this currently is.

          Thanks for your time guys.

          Show
          mikegrant Michael Grant added a comment - Hi Andrew, What I actually need is add_blocks to respect the weighting applied, which this patch does. So I'm happy with how this currently is. Thanks for your time guys.
          Hide
          stronk7 Eloy Lafuente (stronk7) added a comment -

          And this is now part of Moodle and, soon, part of the life of some millions of people out there. Amazing! Well done!

          Consensus isn't just about agreement. It's about changing things around:
          You get a proposal, you work something out, people foresee problems, you
          do creative synthesis. At the end of it, you come up with something that
          everyone thinks is okay. Most people like it, and nobody hates it.
          ---- David Graeber

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - And this is now part of Moodle and, soon, part of the life of some millions of people out there. Amazing! Well done! Consensus isn't just about agreement. It's about changing things around: You get a proposal, you work something out, people foresee problems, you do creative synthesis. At the end of it, you come up with something that everyone thinks is okay. Most people like it, and nobody hates it. ---- David Graeber

            People

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

              Dates

              • Created:
                Updated:
                Resolved:
                Fix Release Date:
                9/Mar/15