diff --git a/isic/ingest/api.py b/isic/ingest/api.py index b61e96c3..a4cea612 100644 --- a/isic/ingest/api.py +++ b/isic/ingest/api.py @@ -18,6 +18,7 @@ from isic.ingest.models.lesion import get_lesion_count_for_user from isic.ingest.services.accession import create_accession from isic.ingest.services.accession.review import bulk_create_accession_reviews +from isic.ingest.services.contributor import compute_contributor_merge_impact from isic.ingest.tasks import update_metadata_task lesion_router = Router() @@ -265,6 +266,45 @@ def contributor_list(request: HttpRequest): ) +class MergeImpactUserOut(Schema): + id: int + name: str + email: str + has_engagement_profile: bool + + +class MergeImpactContributorOut(Schema): + id: int + institution_name: str + accession_count: int + published_image_count: int + + +class ContributorMergeImpactOut(Schema): + dest_contributor: MergeImpactContributorOut + src_contributor: MergeImpactContributorOut + users_gaining_access_to_dest: list[MergeImpactUserOut] + users_gaining_access_to_src: list[MergeImpactUserOut] + engagement_profiles_repointed: list[MergeImpactUserOut] + + +@contributor_router.get( + "/merge-impact/", + response={200: ContributorMergeImpactOut, 400: dict}, + summary="Describe the access impact of merging two contributors.", + include_in_schema=False, + auth=is_staff, +) +def contributor_merge_impact(request: HttpRequest, dest_contributor: int, src_contributor: int): + if dest_contributor == src_contributor: + return 400, {"error": "The two contributors must be different."} + + return 200, compute_contributor_merge_impact( + dest_contributor=get_object_or_404(Contributor, id=dest_contributor), + src_contributor=get_object_or_404(Contributor, id=src_contributor), + ) + + @contributor_router.get( "/{id}/", response=ContributorDetailOut, diff --git a/isic/ingest/services/contributor/__init__.py b/isic/ingest/services/contributor/__init__.py index c505c855..86c6a634 100644 --- a/isic/ingest/services/contributor/__init__.py +++ b/isic/ingest/services/contributor/__init__.py @@ -1,6 +1,10 @@ +from dataclasses import dataclass + from django.contrib.auth.models import User from django.db import transaction +from django.db.models import Count, Q +from isic.ingest.models.accession import Accession from isic.ingest.models.contributor import Contributor @@ -28,6 +32,101 @@ def create_contributor( # noqa: PLR0913 return contributor +@dataclass(frozen=True) +class MergeImpactUser: + id: int + name: str + email: str + has_engagement_profile: bool + + +@dataclass(frozen=True) +class MergeImpactContributor: + id: int + institution_name: str + accession_count: int + published_image_count: int + + +@dataclass(frozen=True) +class ContributorMergeImpact: + dest_contributor: MergeImpactContributor + src_contributor: MergeImpactContributor + # owners of the source that aren't owners of the destination yet, they gain access to + # everything the destination already has. + users_gaining_access_to_dest: list[MergeImpactUser] + # owners of the destination that aren't owners of the source, they gain access to the + # source's data once it moves under the destination. + users_gaining_access_to_src: list[MergeImpactUser] + engagement_profiles_repointed: list[MergeImpactUser] + + +def _merge_impact_contributor(contributor: Contributor) -> MergeImpactContributor: + counts = Accession.objects.filter(cohort__contributor=contributor).aggregate( + accession_count=Count("id"), + # mirrors AccessionQuerySet.published, an accession is published once it has an image + published_image_count=Count("id", filter=Q(image__isnull=False)), + ) + return MergeImpactContributor( + id=contributor.pk, + institution_name=contributor.institution_name, + accession_count=counts["accession_count"], + published_image_count=counts["published_image_count"], + ) + + +def _merge_impact_user(user: User, *, has_engagement_profile: bool) -> MergeImpactUser: + return MergeImpactUser( + id=user.pk, + name=user.get_full_name() or user.email, + email=user.email, + has_engagement_profile=has_engagement_profile, + ) + + +def compute_contributor_merge_impact( + *, dest_contributor: Contributor, src_contributor: Contributor +) -> ContributorMergeImpact: + """ + Describe who gains access to what by merging src_contributor into dest_contributor. + + Access is granted exclusively by Contributor.owners, so merging exposes each contributor's + data to the other's owners. Note that the two contributors must be different. + """ + dest_owners = list(dest_contributor.owners.order_by("email")) + src_owners = list(src_contributor.owners.order_by("email")) + + engagement_user_ids = set( + User.objects.filter( + pk__in=[user.pk for user in dest_owners + src_owners], + engagement_profile__isnull=False, + ).values_list("pk", flat=True) + ) + + def users_gaining_access( + owners: list[User], existing_owners: list[User] + ) -> list[MergeImpactUser]: + existing_owner_ids = {user.pk for user in existing_owners} + return [ + _merge_impact_user(user, has_engagement_profile=user.pk in engagement_user_ids) + for user in owners + if user.pk not in existing_owner_ids + ] + + return ContributorMergeImpact( + dest_contributor=_merge_impact_contributor(dest_contributor), + src_contributor=_merge_impact_contributor(src_contributor), + users_gaining_access_to_dest=users_gaining_access(src_owners, dest_owners), + users_gaining_access_to_src=users_gaining_access(dest_owners, src_owners), + engagement_profiles_repointed=[ + _merge_impact_user(profile.user, has_engagement_profile=True) + for profile in src_contributor.engagement_profiles.select_related("user").order_by( + "user__email" + ) + ], + ) + + def merge_contributors(*, dest_contributor: Contributor, src_contributor: Contributor) -> None: """Merge a src_contributor into dest_contributor.""" with transaction.atomic(): diff --git a/isic/ingest/static/ingest/autocomplete.js b/isic/ingest/static/ingest/autocomplete.js index 5021f282..7d00db4e 100644 --- a/isic/ingest/static/ingest/autocomplete.js +++ b/isic/ingest/static/ingest/autocomplete.js @@ -1,7 +1,13 @@ // Alpine component backing the autocomplete form fields, e.g. the merge cohorts/contributors // pages. suggestUrl returns a list of matches for a query, detailUrl returns a single object // (by id) to preview, and labelKey names the field that's displayed for a match. -function autocompleteInput({ suggestUrl, detailUrl, labelKey = 'name', required = false }) { +function autocompleteInput({ + suggestUrl, + detailUrl, + labelKey = 'name', + required = false, + fieldName = '', +}) { return { selectedId: '', selectedDetail: null, @@ -9,6 +15,7 @@ function autocompleteInput({ suggestUrl, detailUrl, labelKey = 'name', required query: '', suggestions: [], loadingSuggestions: false, + rootEl: null, // the selection lives in a hidden input, which the browser bars from constraint validation, // so the visible search box carries the requirement instead. typing without picking a @@ -18,6 +25,9 @@ function autocompleteInput({ suggestUrl, detailUrl, labelKey = 'name', required }, async init() { + // notifySelection can run after the clicked suggestion has been removed from the DOM, and + // events dispatched from a detached element never bubble, so hold onto the root element. + this.rootEl = this.$el; this.$watch('selectedDetail', (selection) => this.$dispatch('autocomplete-selected', { selection }), ); @@ -27,17 +37,30 @@ function autocompleteInput({ suggestUrl, detailUrl, labelKey = 'name', required await this.populateDetail(); this.query = this.label(this.selectedDetail); } + this.notifySelection(); }, label(item) { return item ? item[labelKey] : ''; }, + // let an ancestor react to selections made across otherwise independent autocomplete fields, + // e.g. the contributor merge impact panel. pages without a listener are unaffected. + notifySelection() { + this.rootEl.dispatchEvent( + new CustomEvent('autocomplete-selection', { + detail: { name: fieldName, id: this.selectedId }, + bubbles: true, + }) + ); + }, + async select(item) { this.selectedId = item.id; this.query = this.label(item); this.suggestions = []; await this.populateDetail(); + this.notifySelection(); }, async populateDetail() { @@ -48,9 +71,14 @@ function autocompleteInput({ suggestUrl, detailUrl, labelKey = 'name', required }, async fetchSuggestions() { + // typing invalidates the selection, but only the first keystroke actually changes it + const hadSelection = Boolean(this.selectedId); this.selectedId = ''; this.selectedDetail = null; this.suggestions = []; + if (hadSelection) { + this.notifySelection(); + } if (this.query.length >= 3) { this.loadingSuggestions = true; const response = await fetch(`${suggestUrl}?query=${encodeURIComponent(this.query)}`); diff --git a/isic/ingest/static/ingest/contributor_merge_impact.js b/isic/ingest/static/ingest/contributor_merge_impact.js new file mode 100644 index 00000000..a23b3dee --- /dev/null +++ b/isic/ingest/static/ingest/contributor_merge_impact.js @@ -0,0 +1,63 @@ +// Alpine component for the merge contributors page. It listens for selections from the two +// autocomplete fields, which are otherwise independent components, and loads a summary of who +// gains access to what once both contributors are chosen. +function contributorMergeImpact({ impactUrl, destField, srcField }) { + return { + selections: {}, + impact: null, + loading: false, + requestCounter: 0, + + onAutocompleteSelection({ name, id }) { + if (name !== destField && name !== srcField) return; + this.selections[name] = id || ''; + this.refresh(); + }, + + async refresh() { + const dest = this.selections[destField] || ''; + const src = this.selections[srcField] || ''; + + // merging a contributor into itself is rejected by the form, so there's nothing to warn about + if (!dest || !src || dest === src) { + this.impact = null; + this.loading = false; + return; + } + + // Clear impact immediately and track this request + this.impact = null; + this.loading = true; + const thisRequest = ++this.requestCounter; + const params = new URLSearchParams({ dest_contributor: dest, src_contributor: src }); + try { + const response = await fetch(`${impactUrl}?${params}`); + // Only update impact if this is still the most recent request + if (thisRequest === this.requestCounter) { + this.impact = response.ok ? await response.json() : null; + } + } finally { + // Only clear loading if this is still the most recent request + if (thisRequest === this.requestCounter) { + this.loading = false; + } + } + }, + + get anyoneGainsAccess() { + return Boolean( + this.impact && + (this.impact.users_gaining_access_to_dest.length || + this.impact.users_gaining_access_to_src.length) + ); + }, + + number(count) { + return new Intl.NumberFormat('en-US').format(count ?? 0); + }, + + pluralize(count, singular, plural) { + return count === 1 ? singular : plural; + }, + }; +} diff --git a/isic/ingest/templates/ingest/contributor_merge.html b/isic/ingest/templates/ingest/contributor_merge.html index 1f61591f..7c54d8e8 100644 --- a/isic/ingest/templates/ingest/contributor_merge.html +++ b/isic/ingest/templates/ingest/contributor_merge.html @@ -4,10 +4,13 @@ {% block head_extra %} {{ block.super }} + {% endblock %} {% block content %} -
+ {% csrf_token %} {{ form.non_field_errors }} @@ -19,6 +22,8 @@ {% include 'ingest/partials/autocomplete_field.html' with field=form.contributor suggest_url=contributor_suggest_url detail_url=contributor_api_url label_key='institution_name' detail_template='ingest/partials/contributor_autocomplete_detail.html' %} {% include 'ingest/partials/autocomplete_field.html' with field=form.contributor_to_merge suggest_url=contributor_suggest_url detail_url=contributor_api_url label_key='institution_name' detail_template='ingest/partials/contributor_autocomplete_detail.html' %} + {% include 'ingest/partials/contributor_merge_impact.html' %} +
{% endblock %} diff --git a/isic/ingest/templates/ingest/partials/autocomplete_field.html b/isic/ingest/templates/ingest/partials/autocomplete_field.html index 40b0f9a9..547de9a0 100644 --- a/isic/ingest/templates/ingest/partials/autocomplete_field.html +++ b/isic/ingest/templates/ingest/partials/autocomplete_field.html @@ -1,4 +1,5 @@ -
+
+ +
+
+ +
+ + +
+ diff --git a/isic/ingest/templates/ingest/partials/merge_impact_users.html b/isic/ingest/templates/ingest/partials/merge_impact_users.html new file mode 100644 index 00000000..3d9c82c9 --- /dev/null +++ b/isic/ingest/templates/ingest/partials/merge_impact_users.html @@ -0,0 +1,11 @@ + diff --git a/isic/ingest/tests/test_api_contributor.py b/isic/ingest/tests/test_api_contributor.py index f4434269..f2bc93c7 100644 --- a/isic/ingest/tests/test_api_contributor.py +++ b/isic/ingest/tests/test_api_contributor.py @@ -1,5 +1,6 @@ from django.urls import reverse import pytest +from pytest_lazy_fixtures import lf @pytest.mark.django_db @@ -52,3 +53,51 @@ def test_api_contributor_autocomplete_only_returns_visible_contributors( assert resp.status_code == 200, resp.json() assert [c["id"] for c in resp.json()] == [owned.pk] + + +@pytest.mark.django_db +def test_api_contributor_merge_impact(contributor_factory, user_factory, staff_client): + src_owner = user_factory() + dest_contributor = contributor_factory() + src_contributor = contributor_factory(owners=[src_owner]) + + resp = staff_client.get( + reverse("api:contributor_merge_impact"), + data={"dest_contributor": dest_contributor.pk, "src_contributor": src_contributor.pk}, + ) + + assert resp.status_code == 200, resp.json() + assert resp.json()["dest_contributor"]["id"] == dest_contributor.pk + assert [u["email"] for u in resp.json()["users_gaining_access_to_dest"]] == [src_owner.email] + + +@pytest.mark.django_db +def test_api_contributor_merge_impact_same_contributor(contributor_factory, staff_client): + contributor = contributor_factory() + + resp = staff_client.get( + reverse("api:contributor_merge_impact"), + data={"dest_contributor": contributor.pk, "src_contributor": contributor.pk}, + ) + + assert resp.status_code == 400, resp.json() + + +@pytest.mark.django_db +@pytest.mark.parametrize( + ("client_", "status_code"), + [ + (lf("client"), 401), + (lf("authenticated_client"), 401), + (lf("staff_client"), 200), + ], +) +def test_api_contributor_merge_impact_permissions(client_, status_code, contributor_factory): + dest_contributor, src_contributor = contributor_factory(), contributor_factory() + + resp = client_.get( + reverse("api:contributor_merge_impact"), + data={"dest_contributor": dest_contributor.pk, "src_contributor": src_contributor.pk}, + ) + + assert resp.status_code == status_code diff --git a/isic/ingest/tests/test_merge.py b/isic/ingest/tests/test_merge.py index c681b7c3..ca0d5d32 100644 --- a/isic/ingest/tests/test_merge.py +++ b/isic/ingest/tests/test_merge.py @@ -11,7 +11,10 @@ from isic.ingest.models.cohort import Cohort from isic.ingest.models.contributor import Contributor from isic.ingest.services.cohort import merge_cohorts -from isic.ingest.services.contributor import merge_contributors +from isic.ingest.services.contributor import ( + compute_contributor_merge_impact, + merge_contributors, +) from isic.studies.tests.factories import StudyFactory @@ -87,6 +90,80 @@ def test_merge_contributors_reassigns_email_domains( assert email_domain.contributor == dest_contributor +@pytest.mark.django_db +def test_compute_contributor_merge_impact_both_directions(contributor_factory, user_factory): + dest_only, src_only, shared = user_factory(), user_factory(), user_factory() + dest_contributor = contributor_factory(owners=[dest_only, shared]) + src_contributor = contributor_factory(owners=[src_only, shared]) + + impact = compute_contributor_merge_impact( + dest_contributor=dest_contributor, src_contributor=src_contributor + ) + + # the shared owner already sees both sides, so they gain nothing + assert [user.id for user in impact.users_gaining_access_to_dest] == [src_only.pk] + assert [user.id for user in impact.users_gaining_access_to_src] == [dest_only.pk] + + +@pytest.mark.django_db +def test_compute_contributor_merge_impact_without_new_access(contributor_factory, user_factory): + owners = [user_factory(), user_factory()] + dest_contributor = contributor_factory(owners=owners) + src_contributor = contributor_factory(owners=owners) + + impact = compute_contributor_merge_impact( + dest_contributor=dest_contributor, src_contributor=src_contributor + ) + + assert impact.users_gaining_access_to_dest == [] + assert impact.users_gaining_access_to_src == [] + + +@pytest.mark.django_db +def test_compute_contributor_merge_impact_counts( + contributor_factory, cohort_factory, accession_factory, image_factory +): + dest_contributor, src_contributor = contributor_factory(), contributor_factory() + dest_cohort = cohort_factory(contributor=dest_contributor) + image_factory(accession=accession_factory(cohort=dest_cohort)) + # an unpublished accession counts towards accessions but not published images + accession_factory(cohort=dest_cohort) + accession_factory(cohort=cohort_factory(contributor=src_contributor)) + + impact = compute_contributor_merge_impact( + dest_contributor=dest_contributor, src_contributor=src_contributor + ) + + assert impact.dest_contributor.accession_count == 2 + assert impact.dest_contributor.published_image_count == 1 + assert impact.src_contributor.accession_count == 1 + assert impact.src_contributor.published_image_count == 0 + + +@pytest.mark.django_db +def test_compute_contributor_merge_impact_engagement_users( + contributor_factory, user_factory, engagement_profile_factory +): + engagement_owner, plain_owner = user_factory(), user_factory() + dest_contributor = contributor_factory(owners=[plain_owner]) + src_contributor = contributor_factory(owners=[engagement_owner]) + engagement_profile_factory(user=engagement_owner, default_contributor=src_contributor) + # an engagement user who isn't an owner gains no access, but their default is repointed + non_owner_profile = engagement_profile_factory(default_contributor=src_contributor) + + impact = compute_contributor_merge_impact( + dest_contributor=dest_contributor, src_contributor=src_contributor + ) + + assert [user.id for user in impact.users_gaining_access_to_dest] == [engagement_owner.pk] + assert impact.users_gaining_access_to_dest[0].has_engagement_profile + assert not impact.users_gaining_access_to_src[0].has_engagement_profile + assert {user.id for user in impact.engagement_profiles_repointed} == { + engagement_owner.pk, + non_owner_profile.user.pk, + } + + @pytest.mark.django_db def test_merge_contributors_view(contributor_with_cohort, staff_client): contributor_a, contributor_b = contributor_with_cohort(), contributor_with_cohort() diff --git a/isic/ingest/tests/test_merge_contributors_browser.py b/isic/ingest/tests/test_merge_contributors_browser.py index 67699d82..795d3728 100644 --- a/isic/ingest/tests/test_merge_contributors_browser.py +++ b/isic/ingest/tests/test_merge_contributors_browser.py @@ -74,6 +74,72 @@ def test_merge_contributors_autocomplete_preview_and_submit( assert contributor_b_cohorts <= set(contributor_a.cohorts.values_list("pk", flat=True)) +@pytest.mark.playwright +def test_merge_contributors_shows_access_impact( + staff_authenticated_page, + contributor_with_cohorts, + accession_factory, + image_factory, + user_factory, + engagement_profile_factory, +): + page = staff_authenticated_page + + dest_contributor = contributor_with_cohorts() + src_contributor = contributor_with_cohorts() + + # an owner of both contributors already sees everything, so they gain nothing from the merge + shared_owner = user_factory() + dest_contributor.owners.add(shared_owner) + src_contributor.owners.add(shared_owner) + dest_only_owner, src_only_owner = user_factory(), user_factory() + dest_contributor.owners.add(dest_only_owner) + src_contributor.owners.add(src_only_owner) + + dest_cohort = dest_contributor.cohorts.first() + image_factory(accession=accession_factory(cohort=dest_cohort)) + accession_factory(cohort=dest_cohort) + + engagement_profile_factory(user=src_only_owner, default_contributor=src_contributor) + engagement_profile_factory(default_contributor=src_contributor) + + page.goto(reverse("ingest/merge-contributors")) + + for fieldset_filter, contributor in [ + ({"has_text": "Contributor to merge into"}, dest_contributor), + ( + {"has_text": "Contributor to merge", "has_not_text": "Contributor to merge into"}, + src_contributor, + ), + ]: + fieldset = page.get_by_role("group").filter(**fieldset_filter) + fieldset.get_by_role("searchbox").press_sequentially( + contributor.institution_name[:5], delay=50 + ) + result = fieldset.get_by_text(contributor.institution_name, exact=True).first + expect(result).to_be_visible() + result.click() + + impact = page.get_by_role("alert") + expect(impact.get_by_text("Access impact of this merge")).to_be_visible() + + # both directions are described, and the owner of both contributors is left out of them + expect(impact.get_by_text(src_only_owner.email)).to_be_visible() + expect(impact.get_by_text(dest_only_owner.email)).to_be_visible() + expect(impact.get_by_text(shared_owner.email)).not_to_be_visible() + + expect(impact.get_by_text("engagement user")).to_be_visible() + expect(impact.get_by_text("will also have access to every future upload")).to_be_visible() + expect(impact.get_by_text("Their default will be repointed to")).to_be_visible() + + # clearing a contributor leaves nothing to describe + second_fieldset = page.get_by_role("group").filter( + has_text="Contributor to merge", has_not_text="Contributor to merge into" + ) + second_fieldset.get_by_role("searchbox").fill("") + expect(impact).not_to_be_visible() + + @pytest.mark.playwright def test_merge_contributors_same_contributor_rejected( staff_authenticated_page, contributor_with_cohorts @@ -100,3 +166,107 @@ def test_merge_contributors_same_contributor_rejected( expect(page.get_by_text("The two contributors must be different.")).to_be_visible() assert Contributor.objects.filter(pk=contributor.pk).exists() + + +@pytest.mark.playwright +def test_merge_contributors_stale_impact_not_shown_after_selection_change( + staff_authenticated_page, + contributor_with_cohorts, + user_factory, +): + """Test that changing selections before impact loads doesn't show stale results.""" + page = staff_authenticated_page + + # Create three contributors with distinct owners + contributor_a = contributor_with_cohorts() + contributor_b = contributor_with_cohorts() + contributor_c = contributor_with_cohorts() + + owner_a = user_factory() + owner_b = user_factory() + owner_c = user_factory() + + contributor_a.owners.add(owner_a) + contributor_b.owners.add(owner_b) + contributor_c.owners.add(owner_c) + + page.goto(reverse("ingest/merge-contributors")) + + # Select contributor_a as destination + first_fieldset = page.get_by_role("group").filter(has_text="Contributor to merge into") + first_input = first_fieldset.get_by_role("searchbox") + first_input.press_sequentially(contributor_a.institution_name[:5], delay=50) + first_result = first_fieldset.get_by_text(contributor_a.institution_name, exact=True).first + expect(first_result).to_be_visible() + first_result.click() + + # Select contributor_b as source (this will trigger an impact fetch) + second_fieldset = page.get_by_role("group").filter( + has_text="Contributor to merge", has_not_text="Contributor to merge into" + ) + second_input = second_fieldset.get_by_role("searchbox") + second_input.press_sequentially(contributor_b.institution_name[:5], delay=50) + second_result = second_fieldset.get_by_text(contributor_b.institution_name, exact=True).first + expect(second_result).to_be_visible() + second_result.click() + + # Immediately change to contributor_c before the first fetch completes + # This should supersede the first request + second_input.fill("") + second_input.press_sequentially(contributor_c.institution_name[:5], delay=50) + third_result = second_fieldset.get_by_text(contributor_c.institution_name, exact=True).first + expect(third_result).to_be_visible() + third_result.click() + + # Wait for the impact alert to appear + impact = page.get_by_role("alert") + expect(impact.get_by_text("Access impact of this merge")).to_be_visible() + + # The impact should only show owner_c (from contributor_c), not owner_b + expect(impact.get_by_text(owner_c.email)).to_be_visible() + expect(impact.get_by_text(owner_b.email)).not_to_be_visible() + expect(impact.get_by_text(owner_a.email)).not_to_be_visible() + + +@pytest.mark.playwright +def test_merge_contributors_stale_impact_not_shown_after_selection_cleared( + staff_authenticated_page, + contributor_with_cohorts, + user_factory, +): + """Test that clearing selection before impact loads doesn't show stale results.""" + page = staff_authenticated_page + + contributor_a = contributor_with_cohorts() + contributor_b = contributor_with_cohorts() + + owner_b = user_factory() + contributor_b.owners.add(owner_b) + + page.goto(reverse("ingest/merge-contributors")) + + # Select contributor_a as destination + first_fieldset = page.get_by_role("group").filter(has_text="Contributor to merge into") + first_input = first_fieldset.get_by_role("searchbox") + first_input.press_sequentially(contributor_a.institution_name[:5], delay=50) + first_result = first_fieldset.get_by_text(contributor_a.institution_name, exact=True).first + expect(first_result).to_be_visible() + first_result.click() + + # Select contributor_b as source (this will trigger an impact fetch) + second_fieldset = page.get_by_role("group").filter( + has_text="Contributor to merge", has_not_text="Contributor to merge into" + ) + second_input = second_fieldset.get_by_role("searchbox") + second_input.press_sequentially(contributor_b.institution_name[:5], delay=50) + second_result = second_fieldset.get_by_text(contributor_b.institution_name, exact=True).first + expect(second_result).to_be_visible() + second_result.click() + + # Immediately clear the selection before the first fetch completes + second_input.fill("") + + # The impact alert should not be visible (it was already tested to disappear + # when cleared in test_merge_contributors_shows_access_impact) + impact = page.get_by_role("alert") + expect(impact).not_to_be_visible()