Moodle
  1. Moodle
  2. MDL-17965

Forum move discussions: Should not let you move to forums you don't have write access to

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.9.3, 2.0
    • Fix Version/s: 1.9.4
    • Component/s: Forum
    • Labels:
      None
    • Affected Branches:
      MOODLE_19_STABLE, MOODLE_20_STABLE
    • Fixed Branches:
      MOODLE_19_STABLE
    • Rank:
      31580

      Description

      About moving forum discussions.

      Currently, if you have mod/forum:movediscussions in a forum, you can move posts to any other forum in the site that you can see (coursemodule_visible_for_user).

      This causes problems because:

      a) You can move a post into a forum where you would not ordinarily be able to post to, meaning that anyone who has move permission on any forum can sidestep 'read-only' restrictions.

      b) You can move a post into a forum by accident (ie intending to move it, but accidentally pick the wrong forum) and then cannot undo that mistake because you aren't allowed to move posts out of the target forum.

      My suggestion is that either:

      1) you should only be able to move a discussion if you have mod/forum:movediscussions in the destination forum as well as the source forum (ie so you can move it back if needed). This is my preferred option that deals with both a and b above.

      OR

      2) you must at least be able to start a discussion in the destination forum. This only deals with a above.

      I'd like to make this change in 1.9.x if possible, otherwise can do just 2.x. Any opinions?

        Issue Links

          Activity

          Hide
          Eloy Lafuente (stronk7) added a comment -

          +1 for movediscussions (origin) plus startdiscussion (target). That's the thing that move discussions is doing. In fact I was thinking about movediscussionsto and movediscussionsfrom, but startdiscussion sounds perfect for me.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - +1 for movediscussions (origin) plus startdiscussion (target). That's the thing that move discussions is doing. In fact I was thinking about movediscussionsto and movediscussionsfrom, but startdiscussion sounds perfect for me. Ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          And why not, +1 for 1.9.4 here. It's an important consistency bug IMO.

          Show
          Eloy Lafuente (stronk7) added a comment - And why not, +1 for 1.9.4 here. It's an important consistency bug IMO.
          Hide
          Dan Poltawski added a comment -

          1) Sounds sensible to me, though its tricky to decide what is the most intuitive.

          This is a remarkably similar sort of problem to the course categories cleanup tim was doing in http://moodle.org/mod/forum/discuss.php?d=111459

          Show
          Dan Poltawski added a comment - 1) Sounds sensible to me, though its tricky to decide what is the most intuitive. This is a remarkably similar sort of problem to the course categories cleanup tim was doing in http://moodle.org/mod/forum/discuss.php?d=111459
          Hide
          Sam Marshall added a comment -

          Thanks people! Here's the patch, it's very simple. I plan to check this in as soon as I've checked it doesn't break anything in 1.9. (I tested in current HEAD, seems fine, doesn't increase db queries or anything.)

          This is following Eloy's suggestion of using startdiscussion (which stops the most serious aspect of this, and I guess requiring movediscussion as well might have some limitations over the current setup if you want to set up 'one-way' moves for some reason).

          Show
          Sam Marshall added a comment - Thanks people! Here's the patch, it's very simple. I plan to check this in as soon as I've checked it doesn't break anything in 1.9. (I tested in current HEAD, seems fine, doesn't increase db queries or anything.) This is following Eloy's suggestion of using startdiscussion (which stops the most serious aspect of this, and I guess requiring movediscussion as well might have some limitations over the current setup if you want to set up 'one-way' moves for some reason).
          Hide
          Tim Hunt added a comment -

          From developer chat:

          "sam: Move causes a lot of confusion in general, people think students were able to post in places they can't it was just moved there, etc. This won't solve that of course"

          I wonder if moved messages need a line like "Moved from Frog forum to Newt forum on 20th Jan 2009 by Manic Moderator." appended to the post?

          Show
          Tim Hunt added a comment - From developer chat: "sam: Move causes a lot of confusion in general, people think students were able to post in places they can't it was just moved there, etc. This won't solve that of course" I wonder if moved messages need a line like "Moved from Frog forum to Newt forum on 20th Jan 2009 by Manic Moderator." appended to the post?
          Hide
          Tim Hunt added a comment -

          Tested and reviewed code. Looks good. Thanks sam. Closing.

          Show
          Tim Hunt added a comment - Tested and reviewed code. Looks good. Thanks sam. Closing.

            People

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

              Dates

              • Created:
                Updated:
                Resolved: