diff --git a/rdmo/projects/progress.py b/rdmo/projects/progress.py index 39c4cd8a06..efe7421369 100644 --- a/rdmo/projects/progress.py +++ b/rdmo/projects/progress.py @@ -8,42 +8,54 @@ from rdmo.questions.models import Page, Question, QuestionSet -def resolve_conditions(project, values): - # get all conditions for this catalog - pages_conditions_subquery = Page.objects.filter_by_catalog(project.catalog) \ +def get_catalog_conditions(catalog): + pages_conditions_subquery = Page.objects.filter_by_catalog(catalog) \ .filter(conditions=OuterRef('pk')) - questionsets_conditions_subquery = QuestionSet.objects.filter_by_catalog(project.catalog) \ + questionsets_conditions_subquery = QuestionSet.objects.filter_by_catalog(catalog) \ .filter(conditions=OuterRef('pk')) - questions_conditions_subquery = Question.objects.filter_by_catalog(project.catalog) \ + questions_conditions_subquery = Question.objects.filter_by_catalog(catalog) \ .filter(conditions=OuterRef('pk')) - catalog_conditions = Condition.objects.annotate(has_page=Exists(pages_conditions_subquery)) \ - .annotate(has_questionset=Exists(questionsets_conditions_subquery)) \ - .annotate(has_question=Exists(questions_conditions_subquery)) \ - .filter(Q(has_page=True) | Q(has_questionset=True) | Q(has_question=True)) \ - .distinct().select_related('source', 'target_option') + return Condition.objects.annotate( + has_page=Exists(pages_conditions_subquery), + has_questionset=Exists(questionsets_conditions_subquery), + has_question=Exists(questions_conditions_subquery) + ).filter( + Q(has_page=True) | Q(has_questionset=True) | Q(has_question=True) + ).distinct().select_related('source', 'target_option') + + +def resolve_conditions(catalog, values, sets): + # resolve all conditions and return for each condition the set_prefix and set_index for which it resolved true + conditions = defaultdict(set) + if sets: + for condition in get_catalog_conditions(catalog): + conditions[condition.id] = { + (set_prefix, set_index) + for set_prefix, set_index in chain.from_iterable(sets.values()) + if condition.resolve(values, set_prefix=set_prefix, set_index=set_index) + } - # evaluate conditions - conditions = set() - for condition in catalog_conditions: - if condition.resolve(values): - conditions.add(condition.id) - - # return all true conditions for this project return conditions +def compute_sets(values): + # compute sets from values (including empty values) + sets = defaultdict(set) + for attribute, set_prefix, set_index in values.distinct_list(): + sets[attribute].add((set_prefix, set_index)) + return sets + + 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') - # get true conditions - conditions = resolve_conditions(project, values) - # compute sets from values (including empty values) - sets = defaultdict(lambda: defaultdict(list)) - for attribute, set_prefix, set_index in values.distinct_list(): - sets[attribute][set_prefix].append(set_index) + sets = compute_sets(values) + + # resolve all conditions to get a dict mapping conditions to set_indexes + conditions = resolve_conditions(project.catalog, values, sets) # query distinct, non empty set values values_list = values.exclude_empty().distinct_list() @@ -63,13 +75,24 @@ def compute_navigation(section, project, snapshot=None): for page in catalog_section.elements: pages_conditions = {page.id for page in page.conditions.all()} - show = bool(not pages_conditions or pages_conditions.intersection(conditions)) + + # 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 # count the total number of questions, taking sets and conditions into account counts = count_questions(page, sets, conditions) # filter the values_list for the attributes, and compute the total sum of counts - count = len(tuple(filter(lambda value: value[0] in counts.keys(), values_list))) + count = sum(1 for value in values_list if value[0] in counts) total = sum(counts.values()) navigation_section['count'] += count @@ -94,23 +117,20 @@ def compute_progress(project, snapshot=None): # get all values for this project and snapshot values = project.values.filter(snapshot=snapshot).select_related('attribute', 'option') - # get true conditions - conditions = resolve_conditions(project, values) - # compute sets from values (including empty values) - sets = defaultdict(lambda: defaultdict(list)) - for attribute, set_prefix, set_index in values.distinct_list(): - sets[attribute][set_prefix].append(set_index) + sets = compute_sets(values) + + # resolve all conditions to get a dict mapping conditions to set_indexes + conditions = resolve_conditions(project.catalog, values, sets) # query distinct, non empty set values values_list = values.exclude_empty().distinct_list() - # count the total number of questions, taking sets and conditions into account counts = count_questions(project.catalog, sets, conditions) # filter the values_list for the attributes, and compute the total sum of counts - count = len(tuple(filter(lambda value: value[0] in counts.keys(), values_list))) + count = sum(1 for value in values_list if value[0] in counts) total = sum(counts.values()) return count, total @@ -119,39 +139,66 @@ def compute_progress(project, snapshot=None): def count_questions(element, sets, conditions): counts = defaultdict(int) - # obtain the maximum number of set-distinct values for the questions in this page or - # question set this number is how often each question is displayed and we will use - # this number to determine how often a question needs to be counted - if isinstance(element, (Page, QuestionSet)) and element.is_collection: - counted_sets = set() + if isinstance(element, (Page, QuestionSet)): + # obtain the maximum number of set-distinct values for the questions in this page or + # question set. this number is how often each question is displayed and we will use + # this number to determine how often a question needs to be counted + element_sets = set() # count the sets for the id attribute of the page or question if element.attribute is not None: - # nested loop over the separate set_index lists in sets[element.attribute.id] - for set_index in chain.from_iterable(sets[element.attribute.id].values()): - counted_sets.add(set_index) + # nested loop over the separate set_prefix, set_index lists in sets[element.attribute.id] + for set_prefix, set_index in sets[element.attribute.id]: + element_sets.add((set_prefix, set_index)) # count the sets for the questions in the page or question for child in element.elements: if isinstance(child, Question): if child.attribute is not None: - # nested loop over the separate set_index lists in sets[element.attribute.id] - for set_index in chain.from_iterable(sets[child.attribute.id].values()): - counted_sets.add(set_index) + # nested loop over the separate set_prefix, set_index lists in sets[element.attribute.id] + for set_prefix, set_index in sets[child.attribute.id]: + element_sets.add((set_prefix, set_index)) - set_count = len(counted_sets) - else: - set_count = 1 + # look for the elements conditions + element_conditions = {condition.id for condition in element.conditions.all()} + + # if this element has conditions: check if those conditions resolve for the found sets + if element_conditions: + # compute the intersection of the conditions of this child with the full set of conditions + element_condition_intersection = { + (set_prefix, set_index) + for condition_id, condition in conditions.items() + for set_prefix, set_index in conditions[condition_id] + if condition_id in element_conditions + } + + resolved_sets = element_sets.intersection(element_condition_intersection) + if resolved_sets: + set_count = len(resolved_sets) if element.is_collection else 1 + else: + # return immediately and do not consider children, this page/questionset is hidden + return counts + else: + set_count = len(element_sets) if element.is_collection else 1 # loop over all children of this element for child in element.elements: - # look for the elements conditions + # look for the child element's conditions if isinstance(child, (Page, QuestionSet, Question)): child_conditions = {condition.id for condition in child.conditions.all()} else: - child_conditions = [] + child_conditions = set() + + # compute the intersection of the conditions of this child with the full set of conditions + child_condition_intersection = { + (set_prefix, set_index) + for condition_id, condition in conditions.items() + for set_prefix, set_index in conditions[condition_id] + if condition_id in child_conditions + } - if not child_conditions or child_conditions.intersection(conditions): + # check if the element either has no condition or its conditions intersect with the full set of conditions + if not child_conditions or child_condition_intersection: if isinstance(child, Question): # for regular questions add the set_count to the counts dict, since the # question should be answered in every set @@ -160,10 +207,17 @@ def count_questions(element, sets, conditions): # use the max function, since the same attribute could appear twice in the tree if child.attribute is not None: if child.is_optional: - child_count = sum(len(set_indexes) for set_indexes in sets[child.attribute.id].values()) + child_count = len(sets[child.attribute.id]) counts[child.attribute.id] = max(counts[child.attribute.id], child_count) else: - counts[child.attribute.id] = max(counts[child.attribute.id], set_count) + if child_condition_intersection: + # update the set_count for the current question (child element) + # count only the sets that have conditions resolved to true + current_count = len(element_sets.intersection(child_condition_intersection)) + else: + current_count = set_count + + counts[child.attribute.id] = max(counts[child.attribute.id], current_count) else: # for everything else, call this function recursively counts.update(count_questions(child, sets, conditions)) diff --git a/rdmo/projects/tests/test_navigation.py b/rdmo/projects/tests/test_navigation.py index 0af6e30f10..6128a02e7f 100644 --- a/rdmo/projects/tests/test_navigation.py +++ b/rdmo/projects/tests/test_navigation.py @@ -24,27 +24,27 @@ 'http://example.com/terms/questions/catalog/set/individual-collection': (9, 10, True), 'http://example.com/terms/questions/catalog/set/collection-single': (14, 18, True), 'http://example.com/terms/questions/catalog/set/collection-collection': (16, 20, True), - 'http://example.com/terms/questions/catalog/conditions': (15, 23), + 'http://example.com/terms/questions/catalog/conditions': (7, 15), 'http://example.com/terms/questions/catalog/conditions/input': (2, 2, True), 'http://example.com/terms/questions/catalog/conditions/text_contains': (1, 1, True), - 'http://example.com/terms/questions/catalog/conditions/text_empty': (1, 1, False), + 'http://example.com/terms/questions/catalog/conditions/text_empty': (0, 0, False), 'http://example.com/terms/questions/catalog/conditions/text_equal': (1, 1, True), - 'http://example.com/terms/questions/catalog/conditions/text_greater_than': (1, 1, False), - 'http://example.com/terms/questions/catalog/conditions/text_greater_than_equal': (1, 1, False), - 'http://example.com/terms/questions/catalog/conditions/text_lesser_than': (1, 1, False), - 'http://example.com/terms/questions/catalog/conditions/text_lesser_than_equal': (1, 1, False), + 'http://example.com/terms/questions/catalog/conditions/text_greater_than': (0, 0, False), + 'http://example.com/terms/questions/catalog/conditions/text_greater_than_equal': (0, 0, False), + 'http://example.com/terms/questions/catalog/conditions/text_lesser_than': (0, 0, False), + 'http://example.com/terms/questions/catalog/conditions/text_lesser_than_equal': (0, 0, False), 'http://example.com/terms/questions/catalog/conditions/text_not_empty': (1, 1, True), - 'http://example.com/terms/questions/catalog/conditions/text_not_equal': (1, 1, False), - 'http://example.com/terms/questions/catalog/conditions/option_empty': (1, 1, False), + 'http://example.com/terms/questions/catalog/conditions/text_not_equal': (0, 0, False), + 'http://example.com/terms/questions/catalog/conditions/option_empty': (0, 0, False), 'http://example.com/terms/questions/catalog/conditions/option_equal': (1, 1, True), 'http://example.com/terms/questions/catalog/conditions/option_not_empty': (1, 1, True), - 'http://example.com/terms/questions/catalog/conditions/option_not_equal': (1, 1, False), + 'http://example.com/terms/questions/catalog/conditions/option_not_equal': (0, 0, False), 'http://example.com/terms/questions/catalog/conditions/set': (0, 2, True), 'http://example.com/terms/questions/catalog/conditions/set_set': (0, 2, True), 'http://example.com/terms/questions/catalog/conditions/optionset': (0, 2, True), 'http://example.com/terms/questions/catalog/conditions/text_set': (0, 2, True), - 'http://example.com/terms/questions/catalog/blocks': (9, 12), - 'http://example.com/terms/questions/catalog/blocks/set': (9, 12, True), + 'http://example.com/terms/questions/catalog/blocks': (9, 18), + 'http://example.com/terms/questions/catalog/blocks/set': (9, 18, True), } diff --git a/rdmo/projects/tests/test_progress.py b/rdmo/projects/tests/test_progress.py index 2b653dc4cd..d153df624b 100644 --- a/rdmo/projects/tests/test_progress.py +++ b/rdmo/projects/tests/test_progress.py @@ -6,7 +6,7 @@ projects = [1, 11] results_map = { - 1: (58, 88), + 1: (58, 94), 11: (0, 36) } diff --git a/rdmo/projects/tests/test_viewset_project_progress.py b/rdmo/projects/tests/test_viewset_project_progress.py index 45f900d68f..5d7529261f 100644 --- a/rdmo/projects/tests/test_viewset_project_progress.py +++ b/rdmo/projects/tests/test_viewset_project_progress.py @@ -119,7 +119,7 @@ def test_progress_post_unchanged(db, client): project = Project.objects.get(id=1) project.progress_count = progress_count = 58 # the progress in the fixture is not up-to-date - project.progress_total = progress_total = 88 + project.progress_total = progress_total = 94 project.save() project.refresh_from_db() project_updated = project.updated