Details

    • Type: Sub-task Sub-task
    • Status: Open
    • Priority: Minor Minor
    • Resolution: Unresolved
    • Affects Version/s: 1.9, 2.3
    • Fix Version/s: DEV backlog
    • Component/s: Forum
    • Labels:
    • Testing Instructions:
      Hide

      Note that to test this properly you need a database with some orphaned posts. Follow these steps with a 2.3 instance without this fix applied.

      1. Revert MDL-31664.
      2. Create a forum.
      3. Create a discussion (post A) as a user.
      4. Post a reply (post B) as another user.
      5. Post another reply (post C) to Post B.
      6. Post a separate reply (post D) to Post A.
      7. Delete Post B.
      8. Verify that Post C and Post D are still in the database.

      Now apply this fix to upgrade mod/forum. Run the upgrade. Post C should be deleted from the database. Posts A and D should still be there.

      Show
      Note that to test this properly you need a database with some orphaned posts. Follow these steps with a 2.3 instance without this fix applied. 1. Revert MDL-31664 . 2. Create a forum. 3. Create a discussion (post A) as a user. 4. Post a reply (post B) as another user. 5. Post another reply (post C) to Post B. 6. Post a separate reply (post D) to Post A. 7. Delete Post B. 8. Verify that Post C and Post D are still in the database. Now apply this fix to upgrade mod/forum. Run the upgrade. Post C should be deleted from the database. Posts A and D should still be there.
    • Affected Branches:
      MOODLE_19_STABLE, MOODLE_23_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-13516-master
    • Rank:
      327

      Description

      posts without parents

        Issue Links

          Activity

          Petr Škoda created issue -
          Petr Škoda made changes -
          Field Original Value New Value
          Fix Version/s 2.0 [ 10122 ]
          Hide
          Petr Škoda added a comment -

          reassigning to HQ

          Show
          Petr Škoda added a comment - reassigning to HQ
          Petr Škoda made changes -
          Assignee Petr ?koda (skodak) [ skodak ] moodle.com [ moodle.com ]
          Martin Dougiamas made changes -
          Fix Version/s 2.0.1 [ 10420 ]
          Fix Version/s 2.0 [ 10122 ]
          Martin Dougiamas made changes -
          Workflow jira [ 24953 ] MDL Workflow [ 43054 ]
          Martin Dougiamas made changes -
          Fix Version/s 2.0.2 [ 10421 ]
          Fix Version/s 2.0.1 [ 10420 ]
          Martin Dougiamas made changes -
          Fix Version/s 2.0.3 [ 10537 ]
          Fix Version/s 2.0.2 [ 10421 ]
          Martin Dougiamas made changes -
          Workflow MDL Workflow [ 43054 ] MDL Full Workflow [ 71456 ]
          Eloy Lafuente (stronk7) made changes -
          Fix Version/s 2.0.4 [ 10652 ]
          Fix Version/s 2.0.3 [ 10537 ]
          Martin Dougiamas made changes -
          Fix Version/s 2.0.5 [ 10950 ]
          Fix Version/s 2.0.4 [ 10652 ]
          Eloy Lafuente (stronk7) made changes -
          Fix Version/s 2.0.6 [ 11250 ]
          Fix Version/s 2.0.5 [ 10950 ]
          Eloy Lafuente (stronk7) made changes -
          Fix Version/s 2.0.7 [ 11451 ]
          Fix Version/s 2.0.6 [ 11250 ]
          Eloy Lafuente (stronk7) made changes -
          Fix Version/s 2.0.8 [ 11554 ]
          Fix Version/s 2.0.7 [ 11451 ]
          Eloy Lafuente (stronk7) made changes -
          Fix Version/s 2.0.9 [ 12051 ]
          Fix Version/s 2.0.8 [ 11554 ]
          Charles Fulton made changes -
          Link This issue has been marked as being related by MDL-31664 [ MDL-31664 ]
          Hide
          Petr Škoda added a comment -

          This should be done in forum/db/upgrade.php, it might be enough to do it in master only because sooner or later everybody will upgrade...

          Show
          Petr Škoda added a comment - This should be done in forum/db/upgrade.php, it might be enough to do it in master only because sooner or later everybody will upgrade...
          Petr Škoda made changes -
          Fix Version/s DEV backlog [ 10464 ]
          Fix Version/s 2.0.9 [ 12051 ]
          Labels triaged
          Charles Fulton made changes -
          Pull Master Diff URL https://github.com/mackensen/moodle/compare/master...MDL-13516-master
          Pull Master Branch MDL-13516-master
          Testing Instructions Note that to test this properly you need a database with some orphaned posts. Follow these steps with 2.3 instance without this fix applied.

          1. Revert MDL-13516.
          2. Create a forum.
          3. Create a discussion (post A) as a user.
          4. Post a reply (post B) as another user.
          5. Post another reply (post C) to Post B.
          6. Post a separate reply (post D) to Post A.
          7. Delete Post B.
          8. Verify that Post C and Post D are still in the database.

          Now apply this fix to upgrade mod/forum. Run the upgrade. Post C should be deleted from the database. Posts A and D should still be there.
          Pull from Repository https://github.com/mackensen/moodle
          Labels triaged patch triaged
          Assignee moodle.com [ moodle.com ] Charles Fulton [ cfulton ]
          Affects Version/s 2.3 [ 10657 ]
          Charles Fulton made changes -
          Status Open [ 1 ] Waiting for peer review [ 10012 ]
          Ankit Agarwal made changes -
          Original Estimate 0 minutes [ 0 ]
          Remaining Estimate 0 minutes [ 0 ]
          Status Waiting for peer review [ 10012 ] Peer review in progress [ 10013 ]
          Peer reviewer ankit_frenz
          Hide
          Petr Škoda added a comment -

          hi, I think the patch is not going to perform well:

          • NOT IN should be probably replaced by LEFT JOIN with NULL condition
          • it might hit memory problems or query limits (oracle), I would recommend using recordsets
          • who is going to delete attachments and embedded images?
          Show
          Petr Škoda added a comment - hi, I think the patch is not going to perform well: NOT IN should be probably replaced by LEFT JOIN with NULL condition it might hit memory problems or query limits (oracle), I would recommend using recordsets who is going to delete attachments and embedded images?
          Ankit Agarwal made changes -
          Comment [ Hi Charles,
          Code looks good to me.
          delete_record_list takes care of the case when $ids is empty. still it might be better to do :-
          {code}
          if(!empty($ids)) {
          $DB->delete_records_list('forum_posts', 'id', array_keys($ids));
          }
          {code}
          But feel free to ignore that and submit this for integration.(let me know if you want me to submit it for integration)
          Thanks ]
          Hide
          Ankit Agarwal added a comment -

          assigning to petr as the reviewer
          Thanks

          Show
          Ankit Agarwal added a comment - assigning to petr as the reviewer Thanks
          Ankit Agarwal made changes -
          Peer reviewer ankit_frenz skodak
          Ankit Agarwal made changes -
          Status Peer review in progress [ 10013 ] Development in progress [ 3 ]
          Charles Fulton made changes -
          Testing Instructions Note that to test this properly you need a database with some orphaned posts. Follow these steps with 2.3 instance without this fix applied.

          1. Revert MDL-13516.
          2. Create a forum.
          3. Create a discussion (post A) as a user.
          4. Post a reply (post B) as another user.
          5. Post another reply (post C) to Post B.
          6. Post a separate reply (post D) to Post A.
          7. Delete Post B.
          8. Verify that Post C and Post D are still in the database.

          Now apply this fix to upgrade mod/forum. Run the upgrade. Post C should be deleted from the database. Posts A and D should still be there.
          Note that to test this properly you need a database with some orphaned posts. Follow these steps with a 2.3 instance without this fix applied.

          1. Revert MDL-31664.
          2. Create a forum.
          3. Create a discussion (post A) as a user.
          4. Post a reply (post B) as another user.
          5. Post another reply (post C) to Post B.
          6. Post a separate reply (post D) to Post A.
          7. Delete Post B.
          8. Verify that Post C and Post D are still in the database.

          Now apply this fix to upgrade mod/forum. Run the upgrade. Post C should be deleted from the database. Posts A and D should still be there.
          Charles Fulton made changes -
          Status Development in progress [ 3 ] Open [ 1 ]
          Hide
          Petr Škoda added a comment -

          oh, I missed this one, +1

          Show
          Petr Škoda added a comment - oh, I missed this one, +1

            People

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

              Dates

              • Created:
                Updated: