Skip to content
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
18 changes: 8 additions & 10 deletions e2e/organizations.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,19 +20,17 @@ test.describe("Organization Creation", () => {
await expect(page).toHaveURL(/\/organizations\/e2e-new-org\//);
});

test("similar name shows matching orgs, force-create works", async ({ page }) => {
test("typing a similar name shows live fuzzy matches", async ({ page }) => {
await page.goto("/organizations/~create");
await page.locator("input[name='name']").fill("e2e public org");
await page.locator("button[type='submit']").click();

// Should show matching organizations and a force-create form
await expect(page).toHaveURL(/\/organizations\/~create/);
await expect(page.locator("input[name='force'][value='true']")).toBeAttached();
// Type a name that matches an existing org
await page.locator("input[name='name']").fill("e2e public org");

// The force form reuses the #login_form id with the hidden force field
await page.locator("#login_form button[type='submit']").click();
await page.waitForURL((url) => !url.pathname.includes("~create"));
await expect(page).toHaveURL(/\/organizations\//);
// The matching org should appear in the results list
const results = page.locator("#org_name_search");
await expect(
results.locator("a[href='/organizations/e2e-public-org/']"),
).toBeVisible({ timeout: 5_000 });
});
});

Expand Down
108 changes: 108 additions & 0 deletions frontend/components/OrgNameSearch.svelte
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
<script lang="ts">
import type { Organization } from "@/types";

import TeamListItem from "./TeamListItem.svelte";

let results: Organization[] = $state([]);
let searched = $state(false);
let timeout: ReturnType<typeof setTimeout> | undefined;

function onInput(event: Event) {
const query = (event.target as HTMLInputElement).value.trim();
clearTimeout(timeout);

if (query.length < 2) {
results = [];
searched = false;
return;
}

timeout = setTimeout(async () => {
const url = `/fe_api/organizations/?individual=false&search=${encodeURIComponent(query)}&fuzzy=true`;
try {
const resp = await fetch(url, { credentials: "include" });
const data = await resp.json();
results = data.results ?? [];
} catch {
results = [];
}
searched = true;
}, 500);
}

// Find the name input and attach the listener
$effect(() => {
const input = document.querySelector<HTMLInputElement>('input[name="name"]');
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This feels brittle. If we change the input name, it's going to break, and it's hard to follow the code.

I realize this is separate from the form, so it's tricky to tie them together.

We could take an input element as a prop, and that would at least guarantee we have an input.

We do have a test that looks for input[name='name'], so that's something.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is very valid. I do have confidence the input name won't change since it's tied to the model: this maps to the name field of Organization, which will probably never change? I think the test is a good guarantee of the stability here—if we're renaming this fundamental model field, this (and many other) tests will fail.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think that's reasonable. As long as we feel confident we'll catch anything that breaks, we should be ok.

if (input) {
input.addEventListener("input", onInput);
return () => input.removeEventListener("input", onInput);
}
});
</script>

{#if results.length > 0}
<div class="card">
<p class="heading">It looks like {results.length > 1 ? 'organizations with similar names are' : 'an organization with a similar name is'} already up and running on MuckRock:</p>
<ul class="results">
{#each results as org (org.id)}
<li>
<a href="/organizations/{org.slug}/" target="_blank" rel="noopener">
<TeamListItem organization={org} />
</a>
</li>
{/each}
</ul>
<p>
If the organization you're trying to create is already set up on MuckRock, please look for the <b>Request to Join</b> option on the organization's profile page. Please <a href="mailto:info@muckrock.com">contact support</a> if the admins listed have left the organization.
</p>
</div>
{:else if searched}
<p class="hint">No similarly named organizations found.</p>
{:else}
<p class="hint">To discourage duplicate organizations, any existing organizations with similar names will appear here.</p>
{/if}

<style>
.card {
padding: 1rem;
color: var(--gray-5);
}

.hint {
width: 100%;
color: var(--gray-4);
border: 1px solid var(--gray-2);
border-radius: 8px;
padding: 1rem;
box-sizing: border-box;
}

p {
margin: 0;
}

.heading {
font-weight: 600;
}

.results {
list-style: none;
padding: 0;
margin: 1rem 0;
}

.results li {
margin-bottom: 0.5rem;
}

.results a {
text-decoration: none;
color: inherit;
display: block;
}

.results a:hover {
background: var(--color-surface-hover, #f5f5f5);
border-radius: 4px;
}
</style>
27 changes: 27 additions & 0 deletions frontend/css/organization_create.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
.create-organization {
max-width: 32rem;
min-height: 50vh;
display: flex;
flex-direction: column;
gap: 2rem;
padding: 4rem 2rem;
}

.create-organization h1, h2 {
margin: 0;
}

.unverified {
gap: 1rem;
}

.unverified p {
color: var(--gray-4);
}

form {
width: 100%;
display: flex;
flex-direction: column;
gap: 1rem;
}
12 changes: 12 additions & 0 deletions frontend/views/organization_create.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import "@/css/team_list_item.css";
import "@/css/organization_create.css";

import { mount } from "svelte";
import OrgNameSearch from "../components/OrgNameSearch.svelte";

window.addEventListener("DOMContentLoaded", () => {
const el = document.getElementById("org_name_search");
if (el) {
mount(OrgNameSearch, { target: el });
}
});
13 changes: 13 additions & 0 deletions squarelet/organizations/fe_api/viewsets.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,19 @@ def get_queryset(self):
search_fields = ["name"]
ordering_fields = ["name"]

def list(self, request, *args, **kwargs):
fuzzy = request.query_params.get("fuzzy", "").lower() == "true"
search = request.query_params.get("search", "").strip()
if fuzzy and search:
# Apply only DjangoFilterBackend (for individual, etc.) but skip
# SearchFilter so fuzzy_search handles the name matching
queryset = self.get_queryset()
queryset = DjangoFilterBackend().filter_queryset(request, queryset, self)
queryset = queryset.fuzzy_search(search)
serializer = self.get_serializer(queryset, many=True)
return Response({"results": serializer.data})
return super().list(request, *args, **kwargs)


class InvitationViewSet(
mixins.ListModelMixin,
Expand Down
35 changes: 35 additions & 0 deletions squarelet/organizations/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,41 @@
from .models import Organization, Plan, ProfileChangeRequest


class CreateForm(forms.ModelForm):
"""Form for creating a new organization"""

avatar = forms.ImageField(
label=_("Avatar"),
required=False,
widget=AvatarWidget,
help_text=(
"This will represent the organization on its profile, "
"on public pages, and in lists."
),
)
about = forms.CharField(
label=_("About"),
widget=forms.Textarea,
required=False,
help_text=_("Markdown formatting supported. Maximum 250 characters."),
)

class Meta:
model = Organization
fields = ["name", "about", "avatar"]

def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
self.helper = FormHelper()
self.helper.template_pack = "forms"
self.helper.layout = Layout(
CrispyField("name"),
CrispyField("avatar"),
CrispyField("about"),
)
self.helper.form_tag = False


class PaymentForm(StripeForm):
"""Update subscription information for an organization"""

Expand Down
20 changes: 20 additions & 0 deletions squarelet/organizations/querysets.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
# Third Party
import stripe
from dateutil.relativedelta import relativedelta
from fuzzywuzzy import fuzz, process

# Squarelet
from squarelet.organizations.choices import ChangeLogReason
Expand Down Expand Up @@ -56,6 +57,25 @@ def get_viewable(self, user):
| Q(private=False, invoices__status="paid")
).distinct()

def fuzzy_search(self, name, limit=10, score_cutoff=83):
"""Fuzzy search for non-individual organizations by name"""
group_orgs = self.filter(individual=False)
matching_orgs = process.extractBests(
name,
{o: o.name for o in group_orgs},
limit=limit,
scorer=fuzz.partial_ratio,
score_cutoff=score_cutoff,
)
matched_pks = [org.pk for _, _, org in matching_orgs]
if not matched_pks:
return self.none()
# Preserve match order using CASE/WHEN
preserved = models.Case(
*[models.When(pk=pk, then=pos) for pos, pk in enumerate(matched_pks)]
)
return self.filter(pk__in=matched_pks).order_by(preserved)

def create_individual(self, user, uuid=None):
"""Create an individual organization for user
The user model must be unsaved
Expand Down
23 changes: 22 additions & 1 deletion squarelet/organizations/tests/test_forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import pytest

# Squarelet
from squarelet.organizations.forms import ProfileChangeRequestForm
from squarelet.organizations.forms import CreateForm, ProfileChangeRequestForm
from squarelet.organizations.models import ProfileChangeRequest


Expand Down Expand Up @@ -227,3 +227,24 @@ def test_url_must_be_unique_for_organization(
assert not form.is_valid()
assert "url" in form.errors
assert "already associated with the organization" in str(form.errors["url"])


@pytest.mark.django_db()
class TestCreateForm:
"""Test CreateForm"""

def test_valid_with_name(self):
"""CreateForm with valid name creates successfully"""
form = CreateForm(data={"name": "New Organization"})
assert form.is_valid(), form.errors

def test_valid_with_all_fields(self):
"""CreateForm accepts optional about field"""
form = CreateForm(data={"name": "New Organization", "about": "A great org"})
assert form.is_valid(), form.errors

def test_invalid_without_name(self):
"""CreateForm without name fails validation"""
form = CreateForm(data={"about": "A great org"})
assert not form.is_valid()
assert "name" in form.errors
28 changes: 28 additions & 0 deletions squarelet/organizations/tests/test_querysets.py
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,34 @@ def test_create_individual_returns_organization(self):
assert org == user.individual_organization


class TestFuzzySearch(TestCase):
"""Unit tests for Organization.objects.fuzzy_search()"""

@pytest.mark.django_db
def test_fuzzy_search_finds_similar_name(self):
"""fuzzy_search returns orgs with similar names"""
OrganizationFactory(name="MuckRock Foundation", individual=False)
results = Organization.objects.fuzzy_search("Muckrock")
assert results.count() == 1
assert results.first().name == "MuckRock Foundation"

@pytest.mark.django_db
def test_fuzzy_search_no_match(self):
"""fuzzy_search returns empty queryset when no orgs match"""
OrganizationFactory(name="MuckRock Foundation", individual=False)
results = Organization.objects.fuzzy_search("xyzzyspoon")
assert results.count() == 0

@pytest.mark.django_db
def test_fuzzy_search_excludes_individual_orgs(self):
"""fuzzy_search excludes individual organizations"""
OrganizationFactory(name="MuckRock User", individual=True, private=True)
OrganizationFactory(name="MuckRock Foundation", individual=False)
results = Organization.objects.fuzzy_search("MuckRock")
assert results.count() == 1
assert results.first().name == "MuckRock Foundation"


class TestMembershipQuerySet(TestCase):
"""Unit tests for Membership queryset"""

Expand Down
28 changes: 26 additions & 2 deletions squarelet/organizations/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -988,14 +988,38 @@ class TestCreate(ViewTestMixin):
view = views.Create
url = "/organizations/~create/"

def test(self, rf, user_factory):
user = user_factory()
def test_post_creates_org(self, rf, user_factory):
user = user_factory(email_verified=True)
self.call_view(rf, user, {"name": "test"})
organization = user.organizations.get(individual=False)
assert organization.plan is None
assert organization.has_admin(user)
assert user.email in organization.receipt_emails.values_list("email", flat=True)

def test_get_unverified_email(self, rf, user_factory):
"""User without verified email sees verification message"""
user = user_factory(email_verified=False)
response = self.call_view(rf, user)
assert response.status_code == 200
assert response.context_data["has_verified_email"] is False

def test_get_verified_email(self, rf, user_factory):
"""User with verified email sees the creation form"""
user = user_factory(email_verified=True)
response = self.call_view(rf, user)
assert response.status_code == 200
assert response.context_data["has_verified_email"] is True
assert "form" in response.context_data

def test_post_creates_org_despite_matching_name(
self, rf, user_factory, organization_factory
):
"""POST creates org even when an org with a similar name exists"""
user = user_factory(email_verified=True)
organization_factory(name="Test Organization", individual=False)
self.call_view(rf, user, {"name": "Test Organization"})
assert user.organizations.filter(individual=False).exists()


@pytest.mark.django_db()
class TestManageMembers(ViewTestMixin): # pylint: disable=too-many-public-methods
Expand Down
3 changes: 2 additions & 1 deletion squarelet/organizations/views/__init__.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# Import all views to maintain backward compatibility with existing imports
# Local
from .admin import Create, Merge
from .admin import Merge
from .create import Create
from .detail import Detail, List, autocomplete
from .domains import ManageDomains
from .members import (
Expand Down
Loading
Loading