Merge pull request #1192 from rdmorganiser/1191-interview-slow-loading-on-proceed-when-a-lot-of-conditional-pages-are-skipped

feat(projects): add check for finding next relevant page

Author: MyPyDavid

rdmo/projects/progress.py

@@ -47,6 +47,44 @@ def compute_sets(values):
     return sets
 
 
+def compute_next_relevant_page(current_page, direction, catalog, resolved_conditions):
+    # recursively compute the next relevant page based on resolved conditions.
+    # first, get the next page from the catalog based on the specified direction
+    next_page = (
+        catalog.get_prev_page(current_page) if direction == 'prev'
+        else catalog.get_next_page(current_page)
+    )
+
+    # if there is no next page, return None
+    if not next_page:
+        return None
+
+    # check if the next page meets the conditions
+    if compute_show_page(next_page, resolved_conditions):
+        return next_page
+
+    # recursive step: check the next page
+    return compute_next_relevant_page(next_page, direction, catalog, resolved_conditions)
+
+
+def compute_show_page(page, conditions):
+    # determine if a page should be shown based on resolved conditions
+    # show only pages with resolved conditions, but show all pages without conditions
+    pages_conditions = {page.id for page in page.conditions.all()}
+
+    if pages_conditions:
+        # check if any valuesets for set_prefix = '' resolved
+        # for non collection pages restrict further to set_index = 0
+
+        return any(
+            (set_prefix == '') and (page.is_collection or set_index == 0)
+            for page_condition in pages_conditions
+            for set_prefix, set_index in conditions[page_condition]
+        )
+    else:
+        return True
+
+
 def compute_navigation(section, project, snapshot=None):
     # get all values for this project and snapshot
     values = project.values.filter(snapshot=snapshot).select_related('attribute', 'option')
@@ -74,19 +112,9 @@ def compute_navigation(section, project, snapshot=None):
             navigation_section['pages'] = []
 
         for page in catalog_section.elements:
-            pages_conditions = {page.id for page in page.conditions.all()}
-
-            # show only pages with resolved conditions, but show all pages without conditions
-            if pages_conditions:
-                # check if any valuesets for set_prefix = '' resolved
-                # for non collection pages restrict further to set_index = 0
-                show = any(
-                    (set_prefix == '') and (page.is_collection or set_index == 0)
-                    for page_condition in pages_conditions
-                    for set_prefix, set_index in conditions[page_condition]
-                )
-            else:
-                show = True
+
+            # determine if a page should be shown or not
+            show = compute_show_page(page, conditions)
 
             # count the total number of questions, taking sets and conditions into account
             counts = count_questions(page, sets, conditions)

rdmo/projects/tests/test_viewset_project_page.py

@@ -88,3 +88,39 @@ def test_detail_page_with_nested_questionsets(db, client):
     assert questionsets_ids == nested_questionsets_id
     element_ids = [i['id'] for qs in questionsets for i in qs['elements']]
     assert element_ids == nested_element_ids
+
+
+@pytest.mark.parametrize('direction', ['next', 'prev'])
+def test_detail_page_resolve_next_relevant_page(db, client, direction):
+    project_id = 1
+    username = 'owner'
+    start_page_id = 64
+    end_page_id = 69
+
+    client.login(username=username, password=username)
+
+    if direction == 'next':
+        next_page_id = start_page_id + 1
+        add_url = ''
+    else:  # direction == 'prev':
+        start_page_id, end_page_id = end_page_id, start_page_id
+        next_page_id = start_page_id - 1
+        add_url = '?back=true'
+
+    # get the starting page
+    url = reverse(urlnames['detail'], args=[project_id, start_page_id])
+    response = client.get(f'{url}{add_url}')
+    assert response.status_code == 200
+    assert response.json().get(f'{direction}_page') == next_page_id
+
+    # get the following page, depending on direction
+    url_next = reverse(urlnames['detail'], args=[project_id, next_page_id])
+    response_next = client.get(f'{url_next}{add_url}')
+    assert response_next.status_code == 303
+
+    # this should show the redirect to the next relevant page
+    assert response_next.url.endswith(f'{end_page_id}/')
+
+    # a get on the redirected url as a double check
+    response_next_relevant = client.get(response_next.url)
+    assert response_next_relevant.status_code == 200

rdmo/projects/viewsets.py

@@ -21,7 +21,7 @@
 
 from rdmo.conditions.models import Condition
 from rdmo.core.permissions import HasModelPermission
-from rdmo.core.utils import human2bytes, return_file_response
+from rdmo.core.utils import human2bytes, is_truthy, return_file_response
 from rdmo.options.models import OptionSet
 from rdmo.questions.models import Catalog, Page, Question, QuestionSet
 from rdmo.tasks.models import Task
@@ -42,7 +42,14 @@
     HasProjectProgressObjectPermission,
     HasProjectsPermission,
 )
-from .progress import compute_navigation, compute_progress
+from .progress import (
+    compute_navigation,
+    compute_next_relevant_page,
+    compute_progress,
+    compute_sets,
+    compute_show_page,
+    resolve_conditions,
+)
 from .serializers.v1 import (
     IntegrationSerializer,
     InviteSerializer,
@@ -541,29 +548,31 @@ def dispatch(self, *args, **kwargs):
 
     def retrieve(self, request, *args, **kwargs):
         page = self.get_object()
-        conditions = page.conditions.select_related('source', 'target_option')
-
+        catalog = self.project.catalog
         values = self.project.values.filter(snapshot=None).select_related('attribute', 'option')
 
-        if check_conditions(conditions, values):
+        sets = compute_sets(values)
+        resolved_conditions = resolve_conditions(catalog, values, sets)
+
+        # check if the current page meets conditions
+        if compute_show_page(page, resolved_conditions):
             serializer = self.get_serializer(page)
             return Response(serializer.data)
         else:
-            if request.GET.get('back') == 'true':
-                prev_page = self.project.catalog.get_prev_page(page)
-                if prev_page is not None:
-                    url = reverse('v1-projects:project-page-detail',
-                                  args=[self.project.id, prev_page.id]) + '?back=true'
-                    return HttpResponseRedirect(url, status=303)
-            else:
-                next_page = self.project.catalog.get_next_page(page)
-                if next_page is not None:
-                    url = reverse('v1-projects:project-page-detail', args=[self.project.id, next_page.id])
-                    return HttpResponseRedirect(url, status=303)
-
-            # indicate end of catalog
+            # determine the direction of navigation (previous or next)
+            direction = 'prev' if is_truthy(request.GET.get('back')) else 'next'
+
+            # find the next relevant page with from pages and resolved conditions
+            next_relevant_page = compute_next_relevant_page(page, direction, catalog, resolved_conditions)
+
+            if next_relevant_page is not None:
+                url = reverse('v1-projects:project-page-detail', args=[self.project.id, next_relevant_page.id])
+                return HttpResponseRedirect(url, status=303)
+
+            # end of catalog, if no next relevant page is found
             return Response(status=204)
 
+
     @action(detail=False, url_path='continue', permission_classes=(HasModelPermission | HasProjectPagePermission, ))
     def get_continue(self, request, pk=None, parent_lookup_project=None):
         try: