From 8bcf74e9bc8cd34780d14e00537ea1cdf0190608 Mon Sep 17 00:00:00 2001
From: Andrew Nicols <andrew@nicols.co.uk>
Date: Fri, 12 Jun 2020 08:46:46 +0800
Subject: [PATCH] MDL-69026 user: Wrap sub-query in brackets

It is perfectly valid to have a query like:

Match None of the following:
- Role is ANY of the following:
-- 'Teacher'
-- 'Editing teacher'
-- 'Manager'; AND
- Keyword is NONE of the following:
-- 'Kevin'

However, due to the way in which the query is constructed, this leads to
a query which includes

    WHERE NOT ef.id IS NOT NULL
    AND NOT
        u.id IN (SELECT userid FROM {role_assignments} WHERE roleid IN (...) AND contextid IN (...))
    AND NOT
        NOT (u.firstname || ' ' || u.lastname LIKE '%Kevin%')

The use of NOT NOT is valid in Postgres, MariaDB, MySQL, and MSSQL, but
not in Oracle.

To counter this when the outer jointype is of type NONE, we must wrap
each of the inner WHERE clauses in a set of brackets, which makes the
query:

    WHERE NOT ef.id IS NOT NULL
    AND NOT
        (u.id IN (SELECT userid FROM {role_assignments} WHERE roleid IN (...) AND contextid IN (...)))
    AND NOT
        (NOT (u.firstname || ' ' || u.lastname LIKE '%Kevin%'))

Whilst Oracle does not support the use of `AND NOT NOT ...`, it does support
`AND NOT (NOT ...)`
---
 user/classes/table/participants_search.php    |  8 +++++++
 user/tests/table/participants_search_test.php | 23 ++++++++++++++++++-
 2 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/user/classes/table/participants_search.php b/user/classes/table/participants_search.php
index 33122ccc369..7946247f96b 100644
--- a/user/classes/table/participants_search.php
+++ b/user/classes/table/participants_search.php
@@ -283,6 +283,14 @@ class participants_search {
                 case $this->filterset::JOINTYPE_NONE:
                     $wherenot = ' NOT ';
                     $wheresjoin = ' AND NOT ';
+
+                    // Some of the $where conditions may begin with `NOT` which results in `AND NOT NOT ...`.
+                    // To prevent this from breaking on Oracle the inner WHERE clause is wrapped in brackets, making it
+                    // `AND NOT (NOT ...)` which is valid in all DBs.
+                    $wheres = array_map(function($where) {
+                        return "({$where})";
+                    }, $wheres);
+
                     break;
                 default:
                     // Default to 'Any' jointype.
diff --git a/user/tests/table/participants_search_test.php b/user/tests/table/participants_search_test.php
index 66ce32c6bc7..2722357ccdc 100644
--- a/user/tests/table/participants_search_test.php
+++ b/user/tests/table/participants_search_test.php
@@ -3203,7 +3203,7 @@ class participants_search_test extends advanced_testcase {
                             'accesssince' => [
                                 'values' => ['-6 months'],
                                 'jointype' => filter::JOINTYPE_ALL,
-                                ],
+                            ],
                         ],
                         'jointype' => filter::JOINTYPE_NONE,
                         'count' => 1,
@@ -3211,6 +3211,27 @@ class participants_search_test extends advanced_testcase {
                             'barbara.bennett',
                         ],
                     ],
+                    'NONE: Filterset combining several filter types and a double-negative on keyword' => (object) [
+                        'jointype' => filter::JOINTYPE_NONE,
+                        'filterdata' => [
+                            // Note: This is a jointype NONE on the parent jointype NONE.
+                            // The result therefore negated in this instance.
+                            // Include Adam and Anthony.
+                            'keywords' => [
+                                'values' => ['ant'],
+                                'jointype' => filter::JOINTYPE_NONE,
+                            ],
+                            // Excludes Tony.
+                            'status' => [
+                                'values' => [ENROL_USER_SUSPENDED],
+                                'jointype' => filter::JOINTYPE_ALL,
+                            ],
+                        ],
+                        'count' => 1,
+                        'expectedusers' => [
+                            'adam.ant',
+                        ],
+                    ],
                 ],
             ],
         ];