Uploaded image for project: '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
    • Status: Closed
    • Priority: 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

      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?

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

              Hide
              stronk7 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
              stronk7 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
              stronk7 Eloy Lafuente (stronk7) added a comment -

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

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - And why not, +1 for 1.9.4 here. It's an important consistency bug IMO.
              Hide
              poltawski 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
              poltawski 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
              quen 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
              quen 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
              timhunt 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
              timhunt 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
              timhunt Tim Hunt added a comment -

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

              Show
              timhunt 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:
                    Fix Release Date:
                    28/Jan/09