From 901a324cc338e2beee399eac1c9886adaff08956 Mon Sep 17 00:00:00 2001 From: Thomas Thorogood Date: Wed, 27 Oct 2021 17:05:16 -0700 Subject: [PATCH] [EDS-606][EDS-608][EDS-613] Use "experimental" query as default for name searches (#122) * Make the new "experimental" search mode the default for name searches * [EDS-613] Fix nesting of footer in the DOM * Remove unnecessary leading indent in footer.html * Remove deprecated name query generation from query_generator.py * Remove deprecated name query generation from query_generator.py * [Bot] Update version to 2.1.0 Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com> --- husky_directory/models/enum.py | 10 - husky_directory/models/search.py | 4 +- husky_directory/services/query_generator.py | 369 +----------------- husky_directory/services/search.py | 13 +- husky_directory/settings/base.dotenv | 1 + husky_directory/templates/base.html | 2 +- husky_directory/templates/footer.html | 64 +-- husky_directory/templates/search_options.html | 1 - .../templates/search_options/search_type.html | 41 -- pyproject.toml | 2 +- tests/blueprints/test_search_blueprint.py | 77 ++-- tests/conftest.py | 12 +- tests/services/test_query_generator.py | 149 +------ tests/services/test_search.py | 10 +- 14 files changed, 102 insertions(+), 653 deletions(-) delete mode 100644 husky_directory/templates/search_options/search_type.html diff --git a/husky_directory/models/enum.py b/husky_directory/models/enum.py index d575cf35..e5b606fa 100644 --- a/husky_directory/models/enum.py +++ b/husky_directory/models/enum.py @@ -24,13 +24,3 @@ class PopulationType(Enum): class ResultDetail(Enum): full = "full" summary = "summary" - - -class SearchType(Enum): - # Old-school cool. It's inefficient but - # very explicit. - classic = "classic" - - # In active development, constantly striving to - # provide more relevant results, faster! - experimental = "experimental" diff --git a/husky_directory/models/search.py b/husky_directory/models/search.py index f8016ae1..ffbfe4c8 100644 --- a/husky_directory/models/search.py +++ b/husky_directory/models/search.py @@ -11,7 +11,7 @@ from husky_directory.models.base import DirectoryBaseModel from husky_directory.models.common import UWDepartmentRole -from husky_directory.models.enum import PopulationType, ResultDetail, SearchType +from husky_directory.models.enum import PopulationType, ResultDetail # There is no direct dependency on the model here, we only need it for type checking; # this protects us from accidentally creating a cyclic dependency between the modules. @@ -59,7 +59,6 @@ class SearchDirectoryFormInput(DirectoryBaseModel): render_query: Optional[str] render_population: Optional[PopulationType] render_length: Optional[ResultDetail] - search_type: SearchType = SearchType.classic include_test_identities: bool = False # Not currently supported @@ -136,7 +135,6 @@ class SearchDirectoryInput(DirectoryBaseModel): population: PopulationType = PopulationType.employees include_test_identities: bool = False person_href: Optional[str] - search_type: SearchType = SearchType.classic @classmethod def search_methods(cls) -> List[str]: diff --git a/husky_directory/services/query_generator.py b/husky_directory/services/query_generator.py index 6133797c..874efad0 100644 --- a/husky_directory/services/query_generator.py +++ b/husky_directory/services/query_generator.py @@ -1,76 +1,15 @@ from __future__ import annotations -from functools import partial -from itertools import product -from typing import Any, Callable, Iterable, List, Optional, Tuple, Union +from typing import Any, Iterable, Tuple, Union -import inflection from flask_injector import request from injector import inject from pydantic import BaseModel, EmailError, validate_email -from husky_directory.models.common import RecordConstraint from husky_directory.models.enum import AffiliationState, PopulationType from husky_directory.models.pws import ListPersonsInput from husky_directory.models.search import SearchDirectoryInput from husky_directory.services.auth import AuthService -from husky_directory.util import ConstraintPredicates - - -class QueryTemplate(BaseModel): - """ - Due to the combinatoric complexity of some of our existing name queries, it's less - maintenance (and less wrist strain) to simply generate our queries, rather than hard-code them. - """ - - # This string should describe what the query "scenario" is, and should - # (by default) accept as many format placeholders (`{}`) as there will be - # args supplied by the caller. - # Example: - # Good: 'a: {}, b: {}, c: {}' when called against [1, 2, 3] or ['zebra', 'bottle', 'phone'] - # Bad: . . . when called against [1] or ['bottle', 'phone', 'tablet', 'monkey'] - # - # However, if you want to have more control over this, you can supply the description_formatter function. - description_fmt: str - - # Allows you to change how your description is formatted based on the expected arguments. - # For example, if you want to accept a variable number of arguments, or recombine the arguments, - # you can provide a function to do so. - # Given a description_formatter of: lambda first, *others: (first, " ".join(others)) - # and a description_fmt of: "First name {}, other terms: {}" - # when called against ["Mary", "J", "Blige"] - # the result would be: ["Mary", "J Blige"] - # See tests/services/test_query_generator.py for more examples. - description_formatter: Optional[Callable[[str], Iterable[str]]] - - # The query generator is similar to the description formatter, but creates a query based on the - # provided arguments instead. For example, continuing the "Mary J Blige" example: - # Given a query_generator of: lambda first, *others: Query(first_name=first, last_name=' '.join(others)) - # then the result would be: Query(first_name="Mary", last_name="J Blige") - # See tests/services/test_query_generator.py for more examples - query_generator: Callable[ # A function or other callable - [str], # That takes a list of strings (the "words" of the query) - ListPersonsInput, # and returns the query itself based on those args. - ] - - # Constraints can be set as an added value to filter the output. - # Constraints are not sent to PWS, but are used by PersonWebServiceClient to - # add additional filters to the output. - constraints: List[RecordConstraint] = [] - - def get_query(self, args: List[str]) -> ListPersonsInput: - """Given a list of arguments, creates a query by calling the query generator.""" - return self.query_generator(*args) - - def get_description(self, args: List[str]) -> str: - """ - Given a list of arguments, creates a human-readable description of the query by calling the - description_formatter, if it exists. Otherwise, simply uses the string.format() function with the - args as-is. - """ - if self.description_formatter: - return self.description_fmt.format(*self.description_formatter(*args)) - return self.description_fmt.format(*args) class WildcardFormat: @@ -128,316 +67,10 @@ class SearchQueryGenerator: # These are just hard-coded templates for the 1- and 2-arg scenarios ("Madonna," "Retta," "Jeff Goldbum") # We could add hard-coded templates for higher-cardinality searches, but instead, it's better to just # autogenerate them. See also: _generate_sliced_name_queries(). - name_query_templates = { - 1: [ - QueryTemplate( - description_fmt='Last name is "{}"', - query_generator=lambda name: Query( - last_name=name, - constraints=[ - RecordConstraint( - namespace="PreferredSurname", - predicate=partial( - ConstraintPredicates.null_or_matches, name - ), - ) - ], - ), - ), - QueryTemplate( - description_fmt='Last name begins with "{}"', - query_generator=lambda name: Query( - last_name=ArgFmt.begins_with(name), - constraints=[ - RecordConstraint( - namespace="PreferredSurname", - predicate=partial( - ConstraintPredicates.null_or_begins_with, name - ), - ) - ], - ), - ), - QueryTemplate( - description_fmt='First name is "{}"', - query_generator=lambda name: Query( - first_name=name, - constraints=[ - RecordConstraint( - namespace="PreferredFirstName", - predicate=partial( - ConstraintPredicates.null_or_matches, name - ), - ) - ], - ), - ), - QueryTemplate( - description_fmt='Name contains: "{}"', - query_generator=lambda name: Query( - display_name=ArgFmt.contains(name), - constraints=[ - RecordConstraint( - namespace="DisplayName", - predicate=partial( - ConstraintPredicates.null_or_includes, name - ), - ) - ], - ), - ), - ], - 2: [ - QueryTemplate( - description_fmt='First name is "{}", last name is "{}"', - query_generator=lambda first, last: Query( - first_name=first, - last_name=last, - constraints=[ - RecordConstraint( - namespace="PreferredFirstName", - predicate=partial( - ConstraintPredicates.null_or_matches, first - ), - ), - RecordConstraint( - namespace="PreferredSurname", - predicate=partial( - ConstraintPredicates.null_or_matches, last - ), - ), - ], - ), - ), - QueryTemplate( - description_fmt='First name begins with "{}", last name is "{}"', - query_generator=lambda first, last: Query( - first_name=ArgFmt.begins_with(first), - last_name=last, - constraints=[ - RecordConstraint( - namespace="PreferredFirstName", - predicate=partial( - ConstraintPredicates.null_or_begins_with, first - ), - ), - RecordConstraint( - namespace="PreferredSurname", - predicate=partial( - ConstraintPredicates.null_or_matches, last - ), - ), - ], - ), - ), - QueryTemplate( - description_fmt='First name is "{}", last name begins with "{}"', - query_generator=lambda first, last: Query( - first_name=first, - last_name=ArgFmt.begins_with(last), - constraints=[ - RecordConstraint( - namespace="PreferredFirstName", - predicate=partial( - ConstraintPredicates.null_or_matches, first - ), - ), - RecordConstraint( - namespace="PreferredSurname", - predicate=partial( - ConstraintPredicates.null_or_begins_with, last - ), - ), - ], - ), - ), - QueryTemplate( - description_fmt='First name begins with "{}", last name begins with "{}"', - query_generator=lambda first, last: Query( - first_name=ArgFmt.begins_with(first), - last_name=ArgFmt.begins_with(last), - constraints=[ - RecordConstraint( - namespace="PreferredFirstName", - predicate=partial( - ConstraintPredicates.null_or_begins_with, first - ), - ), - RecordConstraint( - namespace="PreferredSurname", - predicate=partial( - ConstraintPredicates.null_or_begins_with, last - ), - ), - ], - ), - ), - QueryTemplate( - description_fmt='Last name begins with "{} {}"', - query_generator=lambda *args: Query( - last_name=ArgFmt.begins_with(*args), - constraints=[ - RecordConstraint( - namespace="PreferredFirstName", - predicate=partial( - ConstraintPredicates.null_or_begins_with, " ".join(args) - ), - ), - ], - ), - ), - ], - } - @inject def __init__(self, auth: AuthService): self.auth = auth - @staticmethod - def _build_sliced_name_query_template( - slice: int = 1, - pre_slice_format=ArgFmt.matches, - post_slice_format=ArgFmt.matches, - ) -> QueryTemplate: - post_slice_label = "rest" - if slice == 1: - pre_slice_label = "First name" - else: - pre_slice_label = "First/middle name" - - pre_slice_query_name = inflection.humanize(pre_slice_format.__name__) - post_slice_query_name = inflection.humanize(post_slice_format.__name__) - - def _generate_query(*args: str): - first_name = pre_slice_format(*args[0:slice]) - last_name = post_slice_format(*args[slice:]) - return Query( - first_name=first_name, - last_name=last_name, - constraints=[ - RecordConstraint( - namespace="PreferredFirstName", - predicate=partial( - ConstraintPredicates.null_or_begins_with, first_name - ), - ), - RecordConstraint( - namespace="PreferredSurname", - predicate=partial( - ConstraintPredicates.null_or_begins_with, last_name - ), - ), - ], - ) - - # Ugh; this is so ugly, but it beat hard-coding all of these use cases. I don't have the stamina to type - # that much. See the QueryTemplate documentation for deeper details. - return QueryTemplate( - # The end result of the description looks like: - # 'First name begins with "Husky", rest matches "Dawg"' - # (or any permutations thereof) - description_fmt=f'{pre_slice_label} {pre_slice_query_name} "{{}}", ' - f'{post_slice_label} {post_slice_query_name} "{{}}"', - # Here we split and rejoin the args at their slice index, creating two total sets of arguments; - # one acting as the "first name," and the other acting as the "last name." Either or both may contain - # wildcards, or be blank. See the tests/services/test_query_generator.py for examples. - description_formatter=lambda *args: ( - " ".join(args[0:slice]), - " ".join(args[slice:]), - ), - # This is similar to the description_formatter, except it's formatting those same - # slices with the *_slice_format function parameters. - # See tests/services/test_query_generator.py for examples. - query_generator=_generate_query, - ) - - def _generate_sliced_name_queries(self, max_slice: int) -> Iterable[QueryTemplate]: - scenario_components = list( - product([ArgFmt.matches, ArgFmt.begins_with], repeat=2) - ) - - # This generates query templates whose generator functions will iteratively slice the - # array of args into the "first"/"last" combinations. - # Example: - # ["a", "b", "c"] => ( - # ["a", "b c"], - # ["a b", "c"], - # ) - for i in range(1, max_slice): - for pre_format, post_format in scenario_components: - yield self._build_sliced_name_query_template( - slice=i, pre_slice_format=pre_format, post_slice_format=post_format - ) - - # After we do our combinatorics above, we move onto any other general query templates we want to add. - yield QueryTemplate( - # End result looks like: - # Name parts begin with: a/b/c - description_fmt="Name parts begin with: {}", - description_formatter=lambda *args: ("/".join(args),), - # Generates a query that looks like: "a* b* c*" - query_generator=lambda *args: Query( - display_name=ArgFmt.begins_with(*args, each=True) - ), - ) - - def _generate_short_name_queries(self, name: str) -> GeneratedQuery: - """ - For very short queries, we should make the assumption that the user - is search for a short first name or short surname. We should NOT - do "contains" searches as there are likely to be a lot of irrelevant results.gtgt - :param name: - :return: - """ - yield GeneratedQuery( - description=f'First name starts with "{name}"', - request_input=ListPersonsInput( - employee_affiliation_state=AffiliationState.current, - display_name=f"{name}*", - ), - ) - yield GeneratedQuery( - description=f'Last name starts with "{name}"', - request_input=ListPersonsInput( - employee_affiliation_state=AffiliationState.current, - display_name=f"* {name}*", - ), - ) - - def generate_name_queries(self, name: str) -> Tuple[str, ListPersonsInput]: - if len(name) == 2: - yield from self._generate_short_name_queries(name) - return - - # No matter the case, we will always be checking for an exact match - yield GeneratedQuery( - description=f'Name matches "{name}"', - request_input=ListPersonsInput(display_name=name), - ) - - # If the request contains a wildcard, we don't make any further "guesses" as to - # what the user wants, because they've told us they think they know what they're doing. - # This saves us a lot of queries and guesswork. - if "*" in name: - return - - name_parts = name.split() - cardinality = len(name_parts) - - if cardinality in self.name_query_templates: - for template in self.name_query_templates[cardinality]: - yield GeneratedQuery( - description=template.get_description(name_parts), - request_input=template.get_query(name_parts), - ) - return - - for template in self._generate_sliced_name_queries(cardinality): - yield GeneratedQuery( - description=template.get_description(name_parts), - request_input=template.get_query(name_parts), - ) - def generate_department_queries( self, department: str, include_alt_queries: bool = True ) -> Tuple[str, ListPersonsInput]: diff --git a/husky_directory/services/search.py b/husky_directory/services/search.py index 79def50f..d509a021 100644 --- a/husky_directory/services/search.py +++ b/husky_directory/services/search.py @@ -6,7 +6,7 @@ from flask_injector import request from injector import inject -from husky_directory.models.enum import AffiliationState, SearchType +from husky_directory.models.enum import AffiliationState from husky_directory.models.pws import ( ListPersonsInput, ListPersonsOutput, @@ -118,7 +118,9 @@ def search_directory_experimental( pws_output = self._pws.get_explicit_href( pws_output.next.href, output_type=ListPersonsOutput ) - self.reducer.reduce_output(pws_output, request_input.name, results) + results = self.reducer.reduce_output( + pws_output, request_input.name, results + ) statistics.aggregate(pws_output.request_statistics) statistics.num_duplicates_found = self.reducer.duplicate_hit_count @@ -154,9 +156,6 @@ def search_directory_classic( scenarios: List[DirectoryQueryScenarioOutput] = [] scenario_description_indexes: Dict[str, int] = {} - if request_input.name: - statistics.num_user_search_tokens = len(request_input.name.split()) - for generated in self.query_generator.generate(request_input): self.logger.debug( f"Querying: {generated.description} with " @@ -211,12 +210,10 @@ def search_directory( and returns a DirectoryQueryScenarioOutput.""" if ( - SearchType(request_input.search_type) == SearchType.experimental # Only name search is implemented in experimental mode right now. - and request_input.name + request_input.name # Wildcard searches are already accounted for in "classic" mode. and "*" not in request_input.name ): return self.search_directory_experimental(request_input) - return self.search_directory_classic(request_input) diff --git a/husky_directory/settings/base.dotenv b/husky_directory/settings/base.dotenv index 49c85f9b..17ad7c4a 100644 --- a/husky_directory/settings/base.dotenv +++ b/husky_directory/settings/base.dotenv @@ -1,3 +1,4 @@ +FLASK_ENV=development UWCA_CERT_NAME=uwca UWCA_CERT_PATH=/app/certificates PWS_HOST=https://it-wseval1.s.uw.edu diff --git a/husky_directory/templates/base.html b/husky_directory/templates/base.html index 84a7d0f3..2469fa0e 100644 --- a/husky_directory/templates/base.html +++ b/husky_directory/templates/base.html @@ -44,8 +44,8 @@

UW Directory

{% block content %}{% endblock %} {% include "links.html" %} - {% include "footer.html" %} +{% include "footer.html" %} diff --git a/husky_directory/templates/footer.html b/husky_directory/templates/footer.html index 43aa47c8..e392cd6c 100644 --- a/husky_directory/templates/footer.html +++ b/husky_directory/templates/footer.html @@ -1,32 +1,32 @@ -
- University of Washington -

Be boundless

 -

Connect with us:

- - - -
+
+ University of Washington +

Be boundless

 +

Connect with us:

+ + + +
diff --git a/husky_directory/templates/search_options.html b/husky_directory/templates/search_options.html index e7da9ffa..09240e7c 100644 --- a/husky_directory/templates/search_options.html +++ b/husky_directory/templates/search_options.html @@ -47,7 +47,6 @@

Search options

{% endif %} - {% include "search_options/search_type.html" %} {% set selected_length = 'summary' %} {% if request_input is not blank %} diff --git a/husky_directory/templates/search_options/search_type.html b/husky_directory/templates/search_options/search_type.html deleted file mode 100644 index 206883a7..00000000 --- a/husky_directory/templates/search_options/search_type.html +++ /dev/null @@ -1,41 +0,0 @@ -
-{# - This component is unlikely to ever be seen by "real" users; - however, it's a way for developers to easily switch between - search modes when testing. - - Once the new search function is preferred over the - classic, we can remove this component. -#} -{% if show_experimental %} - {% if request_input is not blank %} - {% set search_type = request_input['search_type'] %} - {% else %} - {% set search_type = 'classic' %} - {% endif %} -
-

Search Method

- - -{% endif %} -
diff --git a/pyproject.toml b/pyproject.toml index 6839496e..28d4c6be 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "uw-husky-directory" -version = "2.0.13" +version = "2.1.0" description = "An updated version of the UW Directory" authors = ["Thomas Thorogood "] license = "MIT" diff --git a/tests/blueprints/test_search_blueprint.py b/tests/blueprints/test_search_blueprint.py index 1b03fdf6..3ead3987 100644 --- a/tests/blueprints/test_search_blueprint.py +++ b/tests/blueprints/test_search_blueprint.py @@ -10,6 +10,7 @@ from werkzeug.local import LocalProxy from husky_directory.models.enum import PopulationType, ResultDetail +from husky_directory.models.pws import ListPersonsInput, ListPersonsOutput from husky_directory.models.search import SearchDirectoryFormInput, SearchDirectoryInput from husky_directory.services.pws import PersonWebServiceClient @@ -35,8 +36,12 @@ def initialize(self, client, mock_people, injector, html_validator, mock_injecte class TestSearchBlueprint(BlueprintSearchTestBase): - def test_render_summary_success(self): - response = self.flask_client.post("/", data={"method": "name", "query": "foo"}) + def test_render_summary_success( + self, search_method: str = "name", search_query: str = "lovelace" + ): + response = self.flask_client.post( + "/", data={"method": search_method, "query": search_query} + ) assert response.status_code == 200 profile = self.mock_people.contactable_person with self.html_validator.validate_response(response) as html: @@ -51,6 +56,23 @@ def test_render_summary_success(self): ) assert "autofocus" not in html.find("input", attrs={"name": "query"}).attrs + def _set_up_multipage_search(self): + page_one = self.mock_people.as_search_output( + next_=ListPersonsInput(href="https://foo/page-2") + ) + page_two = self.mock_send_request.return_value + self.mock_send_request.return_value = page_one + mock_next_page = mock.patch.object(self.pws_client, "get_explicit_href").start() + mock_next_page.return_value = ListPersonsOutput.parse_obj(page_two) + + def test_render_multi_page_experimental_search(self): + self._set_up_multipage_search() + self.test_render_summary_success() + + def test_render_multi_page_classic_search(self): + self._set_up_multipage_search() + self.test_render_summary_success(search_method="email", search_query="foo") + def test_copyright_footer(self): response = self.flask_client.get("/") assert response.status_code == 200 @@ -67,18 +89,24 @@ def test_render_full_success(self, log_in): assert self.session.get("uwnetid") response = self.flask_client.post( - "/", data={"query": "lovelace", "length": "full", "population": "all"} + "/", + data={ + "query": "lovelace", + "method": "name", + "length": "full", + "population": "all", + }, ) profile = self.mock_people.contactable_person with self.html_validator.validate_response(response): self.html_validator.assert_has_tag_with_text("h4", profile.display_name) self.html_validator.assert_has_scenario_anchor( - "employees-name-matches-lovelace" + "employees-last-name-is-lovelace" ) if log_in: self.html_validator.assert_has_scenario_anchor( - "students-name-matches-lovelace" + "students-last-name-is-lovelace" ) with self.html_validator.scope("ul", class_="dir-listing"): self.html_validator.assert_has_tag_with_text( @@ -93,17 +121,17 @@ def test_render_full_success(self, log_in): def test_render_no_results(self): self.mock_send_request.return_value = self.mock_people.as_search_output() - response = self.flask_client.post("/", data={"query": "foo"}) + response = self.flask_client.post("/", data={"query": "lovelace"}) with self.html_validator.validate_response(response) as html: self.html_validator.assert_not_has_scenario_anchor( - "employees-name-matches-foo" + "employees-name-matches-lovelace" ) self.html_validator.assert_not_has_scenario_anchor( - "students-name-matches-foo" + "students-name-matches-lovelace" ) assert not html.find("table", summary="results") assert html.find(string=re.compile("No matches for")) - self.html_validator.assert_has_tag_with_text("b", 'Name is "foo"') + self.html_validator.assert_has_tag_with_text("b", 'Name is "lovelace"') def test_render_invalid_box_number(self): response = self.flask_client.post( @@ -126,16 +154,18 @@ def test_render_full_no_box_number(self): self.flask_client.post( "/", data={ - "query": "foo", + "query": "lovelace", "length": "full", "population": "all", }, ) ) as html: assert not html.find_all("li", class_="dir-boxstuff") - self.html_validator.assert_has_scenario_anchor("students-name-matches-foo") + self.html_validator.assert_has_scenario_anchor( + "students-last-name-is-lovelace" + ) self.html_validator.assert_not_has_scenario_anchor( - "employees-name-matches-foo" + "employees-last-name-is-lovelace" ) with self.html_validator.scope("div", class_="usebar"): self.html_validator.assert_has_submit_button_with_text("Download vcard") @@ -167,7 +197,7 @@ def test_user_stays_logged_in_after_search(self): self.flask_client.post( "/", data={ - "query": "foo", + "query": "lovelace", "method": "name", }, ) @@ -322,7 +352,7 @@ def test_render_form_option_stickiness( based on its input. """ query_value = ( - "abcdefg" if search_field not in ("phone", "box_number") else "12345" + "lovelace" if search_field not in ("phone", "box_number") else "12345" ) request = SearchDirectoryFormInput( @@ -354,19 +384,11 @@ def test_get_person_non_matching_surname(self, search_term): """ profile = self.mock_people.published_employee profile.preferred_last_name = "Smith" - empty_results = self.mock_people.as_search_output() + profile.display_name = "Ada Smith" expected_num_results = 1 if profile.preferred_last_name == search_term else 0 - outputs = [ - empty_results, - self.mock_people.as_search_output(profile), - ] - - def mock_get(*args, **kwargs): - return outputs.pop(0) if outputs else empty_results - - self.mock_send_request.side_effect = mock_get + self.mock_send_request.return_value = self.mock_people.as_search_output(profile) response = self.flask_client.post( "/", data={"method": "name", "query": search_term} @@ -375,10 +397,9 @@ def mock_get(*args, **kwargs): with self.html_validator.validate_response(response) as html: assert ( len(html.find_all("tr", class_="summary-row")) == expected_num_results - ) + ), str(html) - @pytest.mark.parametrize("search_type", ("classic", "experimental")) - def test_list_people_sort(self, random_string, search_type): + def test_list_people_sort(self, random_string): ada_1 = self.mock_people.published_employee.copy( update={ "display_name": "Ada Zlovelace", @@ -412,7 +433,7 @@ def test_list_people_sort(self, random_string, search_type): self.mock_send_request.return_value = people response = self.flask_client.post( "/", - data={"method": "name", "query": "lovelace", "search_type": search_type}, + data={"method": "name", "query": "lovelace"}, ) html = response.data.decode("UTF-8") assert response.status_code == 200 diff --git a/tests/conftest.py b/tests/conftest.py index 8bfeb8a8..fc617a68 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -92,8 +92,6 @@ class People: cases, it's probably better to just declare them in situ. """ - no_affiliations = generate_person() - test_entity = generate_person(is_test_entity=True) published_employee = generate_person( affiliations=PersonAffiliations( employee=EmployeePersonAffiliation( @@ -110,15 +108,7 @@ class People: ) ), ) - unpublished_employee = generate_person( - affiliations=PersonAffiliations( - employee=EmployeePersonAffiliation( - directory_listing=EmployeeDirectoryListing( - publish_in_directory=False - ) - ) - ) - ) + published_student = generate_person( affiliations=PersonAffiliations( student=StudentPersonAffiliation( diff --git a/tests/services/test_query_generator.py b/tests/services/test_query_generator.py index bab1ce1e..66fd26ac 100644 --- a/tests/services/test_query_generator.py +++ b/tests/services/test_query_generator.py @@ -1,5 +1,3 @@ -from typing import Dict, Optional - import pytest from injector import Injector from werkzeug.local import LocalProxy @@ -7,40 +5,11 @@ from husky_directory.models.enum import PopulationType from husky_directory.models.search import SearchDirectoryInput from husky_directory.services.query_generator import ( - Query, - QueryTemplate, SearchQueryGenerator, WildcardFormat, ) -class TestQueryTemplate: - @pytest.fixture(autouse=True) - def initialize(self): - self.template = QueryTemplate( - description_fmt='First name "{}", last name "{}"', - query_generator=lambda first, last: Query(first_name=first, last_name=last), - ) - self.args = ["Bill", "Waterson"] - - def test_get_query(self): - query = self.template.get_query(self.args) - assert query.first_name == "Bill" - assert query.last_name == "Waterson" - - def test_get_description(self): - description = self.template.get_description(self.args) - assert description == 'First name "Bill", last name "Waterson"' - - def test_description_formatter(self): - self.template.description_formatter = lambda first, last: ( - " ".join([first, last]), - ) - self.template.description_fmt = 'Name "{}"' - description = self.template.get_description(self.args) - assert description == 'Name "Bill Waterson"' - - @pytest.mark.parametrize( "fmt, expected", [ @@ -66,120 +35,10 @@ def initialize(self, injector: Injector, mock_injected): def test_wildcard_input(self): generated = list( - self.query_generator.generate(SearchDirectoryInput(name="foo*")) + self.query_generator.generate(SearchDirectoryInput(email="foo*")) ) assert len(generated) == 1 - assert generated[0].description == 'Name matches "foo*"' - - def test_single_name_input(self): - generated = list( - self.query_generator.generate(SearchDirectoryInput(name="foo")) - ) - assert len(generated) == 5 # 1 global query, 4 from name_query_templates[1] - expected_queries = [ - dict(display_name="foo"), - dict(last_name="foo"), - dict(last_name="foo*"), - dict(first_name="foo"), - dict(display_name="*foo*"), - ] - actual_queries = [ - query.request_input.dict( - exclude_unset=True, - exclude_none=True, - exclude_defaults=True, - exclude={"constraints"}, - ) - for query in generated - ] - assert expected_queries == actual_queries - - def test_short_name_input(self): - generated = list(self.query_generator.generate(SearchDirectoryInput(name="hi"))) - assert len(generated) == 2 - assert generated[0].description == 'First name starts with "hi"' - assert generated[1].description == 'Last name starts with "hi"' - - def test_two_name_input(self): - generated = list( - self.query_generator.generate(SearchDirectoryInput(name="foo bar")) - ) - assert len(generated) == 6 # 1 global query, 5 fro name_query_templates[2] - expected_queries = [ - dict(display_name="foo bar"), - dict(first_name="foo", last_name="bar"), - dict(first_name="foo*", last_name="bar"), - dict(first_name="foo", last_name="bar*"), - dict(first_name="foo*", last_name="bar*"), - dict(last_name="foo bar*"), - ] - actual_queries = [ - query.request_input.dict( - exclude_unset=True, - exclude_none=True, - exclude_defaults=True, - exclude={"constraints"}, - ) - for query in generated - ] - assert expected_queries == actual_queries - - @pytest.mark.parametrize( - "request_input, expected_num_queries, assert_included", - [ - ( - "foo bar baz", - 10, - [ - dict(display_name="foo bar baz"), - dict(display_name="foo* bar* baz*"), - dict(first_name="foo", last_name="bar baz"), - dict(first_name="foo", last_name="bar baz*"), - dict(first_name="foo*", last_name="bar baz"), - dict(first_name="foo*", last_name="bar baz*"), - dict(first_name="foo bar", last_name="baz"), - dict(first_name="foo bar", last_name="baz*"), - dict(first_name="foo bar*", last_name="baz"), - dict(first_name="foo bar*", last_name="baz*"), - ], - ), - # Skipping the detailed tests for the cardinalities, because that's a whole lot of typing. Just checking - # a couple of cases to verify that the slicing is being handled as expected. - ( - "foo bar baz bop", - 14, - [ - dict(first_name="foo", last_name="bar baz bop"), - dict(first_name="foo bar", last_name="baz bop"), - dict(first_name="foo bar baz", last_name="bop"), - ], - ), - ("foo bar baz bop blop", 18, None), - ], - ) - def test_multi_name_input_generation( - self, - request_input: str, - expected_num_queries: int, - assert_included: Optional[Dict[str, str]], - ): - generated = list( - self.query_generator.generate(SearchDirectoryInput(name=request_input)) - ) - if assert_included: - actual_queries = [ - query.request_input.dict( - exclude_unset=True, - exclude_none=True, - exclude_defaults=True, - exclude={"constraints"}, - ) - for query in generated - ] - for case in assert_included: - assert case in actual_queries - - assert len(generated) == expected_num_queries + assert generated[0].description == 'Email matches "foo*"' def test_phone_input_short_number(self): request_input = SearchDirectoryInput(phone="2065554321") @@ -277,7 +136,7 @@ def test_query_all_populations(self, authenticate: bool): if authenticate: self.session["uwnetid"] = "foo" request_input = SearchDirectoryInput( - name="*whatever", population=PopulationType.all + email="*whatever", population=PopulationType.all ) queries = list(self.query_generator.generate(request_input)) @@ -292,7 +151,7 @@ def test_query_student_population(self, authenticate: bool): if authenticate: self.session["uwnetid"] = "foo" request_input = SearchDirectoryInput( - name="*whatever", population=PopulationType.students + email="*whatever", population=PopulationType.students ) queries = list(self.query_generator.generate(request_input)) diff --git a/tests/services/test_search.py b/tests/services/test_search.py index 5e050230..930eb7b6 100644 --- a/tests/services/test_search.py +++ b/tests/services/test_search.py @@ -59,7 +59,7 @@ def set_get_next_output(self, output: ListPersonsOutput): def test_search_directory_happy(self): request_input = SearchDirectoryInput( - name="foo", population=PopulationType.employees + name="lovelace", population=PopulationType.employees ) output = self.client.search_directory(request_input) @@ -86,7 +86,9 @@ def test_output_includes_phones(self): person = self.mock_people.contactable_person self.list_persons_output["Persons"] = [person.dict(by_alias=True)] self.session["uwnetid"] = "foo" - request_input = SearchDirectoryInput(name="foo", population=PopulationType.all) + request_input = SearchDirectoryInput( + name="lovelace", population=PopulationType.all + ) output = self.client.search_directory(request_input) contacts = output.scenarios[0].populations["employees"].people[0].phone_contacts @@ -184,7 +186,7 @@ def test_department_ignores_invalid_data(self): person.affiliations.employee.directory_listing.positions[0].department = None self.list_persons_output["Persons"] = [person.dict(by_alias=True)] request_input = SearchDirectoryInput( - name="whatever", population=PopulationType.employees + name="lovelace", population=PopulationType.employees ) output = self.client.search_directory(request_input) output_person: Person = output.scenarios[0].populations["employees"].people[0] @@ -194,7 +196,7 @@ def test_output_includes_box_number(self): person = self.mock_people.published_employee self.list_persons_output["Persons"] = [person.dict(by_alias=True)] output = self.client.search_directory( - SearchDirectoryInput(name="*blah", population=PopulationType.employees) + SearchDirectoryInput(email="*blah", population=PopulationType.employees) ) output_person: Person = output.scenarios[0].populations["employees"].people[0] assert output_person.box_number == "351234"