tea/browsertrix
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Improve handling of duplicate org name/slug (#1917)

Browse Source 
Initial implementation of #1892 

- Modifies the backend to return `duplicate_org_name` or
`duplicate_org_slug` as appropriate on a pymongo `DuplicateKeyError`
- Updates frontend to handle `duplicate_org_name`, `duplicate_org_slug`,
and `invalid_slug` error details
- Update errors to be more consistent, also return `duplicate_org_subscription.subId` for duplicate subscription instead of the more generic `already_exists`
---------
Co-authored-by: Ilya Kreymer <ikreymer@gmail.com>
This commit is contained in:
Tessa Walsh 2024-07-10 22:24:50 -04:00 committed by GitHub
parent 9a67e28f13
commit a546fb6fe0
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 127 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
backend/btrixcloud/orgs.py
View File

@ -67,7 +67,12 @@ from .models import (
ACTIVE,
)
from .pagination import DEFAULT_PAGE_SIZE, paginated_format
from .utils import slug_from_name, validate_slug, JSONSerializer
from .utils import (
slug_from_name,
validate_slug,
get_duplicate_key_error_field,
JSONSerializer,
)
if TYPE_CHECKING:
from .invites import InviteOps
@ -301,8 +306,15 @@ class OrgOps:
)
try:
await self.orgs.insert_one(org.to_dict())
except DuplicateKeyError:
print(f"Organization name {org.name} already in use - skipping", flush=True)
except DuplicateKeyError as err:
field = get_duplicate_key_error_field(err)
value = org.name
if field == "slug":
value = org.slug
print(
f"Organization {field} {value} already in use - skipping",
flush=True,
)
async def create_org(
self,
@ -337,7 +349,10 @@ class OrgOps:
try:
await self.orgs.insert_one(org.to_dict())
except DuplicateKeyError as dupe:
raise HTTPException(status_code=400, detail="already_exists") from dupe
field = get_duplicate_key_error_field(dupe)
raise HTTPException(
status_code=400, detail=f"duplicate_org_{field}"
) from dupe
return org
@ -1390,9 +1405,11 @@ def init_orgs_api(
try:
await ops.update_slug_and_name(org)
except DuplicateKeyError:
# pylint: disable=raise-missing-from
raise HTTPException(status_code=400, detail="duplicate_org_name")
except DuplicateKeyError as dupe:
field = get_duplicate_key_error_field(dupe)
raise HTTPException(
status_code=400, detail=f"duplicate_org_{field}"
) from dupe
return {"updated": True}

14
backend/btrixcloud/utils.py
View File

@ -16,6 +16,7 @@ from uuid import UUID
from fastapi import HTTPException
from fastapi.responses import StreamingResponse
from pymongo.errors import DuplicateKeyError
from slugify import slugify
@ -166,3 +167,16 @@ def stream_dict_list_as_csv(data: List[Dict[str, Union[str, int]]], filename: st
media_type="text/csv",
headers={"Content-Disposition": f"attachment;filename={filename}"},
)
def get_duplicate_key_error_field(err: DuplicateKeyError) -> str:
"""Get name of duplicate field from pymongo DuplicateKeyError"""
dupe_field = "name"
if err.details:
key_value = err.details.get("keyValue")
if key_value:
try:
dupe_field = list(key_value.keys())[0]
except IndexError:
pass
return dupe_field

3
backend/test/conftest.py
View File

@ -28,6 +28,7 @@ _all_crawls_config_id = None
_all_crawls_delete_config_id = None
NON_DEFAULT_ORG_NAME = "Non-default org"
NON_DEFAULT_ORG_SLUG = "non-default-org"
FAILED_STATES = ["canceled", "failed", "skipped_quota_reached"]
@ -77,7 +78,7 @@ def non_default_org_id(admin_auth_headers):
r = requests.post(
f"{API_PREFIX}/orgs/create",
headers=admin_auth_headers,
json={"name": NON_DEFAULT_ORG_NAME, "slug": "non-default-org"},
json={"name": NON_DEFAULT_ORG_NAME, "slug": NON_DEFAULT_ORG_SLUG},
)
assert r.status_code == 200

54
backend/test/test_org.py
View File

@ -4,7 +4,7 @@ import uuid
import pytest
from .conftest import API_PREFIX
from .conftest import API_PREFIX, NON_DEFAULT_ORG_NAME, NON_DEFAULT_ORG_SLUG
from .utils import read_in_chunks
curr_dir = os.path.dirname(os.path.realpath(__file__))
@ -92,6 +92,34 @@ def test_rename_org_invalid_slug(admin_auth_headers, default_org_id):
assert r.json()["detail"] == "invalid_slug"
def test_rename_org_duplicate_name(
admin_auth_headers, default_org_id, non_default_org_id
):
rename_data = {"name": NON_DEFAULT_ORG_NAME, "slug": "this-slug-should-be-okay"}
r = requests.post(
f"{API_PREFIX}/orgs/{default_org_id}/rename",
headers=admin_auth_headers,
json=rename_data,
)
assert r.status_code == 400
assert r.json()["detail"] == "duplicate_org_name"
def test_rename_org_duplicate_name(
admin_auth_headers, default_org_id, non_default_org_id
):
rename_data = {"name": "Should be okay", "slug": NON_DEFAULT_ORG_SLUG}
r = requests.post(
f"{API_PREFIX}/orgs/{default_org_id}/rename",
headers=admin_auth_headers,
json=rename_data,
)
assert r.status_code == 400
assert r.json()["detail"] == "duplicate_org_slug"
def test_create_org(admin_auth_headers):
NEW_ORG_NAME = "New Org"
r = requests.post(
@ -118,6 +146,30 @@ def test_create_org(admin_auth_headers):
assert NEW_ORG_NAME in org_names
def test_create_org_duplicate_name(admin_auth_headers, non_default_org_id):
r = requests.post(
f"{API_PREFIX}/orgs/create",
headers=admin_auth_headers,
json={"name": NON_DEFAULT_ORG_NAME, "slug": "another-new-org"},
)
assert r.status_code == 400
data = r.json()
assert data["detail"] == "duplicate_org_name"
def test_create_org_duplicate_slug(admin_auth_headers, non_default_org_id):
r = requests.post(
f"{API_PREFIX}/orgs/create",
headers=admin_auth_headers,
json={"name": "Yet another new org", "slug": NON_DEFAULT_ORG_SLUG},
)
assert r.status_code == 400
data = r.json()
assert data["detail"] == "duplicate_org_slug"
# disable until storage customization is enabled
def _test_change_org_storage(admin_auth_headers):
# change to invalid storage

2
backend/test/test_org_subs.py
View File

@ -144,7 +144,7 @@ def test_create_sub_org_and_invite_existing_user_dupe_sub(admin_auth_headers):
)
assert r.status_code == 400
assert r.json()["detail"] == "already_exists"
assert r.json()["detail"] == "duplicate_org_subscription.subId"
def test_create_sub_org_and_invite_existing_user(admin_auth_headers):

20
frontend/src/pages/invite/ui/org-form.ts
View File

@ -147,10 +147,22 @@ export class OrgForm extends TailwindElement {
await this.onRenameSuccess(payload);
} catch (e) {
console.debug(e);
if (isApiError(e) && e.details === "duplicate_org_name") {
throw new Error(
msg("This org name or URL is already taken, try another one."),
);
if (isApiError(e)) {
if (e.details === "duplicate_org_name") {
throw new Error(
msg("This org name is already taken, try another one."),
);
} else if (e.details === "duplicate_org_slug") {
throw new Error(
msg("This org URL is already taken, try another one."),
);
} else if (e.details === "invalid_slug") {
throw new Error(
msg(
"This org URL is invalid. Please use alphanumeric characters and dashes (-) only.",
),
);
}
}
this._notify.toast({

23
frontend/src/pages/org/settings/settings.ts
View File

@ -626,13 +626,24 @@ export class OrgSettings extends TailwindElement {
} catch (e) {
console.debug(e);
let message = msg(
"Sorry, couldn't rename organization at this time. Try again later from org settings.",
);
if (isApiError(e)) {
if (e.details === "duplicate_org_name") {
message = msg("This org name is already taken, try another one.");
} else if (e.details === "duplicate_org_slug") {
message = msg("This org URL is already taken, try another one.");
} else if (e.details === "invalid_slug") {
message = msg(
"This org URL is invalid. Please use alphanumeric characters and dashes (-) only.",
);
}
}
this.notify.toast({
message:
isApiError(e) && e.details === "duplicate_org_name"
? msg("This org name or URL is already taken, try another one.")
: msg(
"Sorry, couldn't rename organization at this time. Try again later from org settings.",
),
message: message,
variant: "danger",
icon: "exclamation-octagon",
});