Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 41 additions & 13 deletions rdmo/projects/progress.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)


MyPyDavid marked this conversation as resolved.
Show resolved Hide resolved
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')
Expand Down Expand Up @@ -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)
MyPyDavid marked this conversation as resolved.
Show resolved Hide resolved

# count the total number of questions, taking sets and conditions into account
counts = count_questions(page, sets, conditions)
Expand Down
36 changes: 36 additions & 0 deletions rdmo/projects/tests/test_viewset_project_page.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
jochenklar marked this conversation as resolved.
Show resolved Hide resolved
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
jochenklar marked this conversation as resolved.
Show resolved Hide resolved
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
45 changes: 27 additions & 18 deletions rdmo/projects/viewsets.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand Down Expand Up @@ -521,29 +528,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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder how much more expensive this is. In the future we could move functions like compute_sets to the manager. Just having ideas...

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:
Expand Down
Loading