From caefb2a2373ed5fb36041378bdb28ac45eab5033 Mon Sep 17 00:00:00 2001 From: Tessa Walsh Date: Mon, 13 Jan 2025 14:47:06 -0500 Subject: [PATCH 01/18] Add slugs to collections and backfill via migration --- backend/btrixcloud/colls.py | 45 ++++++++++++++++--- .../migrations/migration_0038_coll_slugs.py | 38 ++++++++++++++++ backend/btrixcloud/models.py | 7 +++ backend/test/test_collections.py | 27 ++++++++++- 4 files changed, 109 insertions(+), 8 deletions(-) create mode 100644 backend/btrixcloud/migrations/migration_0038_coll_slugs.py diff --git a/backend/btrixcloud/colls.py b/backend/btrixcloud/colls.py index f6c13dad59..8cdd6d8070 100644 --- a/backend/btrixcloud/colls.py +++ b/backend/btrixcloud/colls.py @@ -13,6 +13,7 @@ import asyncio import pymongo +from pymongo.collation import Collation from fastapi import Depends, HTTPException, Response from fastapi.responses import StreamingResponse from starlette.requests import Request @@ -51,7 +52,7 @@ MIN_UPLOAD_PART_SIZE, PublicCollOut, ) -from .utils import dt_now +from .utils import dt_now, slug_from_name, get_duplicate_key_error_field if TYPE_CHECKING: from .orgs import OrgOps @@ -98,8 +99,17 @@ def set_page_ops(self, ops): async def init_index(self): """init lookup index""" + case_insensitive_collation = Collation(locale="en", strength=1) await self.collections.create_index( - [("oid", pymongo.ASCENDING), ("name", pymongo.ASCENDING)], unique=True + [("oid", pymongo.ASCENDING), ("name", pymongo.ASCENDING)], + unique=True, + collation=case_insensitive_collation, + ) + + await self.collections.create_index( + [("oid", pymongo.ASCENDING), ("slug", pymongo.ASCENDING)], + unique=True, + collation=case_insensitive_collation, ) await self.collections.create_index( @@ -117,6 +127,7 @@ async def add_collection(self, oid: UUID, coll_in: CollIn): id=coll_id, oid=oid, name=coll_in.name, + slug=coll_in.slug or slug_from_name(coll_in.name), description=coll_in.description, caption=coll_in.caption, created=created, @@ -139,8 +150,11 @@ async def add_collection(self, oid: UUID, coll_in: CollIn): ) return {"added": True, "id": coll_id, "name": coll.name} - except pymongo.errors.DuplicateKeyError: + except pymongo.errors.DuplicateKeyError as err: # pylint: disable=raise-missing-from + field = get_duplicate_key_error_field(err) + if field == "slug": + raise HTTPException(status_code=400, detail="collection_slug_taken") raise HTTPException(status_code=400, detail="collection_name_taken") async def update_collection( @@ -152,17 +166,36 @@ async def update_collection( if len(query) == 0: raise HTTPException(status_code=400, detail="no_update_data") + name_update = query.get("name") + slug_update = query.get("slug") + + previous_slug = None + + if name_update or slug_update: + # If we're updating slug, save old one to previousSlugs to support redirects + coll = await self.get_collection(coll_id) + previous_slug = coll.slug + + if name_update and not slug_update: + slug = slug_from_name(name_update) + query["slug"] = slug + query["modified"] = dt_now() + db_update = {"$set": query} + if previous_slug: + db_update["$push"] = {"previousSlugs": previous_slug} + try: result = await self.collections.find_one_and_update( {"_id": coll_id, "oid": org.id}, - {"$set": query}, + db_update, return_document=pymongo.ReturnDocument.AFTER, ) - except pymongo.errors.DuplicateKeyError: + except pymongo.errors.DuplicateKeyError as err: # pylint: disable=raise-missing-from - raise HTTPException(status_code=400, detail="collection_name_taken") + field = get_duplicate_key_error_field(err) + raise HTTPException(status_code=400, detail=f"collection_{field}_taken") if not result: raise HTTPException(status_code=404, detail="collection_not_found") diff --git a/backend/btrixcloud/migrations/migration_0038_coll_slugs.py b/backend/btrixcloud/migrations/migration_0038_coll_slugs.py new file mode 100644 index 0000000000..362f0afebe --- /dev/null +++ b/backend/btrixcloud/migrations/migration_0038_coll_slugs.py @@ -0,0 +1,38 @@ +""" +Migration 0038 -- collection access +""" + +from btrixcloud.migrations import BaseMigration +from btrixcloud.utils import slug_from_name + + +MIGRATION_VERSION = "0038" + + +class Migration(BaseMigration): + """Migration class.""" + + # pylint: disable=unused-argument + def __init__(self, mdb, **kwargs): + super().__init__(mdb, migration_version=MIGRATION_VERSION) + + async def migrate_up(self): + """Perform migration up. + + Add slug to collections that don't have one yet, based on name + """ + colls_mdb = self.mdb["collections"] + + async for coll_raw in colls_mdb.find({"slug": None}): + coll_id = coll_raw["_id"] + try: + await colls_mdb.find_one_and_update( + {"_id": coll_id}, + {"$set": {"slug": slug_from_name(coll_raw.get("name", ""))}}, + ) + # pylint: disable=broad-exception-caught + except Exception as err: + print( + f"Error saving slug for collection {coll_id}: {err}", + flush=True, + ) diff --git a/backend/btrixcloud/models.py b/backend/btrixcloud/models.py index 512e3cd42a..1cf0d0e99d 100644 --- a/backend/btrixcloud/models.py +++ b/backend/btrixcloud/models.py @@ -1236,6 +1236,7 @@ class Collection(BaseMongoModel): """Org collection structure""" name: str = Field(..., min_length=1) + slug: str = Field(..., min_length=1) oid: UUID description: Optional[str] = None caption: Optional[str] = None @@ -1264,12 +1265,15 @@ class Collection(BaseMongoModel): allowPublicDownload: Optional[bool] = True + previousSlugs: List[str] = [] + # ============================================================================ class CollIn(BaseModel): """Collection Passed in By User""" name: str = Field(..., min_length=1) + slug: Optional[str] = None description: Optional[str] = None caption: Optional[str] = None crawlIds: Optional[List[str]] = [] @@ -1285,6 +1289,7 @@ class CollOut(BaseMongoModel): """Collection output model with annotations.""" name: str + slug: str oid: UUID description: Optional[str] = None caption: Optional[str] = None @@ -1319,6 +1324,7 @@ class PublicCollOut(BaseMongoModel): """Collection output model with annotations.""" name: str + slug: str oid: UUID description: Optional[str] = None caption: Optional[str] = None @@ -1349,6 +1355,7 @@ class UpdateColl(BaseModel): """Update collection""" name: Optional[str] = None + slug: Optional[str] = None description: Optional[str] = None caption: Optional[str] = None access: Optional[CollAccessType] = None diff --git a/backend/test/test_collections.py b/backend/test/test_collections.py index 7b274bf8fe..c70c779c2f 100644 --- a/backend/test/test_collections.py +++ b/backend/test/test_collections.py @@ -9,8 +9,11 @@ from .utils import read_in_chunks COLLECTION_NAME = "Test collection" +COLLECTION_SLUG = "test-collection" PUBLIC_COLLECTION_NAME = "Public Test collection" +PUBLIC_COLLECTION_SLUG = "custom-public-collection-slug" UPDATED_NAME = "Updated tést cöllection" +UPDATED_SLUG = "updated-test-collection" SECOND_COLLECTION_NAME = "second-collection" DESCRIPTION = "Test description" CAPTION = "Short caption" @@ -74,6 +77,7 @@ def test_create_collection( assert data["id"] == _coll_id assert data["name"] == COLLECTION_NAME + assert data["slug"] == COLLECTION_SLUG assert data["caption"] == CAPTION assert data["crawlCount"] == 1 assert data["pageCount"] > 0 @@ -98,6 +102,7 @@ def test_create_public_collection( json={ "crawlIds": [crawler_crawl_id], "name": PUBLIC_COLLECTION_NAME, + "slug": PUBLIC_COLLECTION_SLUG, "caption": CAPTION, "access": "public", "allowPublicDownload": False, @@ -131,7 +136,7 @@ def test_create_collection_taken_name( }, ) assert r.status_code == 400 - assert r.json()["detail"] == "collection_name_taken" + assert r.json()["detail"] in ("collection_name_taken", "collection_slug_taken") def test_create_collection_empty_name( @@ -207,6 +212,7 @@ def test_rename_collection( assert data["id"] == _coll_id assert data["name"] == UPDATED_NAME + assert data["slug"] == UPDATED_SLUG assert data["modified"] >= modified @@ -237,7 +243,16 @@ def test_rename_collection_taken_name( json={"name": SECOND_COLLECTION_NAME}, ) assert r.status_code == 400 - assert r.json()["detail"] == "collection_name_taken" + assert r.json()["detail"] in ("collection_name_taken", "collection_slug_taken") + + # Try to set first coll's slug to value already used for second collection + r = requests.patch( + f"{API_PREFIX}/orgs/{default_org_id}/collections/{_coll_id}", + headers=crawler_auth_headers, + json={"slug": SECOND_COLLECTION_NAME}, + ) + assert r.status_code == 400 + assert r.json()["detail"] == "collection_slug_taken" def test_add_remove_crawl_from_collection( @@ -324,6 +339,7 @@ def test_get_collection(crawler_auth_headers, default_org_id): data = r.json() assert data["id"] == _coll_id assert data["name"] == UPDATED_NAME + assert data["slug"] == UPDATED_SLUG assert data["oid"] == default_org_id assert data["description"] == DESCRIPTION assert data["caption"] == UPDATED_CAPTION @@ -346,6 +362,7 @@ def test_get_collection_replay(crawler_auth_headers, default_org_id): data = r.json() assert data["id"] == _coll_id assert data["name"] == UPDATED_NAME + assert data["slug"] == UPDATED_SLUG assert data["oid"] == default_org_id assert data["description"] == DESCRIPTION assert data["caption"] == UPDATED_CAPTION @@ -524,6 +541,7 @@ def test_list_collections( first_coll = [coll for coll in items if coll["name"] == UPDATED_NAME][0] assert first_coll["id"] == _coll_id assert first_coll["name"] == UPDATED_NAME + assert first_coll["slug"] == UPDATED_SLUG assert first_coll["oid"] == default_org_id assert first_coll["description"] == DESCRIPTION assert first_coll["caption"] == UPDATED_CAPTION @@ -540,6 +558,7 @@ def test_list_collections( second_coll = [coll for coll in items if coll["name"] == SECOND_COLLECTION_NAME][0] assert second_coll["id"] assert second_coll["name"] == SECOND_COLLECTION_NAME + assert second_coll["slug"] == SECOND_COLLECTION_NAME assert second_coll["oid"] == default_org_id assert second_coll.get("description") is None assert second_coll["crawlCount"] == 1 @@ -888,6 +907,7 @@ def test_list_public_collections( assert collection["name"] assert collection["created"] assert collection["modified"] + assert collection["slug"] assert collection["dateEarliest"] assert collection["dateLatest"] assert collection["crawlCount"] > 0 @@ -1122,6 +1142,7 @@ def test_get_public_collection(default_org_id): assert coll["name"] assert coll["created"] assert coll["modified"] + assert coll["slug"] assert coll["resources"] assert coll["dateEarliest"] assert coll["dateLatest"] @@ -1200,6 +1221,7 @@ def test_get_public_collection_unlisted(crawler_auth_headers, default_org_id): assert coll["name"] assert coll["created"] assert coll["modified"] + assert coll["slug"] assert coll["resources"] assert coll["dateEarliest"] assert coll["dateLatest"] @@ -1240,6 +1262,7 @@ def test_get_public_collection_unlisted_org_profile_disabled( assert coll["name"] assert coll["created"] assert coll["modified"] + assert coll["slug"] assert coll["resources"] assert coll["dateEarliest"] assert coll["dateLatest"] From e8984aa25feee76992fbc46488e58491de4d58d0 Mon Sep 17 00:00:00 2001 From: Tessa Walsh Date: Mon, 13 Jan 2025 16:21:23 -0500 Subject: [PATCH 02/18] Use slugs for public coll urls, support redirects from old slugs In case of redirect, add redirectToSlug field to response with new slug to make it simple for frontend to pick up on redirect. --- backend/btrixcloud/colls.py | 74 +++++++++++++++++++++++++++----- backend/btrixcloud/models.py | 1 + backend/test/test_collections.py | 54 +++++++++++++++++------ 3 files changed, 106 insertions(+), 23 deletions(-) diff --git a/backend/btrixcloud/colls.py b/backend/btrixcloud/colls.py index 8cdd6d8070..dc9584469d 100644 --- a/backend/btrixcloud/colls.py +++ b/backend/btrixcloud/colls.py @@ -153,9 +153,7 @@ async def add_collection(self, oid: UUID, coll_in: CollIn): except pymongo.errors.DuplicateKeyError as err: # pylint: disable=raise-missing-from field = get_duplicate_key_error_field(err) - if field == "slug": - raise HTTPException(status_code=400, detail="collection_slug_taken") - raise HTTPException(status_code=400, detail="collection_name_taken") + raise HTTPException(status_code=400, detail=f"collection_{field}_taken") async def update_collection( self, coll_id: UUID, org: Organization, update: UpdateColl @@ -267,6 +265,27 @@ async def get_collection_raw( return result + async def get_collection_raw_by_slug( + self, + coll_slug: str, + previous_slugs: bool = False, + public_or_unlisted_only: bool = False, + ) -> Dict[str, Any]: + """Get collection by slug (current or previous) as dict from database""" + query: dict[str, object] = {} + if previous_slugs: + query["previousSlugs"] = coll_slug + else: + query["slug"] = coll_slug + if public_or_unlisted_only: + query["access"] = {"$in": ["public", "unlisted"]} + + result = await self.collections.find_one(query) + if not result: + raise HTTPException(status_code=404, detail="collection_not_found") + + return result + async def get_collection( self, coll_id: UUID, public_or_unlisted_only: bool = False ) -> Collection: @@ -274,6 +293,26 @@ async def get_collection( result = await self.get_collection_raw(coll_id, public_or_unlisted_only) return Collection.from_dict(result) + async def get_collection_by_slug( + self, coll_slug: str, public_or_unlisted_only: bool = False + ) -> Tuple[Collection, bool]: + """Get collection by slug, as well as bool indicating if redirected from previous slug""" + try: + result = await self.get_collection_raw_by_slug( + coll_slug, public_or_unlisted_only=public_or_unlisted_only + ) + return Collection.from_dict(result), False + # pylint: disable=broad-exception-caught + except Exception: + pass + + result = await self.get_collection_raw_by_slug( + coll_slug, + previous_slugs=True, + public_or_unlisted_only=public_or_unlisted_only, + ) + return Collection.from_dict(result), True + async def get_collection_out( self, coll_id: UUID, @@ -297,7 +336,11 @@ async def get_collection_out( return CollOut.from_dict(result) async def get_public_collection_out( - self, coll_id: UUID, org: Organization, allow_unlisted: bool = False + self, + coll_id: UUID, + org: Organization, + redirect: bool = False, + allow_unlisted: bool = False, ) -> PublicCollOut: """Get PublicCollOut by id""" result = await self.get_collection_raw(coll_id) @@ -318,6 +361,9 @@ async def get_public_collection_out( org, self.storage_ops ) + if redirect: + result["redirectToSlug"] = result.get("slug", "") + return PublicCollOut.from_dict(result) async def list_collections( @@ -1045,13 +1091,13 @@ async def get_org_public_collections( ) @app.get( - "/public/orgs/{org_slug}/collections/{coll_id}", + "/public/orgs/{org_slug}/collections/{coll_slug}", tags=["collections", "public"], response_model=PublicCollOut, ) async def get_public_collection( org_slug: str, - coll_id: UUID, + coll_slug: str, ): try: org = await colls.orgs.get_org_by_slug(org_slug) @@ -1060,16 +1106,20 @@ async def get_public_collection( # pylint: disable=raise-missing-from raise HTTPException(status_code=404, detail="collection_not_found") - return await colls.get_public_collection_out(coll_id, org, allow_unlisted=True) + coll, redirect = await colls.get_collection_by_slug(coll_slug) + + return await colls.get_public_collection_out( + cast(UUID, coll.id), org, redirect=redirect, allow_unlisted=True + ) @app.get( - "/public/orgs/{org_slug}/collections/{coll_id}/download", + "/public/orgs/{org_slug}/collections/{coll_slug}/download", tags=["collections", "public"], response_model=bytes, ) async def download_public_collection( org_slug: str, - coll_id: UUID, + coll_slug: str, ): try: org = await colls.orgs.get_org_by_slug(org_slug) @@ -1079,12 +1129,14 @@ async def download_public_collection( raise HTTPException(status_code=404, detail="collection_not_found") # Make sure collection exists and is public/unlisted - coll = await colls.get_collection(coll_id, public_or_unlisted_only=True) + coll, _ = await colls.get_collection_by_slug( + coll_slug, public_or_unlisted_only=True + ) if coll.allowPublicDownload is False: raise HTTPException(status_code=403, detail="not_allowed") - return await colls.download_collection(coll_id, org) + return await colls.download_collection(cast(UUID, coll.id), org) @app.get( "/orgs/{oid}/collections/{coll_id}/urls", diff --git a/backend/btrixcloud/models.py b/backend/btrixcloud/models.py index 1cf0d0e99d..458384ca0b 100644 --- a/backend/btrixcloud/models.py +++ b/backend/btrixcloud/models.py @@ -1348,6 +1348,7 @@ class PublicCollOut(BaseMongoModel): defaultThumbnailName: Optional[str] = None allowPublicDownload: bool = True + redirectToSlug: Optional[str] = None # ============================================================================ diff --git a/backend/test/test_collections.py b/backend/test/test_collections.py index c70c779c2f..eca8dcef61 100644 --- a/backend/test/test_collections.py +++ b/backend/test/test_collections.py @@ -18,6 +18,7 @@ DESCRIPTION = "Test description" CAPTION = "Short caption" UPDATED_CAPTION = "Updated caption" +SECOND_PUBLIC_COLL_SLUG = "second-public-collection" NON_PUBLIC_COLL_FIELDS = ( "tags", @@ -830,6 +831,7 @@ def test_list_public_collections( json={ "crawlIds": [crawler_crawl_id], "name": "Second public collection", + "slug": SECOND_PUBLIC_COLL_SLUG, "description": "Lorem ipsum", "access": "public", }, @@ -1131,7 +1133,7 @@ def test_list_public_colls_home_url_thumbnail(): def test_get_public_collection(default_org_id): r = requests.get( - f"{API_PREFIX}/public/orgs/{default_org_slug}/collections/{_public_coll_id}" + f"{API_PREFIX}/public/orgs/{default_org_slug}/collections/{PUBLIC_COLLECTION_SLUG}" ) assert r.status_code == 200 coll = r.json() @@ -1142,13 +1144,14 @@ def test_get_public_collection(default_org_id): assert coll["name"] assert coll["created"] assert coll["modified"] - assert coll["slug"] + assert coll["slug"] == PUBLIC_COLLECTION_SLUG assert coll["resources"] assert coll["dateEarliest"] assert coll["dateLatest"] assert coll["crawlCount"] > 0 assert coll["pageCount"] > 0 assert coll["totalSize"] > 0 + assert coll.get("redirectToSlug") is None for field in NON_PUBLIC_COLL_FIELDS: assert field not in coll @@ -1175,22 +1178,22 @@ def test_get_public_collection(default_org_id): # Invalid org slug - don't reveal whether org exists or not, use # same exception as if collection doesn't exist r = requests.get( - f"{API_PREFIX}/public/orgs/doesntexist/collections/{_public_coll_id}" + f"{API_PREFIX}/public/orgs/doesntexist/collections/{PUBLIC_COLLECTION_SLUG}" ) assert r.status_code == 404 assert r.json()["detail"] == "collection_not_found" - # Invalid collection id + # Unused slug random_uuid = uuid4() r = requests.get( - f"{API_PREFIX}/public/orgs/{default_org_slug}/collections/{random_uuid}" + f"{API_PREFIX}/public/orgs/{default_org_slug}/collections/someslugnotinuse" ) assert r.status_code == 404 assert r.json()["detail"] == "collection_not_found" # Collection isn't public r = requests.get( - f"{API_PREFIX}/public/orgs/{default_org_slug}/collections/{ _coll_id}" + f"{API_PREFIX}/public/orgs/{default_org_slug}/collections/{PUBLIC_COLLECTION_SLUG}" ) assert r.status_code == 404 assert r.json()["detail"] == "collection_not_found" @@ -1210,7 +1213,7 @@ def test_get_public_collection_unlisted(crawler_auth_headers, default_org_id): # Verify single public collection GET endpoint works for unlisted collection r = requests.get( - f"{API_PREFIX}/public/orgs/{default_org_slug}/collections/{_second_public_coll_id}" + f"{API_PREFIX}/public/orgs/{default_org_slug}/collections/{SECOND_PUBLIC_COLL_SLUG}" ) assert r.status_code == 200 coll = r.json() @@ -1221,7 +1224,7 @@ def test_get_public_collection_unlisted(crawler_auth_headers, default_org_id): assert coll["name"] assert coll["created"] assert coll["modified"] - assert coll["slug"] + assert coll["slug"] == SECOND_PUBLIC_COLL_SLUG assert coll["resources"] assert coll["dateEarliest"] assert coll["dateLatest"] @@ -1251,7 +1254,7 @@ def test_get_public_collection_unlisted_org_profile_disabled( # Verify we can still get public details for unlisted collection r = requests.get( - f"{API_PREFIX}/public/orgs/{default_org_slug}/collections/{_second_public_coll_id}" + f"{API_PREFIX}/public/orgs/{default_org_slug}/collections/{SECOND_PUBLIC_COLL_SLUG}" ) assert r.status_code == 200 coll = r.json() @@ -1337,7 +1340,7 @@ def test_unset_collection_home_url( def test_download_streaming_public_collection(crawler_auth_headers, default_org_id): # Check that download is blocked if allowPublicDownload is False with requests.get( - f"{API_PREFIX}/public/orgs/{default_org_slug}/collections/{_public_coll_id}/download", + f"{API_PREFIX}/public/orgs/{default_org_slug}/collections/{PUBLIC_COLLECTION_SLUG}/download", stream=True, ) as r: assert r.status_code == 403 @@ -1355,7 +1358,7 @@ def test_download_streaming_public_collection(crawler_auth_headers, default_org_ with TemporaryFile() as fh: with requests.get( - f"{API_PREFIX}/public/orgs/{default_org_slug}/collections/{_public_coll_id}/download", + f"{API_PREFIX}/public/orgs/{default_org_slug}/collections/{PUBLIC_COLLECTION_SLUG}/download", stream=True, ) as r: assert r.status_code == 200 @@ -1388,7 +1391,7 @@ def test_download_streaming_public_collection_profile_disabled( with TemporaryFile() as fh: with requests.get( - f"{API_PREFIX}/public/orgs/{default_org_slug}/collections/{_public_coll_id}/download", + f"{API_PREFIX}/public/orgs/{default_org_slug}/collections/{PUBLIC_COLLECTION_SLUG}/download", stream=True, ) as r: assert r.status_code == 200 @@ -1405,6 +1408,33 @@ def test_download_streaming_public_collection_profile_disabled( assert zip_file.getinfo(filename).compress_type == ZIP_STORED +def test_get_public_collection_slug_redirect(admin_auth_headers, default_org_id): + # Update public collection slug + new_slug = "new-slug" + + r = requests.patch( + f"{API_PREFIX}/orgs/{default_org_id}/collections/{_public_coll_id}", + headers=admin_auth_headers, + json={ + "slug": new_slug, + }, + ) + assert r.status_code == 200 + assert r.json()["updated"] + + # Get public collection from previous slug + r = requests.get( + f"{API_PREFIX}/public/orgs/{default_org_slug}/collections/{PUBLIC_COLLECTION_SLUG}" + ) + assert r.status_code == 200 + coll = r.json() + + assert coll["id"] == _public_coll_id + assert coll["oid"] == default_org_id + assert coll["slug"] == new_slug + assert coll["redirectToSlug"] == new_slug + + def test_delete_collection(crawler_auth_headers, default_org_id, crawler_crawl_id): # Delete second collection r = requests.delete( From a7da963f6accf2ff49dc1a5dd7bbb1b7a9a98e1e Mon Sep 17 00:00:00 2001 From: Tessa Walsh Date: Mon, 13 Jan 2025 17:27:42 -0500 Subject: [PATCH 03/18] Fix broken tests --- backend/btrixcloud/utils.py | 10 ++++------ backend/test/test_collections.py | 2 +- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/backend/btrixcloud/utils.py b/backend/btrixcloud/utils.py index 468c892421..0d7f84b9a6 100644 --- a/backend/btrixcloud/utils.py +++ b/backend/btrixcloud/utils.py @@ -171,15 +171,13 @@ def stream_dict_list_as_csv( 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 + for field in key_value.keys(): + if field in ("name", "slug"): + return field + return "name" def get_origin(headers) -> str: diff --git a/backend/test/test_collections.py b/backend/test/test_collections.py index eca8dcef61..7debed0ff2 100644 --- a/backend/test/test_collections.py +++ b/backend/test/test_collections.py @@ -1193,7 +1193,7 @@ def test_get_public_collection(default_org_id): # Collection isn't public r = requests.get( - f"{API_PREFIX}/public/orgs/{default_org_slug}/collections/{PUBLIC_COLLECTION_SLUG}" + f"{API_PREFIX}/public/orgs/{default_org_slug}/collections/{UPDATED_SLUG}" ) assert r.status_code == 404 assert r.json()["detail"] == "collection_not_found" From ce26a9e583a25d069740e720202b0016f5f78edf Mon Sep 17 00:00:00 2001 From: Tessa Walsh Date: Mon, 13 Jan 2025 17:29:33 -0500 Subject: [PATCH 04/18] Add slug to collections on org import if not already present --- backend/btrixcloud/orgs.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/backend/btrixcloud/orgs.py b/backend/btrixcloud/orgs.py index d0ca47b2d0..318cf6e2fd 100644 --- a/backend/btrixcloud/orgs.py +++ b/backend/btrixcloud/orgs.py @@ -1306,6 +1306,8 @@ async def import_org( # collections for collection in org_data.get("collections", []): collection = json_stream.to_standard_types(collection) + if not collection.get("slug"): + collection["slug"] = slug_from_name(collection["name"]) await self.colls_db.insert_one(Collection.from_dict(collection).to_dict()) async def delete_org_and_data( From 481f2425a76574301b8619518b9641e21f3f7694 Mon Sep 17 00:00:00 2001 From: Tessa Walsh Date: Mon, 13 Jan 2025 17:55:37 -0500 Subject: [PATCH 05/18] Fix regression with duplicate org sub response --- backend/btrixcloud/utils.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/backend/btrixcloud/utils.py b/backend/btrixcloud/utils.py index 0d7f84b9a6..8a555a4c7c 100644 --- a/backend/btrixcloud/utils.py +++ b/backend/btrixcloud/utils.py @@ -171,12 +171,15 @@ def stream_dict_list_as_csv( def get_duplicate_key_error_field(err: DuplicateKeyError) -> str: """Get name of duplicate field from pymongo DuplicateKeyError""" + allowed_fields = ("name", "slug", "subscription.subId") + if err.details: key_value = err.details.get("keyValue") if key_value: for field in key_value.keys(): - if field in ("name", "slug"): + if field in allowed_fields: return field + return "name" From f8bd543b3ce88cf625a4ddebad3582246eb05027 Mon Sep 17 00:00:00 2001 From: sua yoo Date: Mon, 13 Jan 2025 18:40:03 -0800 Subject: [PATCH 06/18] rename org slug --- frontend/src/classes/BtrixElement.ts | 2 +- .../src/features/collections/collections-grid.ts | 2 +- .../src/features/collections/share-collection.ts | 2 +- .../src/features/crawl-workflows/workflow-list.ts | 2 +- frontend/src/index.ts | 12 ++++++++---- frontend/src/pages/collections/collection.ts | 2 +- frontend/src/pages/log-in.ts | 5 +++-- frontend/src/pages/org/archived-item-detail/ui/qa.ts | 2 +- frontend/src/pages/org/collections-list.ts | 2 +- frontend/src/pages/org/dashboard.ts | 6 +++--- frontend/src/pages/org/index.ts | 8 ++++---- frontend/src/pages/org/profile.ts | 2 +- .../src/pages/org/settings/components/profile.ts | 4 ++-- frontend/src/pages/org/settings/settings.ts | 4 ++-- frontend/src/utils/LiteElement.ts | 2 +- 15 files changed, 31 insertions(+), 26 deletions(-) diff --git a/frontend/src/classes/BtrixElement.ts b/frontend/src/classes/BtrixElement.ts index 8bfc76484c..be977083b1 100644 --- a/frontend/src/classes/BtrixElement.ts +++ b/frontend/src/classes/BtrixElement.ts @@ -32,7 +32,7 @@ export class BtrixElement extends TailwindElement { return this.appState.orgId; } - protected get orgSlug() { + protected get orgSlugState() { return this.appState.orgSlug; } diff --git a/frontend/src/features/collections/collections-grid.ts b/frontend/src/features/collections/collections-grid.ts index 1eb925594a..c5894f39dd 100644 --- a/frontend/src/features/collections/collections-grid.ts +++ b/frontend/src/features/collections/collections-grid.ts @@ -109,7 +109,7 @@ export class CollectionsGrid extends BtrixElement { ${msg("Visit Public Page")} diff --git a/frontend/src/features/collections/share-collection.ts b/frontend/src/features/collections/share-collection.ts index ea9fe449f9..8170068413 100644 --- a/frontend/src/features/collections/share-collection.ts +++ b/frontend/src/features/collections/share-collection.ts @@ -59,7 +59,7 @@ export class ShareCollection extends BtrixElement { private get shareLink() { const baseUrl = `${window.location.protocol}//${window.location.hostname}${window.location.port ? `:${window.location.port}` : ""}`; if (this.collection) { - return `${baseUrl}/${this.collection.access === CollectionAccess.Private ? `${RouteNamespace.PrivateOrgs}/${this.orgSlug}/collections/view` : `${RouteNamespace.PublicOrgs}/${this.orgSlug}/collections`}/${this.collectionId}`; + return `${baseUrl}/${this.collection.access === CollectionAccess.Private ? `${RouteNamespace.PrivateOrgs}/${this.orgSlugState}/collections/view` : `${RouteNamespace.PublicOrgs}/${this.orgSlugState}/collections`}/${this.collectionId}`; } return ""; } diff --git a/frontend/src/features/crawl-workflows/workflow-list.ts b/frontend/src/features/crawl-workflows/workflow-list.ts index a1b3629c5f..5c776dd93e 100644 --- a/frontend/src/features/crawl-workflows/workflow-list.ts +++ b/frontend/src/features/crawl-workflows/workflow-list.ts @@ -220,7 +220,7 @@ export class WorkflowListItem extends BtrixElement { } e.preventDefault(); await this.updateComplete; - const href = `/orgs/${this.orgSlug}/workflows/${ + const href = `/orgs/${this.orgSlugState}/workflows/${ this.workflow?.id }#${this.workflow?.isCrawlRunning ? "watch" : "crawls"}`; this.navigate.to(href); diff --git a/frontend/src/index.ts b/frontend/src/index.ts index 3f310ae254..0865ee4e91 100644 --- a/frontend/src/index.ts +++ b/frontend/src/index.ts @@ -113,8 +113,12 @@ export class App extends BtrixElement { @query("#userGuideDrawer") private readonly userGuideDrawer!: SlDrawer; + get orgSlugInPath() { + return this.viewState.params.slug || ""; + } + get isUserInCurrentOrg(): boolean { - const { slug } = this.viewState.params; + const slug = this.orgSlugInPath; if (!this.userInfo || !slug) return false; return Boolean(this.userInfo.orgs.some((org) => org.slug === slug)); } @@ -567,12 +571,12 @@ export class App extends BtrixElement { const orgs = this.userInfo?.orgs; if (!orgs) return; - const selectedOption = this.orgSlug - ? orgs.find(({ slug }) => slug === this.orgSlug) + const selectedOption = this.orgSlugInPath + ? orgs.find(({ slug }) => slug === this.orgSlugInPath) : { slug: "", name: msg("All Organizations") }; if (!selectedOption) { console.debug( - `Couldn't find organization with slug ${this.orgSlug}`, + `Couldn't find organization with slug ${this.orgSlugInPath}`, orgs, ); return; diff --git a/frontend/src/pages/collections/collection.ts b/frontend/src/pages/collections/collection.ts index 1b54f6467a..5d5052efb7 100644 --- a/frontend/src/pages/collections/collection.ts +++ b/frontend/src/pages/collections/collection.ts @@ -30,7 +30,7 @@ export class Collection extends BtrixElement { tab: Tab | string = Tab.Replay; get canEditCollection() { - return this.slug === this.orgSlug && this.appState.isCrawler; + return this.slug === this.orgSlugState && this.appState.isCrawler; } private readonly tabLabels: Record< diff --git a/frontend/src/pages/log-in.ts b/frontend/src/pages/log-in.ts index 849a92eaeb..85640f5143 100644 --- a/frontend/src/pages/log-in.ts +++ b/frontend/src/pages/log-in.ts @@ -389,8 +389,9 @@ export class LogInPage extends BtrixElement { // Check if org slug in app state matches newly logged in user const slug = - this.orgSlug && data.user.orgs.some((org) => org.slug === this.orgSlug) - ? this.orgSlug + this.orgSlugState && + data.user.orgs.some((org) => org.slug === this.orgSlugState) + ? this.orgSlugState : data.user.orgs[0].slug; AppStateService.updateUser(formatAPIUser(data.user), slug); diff --git a/frontend/src/pages/org/archived-item-detail/ui/qa.ts b/frontend/src/pages/org/archived-item-detail/ui/qa.ts index 92d300168a..f7baa46ea8 100644 --- a/frontend/src/pages/org/archived-item-detail/ui/qa.ts +++ b/frontend/src/pages/org/archived-item-detail/ui/qa.ts @@ -269,7 +269,7 @@ export class ArchivedItemDetailQA extends BtrixElement { ? html`— ${msg("View error logs")}` : ""} diff --git a/frontend/src/pages/org/collections-list.ts b/frontend/src/pages/org/collections-list.ts index 546a749e53..9e7761c041 100644 --- a/frontend/src/pages/org/collections-list.ts +++ b/frontend/src/pages/org/collections-list.ts @@ -110,7 +110,7 @@ export class CollectionsList extends BtrixElement { }); private getShareLink(collection: Collection) { - return `${window.location.protocol}//${window.location.hostname}${window.location.port ? `:${window.location.port}` : ""}/${collection.access === CollectionAccess.Private ? `${RouteNamespace.PrivateOrgs}/${this.orgSlug}/collections/view` : `${RouteNamespace.PublicOrgs}/${this.orgSlug}/collections`}/${collection.id}`; + return `${window.location.protocol}//${window.location.hostname}${window.location.port ? `:${window.location.port}` : ""}/${collection.access === CollectionAccess.Private ? `${RouteNamespace.PrivateOrgs}/${this.orgSlugState}/collections/view` : `${RouteNamespace.PublicOrgs}/${this.orgSlugState}/collections`}/${collection.id}`; } private get hasSearchStr() { diff --git a/frontend/src/pages/org/dashboard.ts b/frontend/src/pages/org/dashboard.ts index 1978ef25f1..8315504a49 100644 --- a/frontend/src/pages/org/dashboard.ts +++ b/frontend/src/pages/org/dashboard.ts @@ -62,7 +62,7 @@ export class Dashboard extends BtrixElement { const collections = await this.fetchCollections({ slug }); return collections; }, - args: () => [this.orgSlug, this.metrics] as const, + args: () => [this.orgSlugState, this.metrics] as const, }); willUpdate(changedProperties: PropertyValues & Map) { @@ -282,7 +282,7 @@ export class Dashboard extends BtrixElement { ${msg("Manage Collections")} ${this.org?.enablePublicProfile @@ -307,7 +307,7 @@ export class Dashboard extends BtrixElement {
${this.renderNoPublicCollections()} diff --git a/frontend/src/pages/org/index.ts b/frontend/src/pages/org/index.ts index 6e55e6cff5..e27e4d09d6 100644 --- a/frontend/src/pages/org/index.ts +++ b/frontend/src/pages/org/index.ts @@ -154,7 +154,7 @@ export class Org extends BtrixElement { if ( changedProperties.has("appState.orgSlug") && this.userInfo && - this.orgSlug + this.orgSlugState ) { if (this.userOrg) { void this.updateOrg(); @@ -236,14 +236,14 @@ export class Org extends BtrixElement { async firstUpdated() { // if slug is actually an orgId (UUID), attempt to lookup the slug // and redirect to the slug url - if (this.orgSlug && UUID_REGEX.test(this.orgSlug)) { - const org = await this.getOrg(this.orgSlug); + if (this.orgSlugState && UUID_REGEX.test(this.orgSlugState)) { + const org = await this.getOrg(this.orgSlugState); const actualSlug = org?.slug; if (actualSlug) { this.navigate.to( window.location.href .slice(window.location.origin.length) - .replace(this.orgSlug, actualSlug), + .replace(this.orgSlugState, actualSlug), ); return; } diff --git a/frontend/src/pages/org/profile.ts b/frontend/src/pages/org/profile.ts index d0687d1aef..2e0a373447 100644 --- a/frontend/src/pages/org/profile.ts +++ b/frontend/src/pages/org/profile.ts @@ -18,7 +18,7 @@ export class OrgProfile extends BtrixElement { private isPrivatePreview = false; get canEditOrg() { - return this.slug === this.orgSlug && this.appState.isAdmin; + return this.slug === this.orgSlugState && this.appState.isAdmin; } private readonly orgCollections = new Task(this, { diff --git a/frontend/src/pages/org/settings/components/profile.ts b/frontend/src/pages/org/settings/components/profile.ts index b5af3a3582..fc1708fb3c 100644 --- a/frontend/src/pages/org/settings/components/profile.ts +++ b/frontend/src/pages/org/settings/components/profile.ts @@ -71,7 +71,7 @@ export class OrgSettingsProfile extends BtrixElement {
@@ -97,7 +97,7 @@ export class OrgSettingsProfile extends BtrixElement {
${columns(cols)}
+ ${when( + this.collection?.access != CollectionAccess.Private, + () => html`
${this.renderShareLink()}
`, + )}
${msg("Thumbnail")} @@ -409,7 +417,7 @@ export class ShareCollection extends BtrixElement { private readonly renderShareLink = () => { return html` -
+
${msg("Link to Share")}
`; case "publicCollection": { - const { collectionId, collectionTab } = this.viewState.params; + const { collectionSlug, collectionTab } = this.viewState.params; - if (!collectionId) { + if (!collectionSlug) { break; } return html``; } diff --git a/frontend/src/pages/collections/collection.ts b/frontend/src/pages/collections/collection.ts index 5d5052efb7..aeb610b101 100644 --- a/frontend/src/pages/collections/collection.ts +++ b/frontend/src/pages/collections/collection.ts @@ -21,16 +21,16 @@ enum Tab { @customElement("btrix-collection") export class Collection extends BtrixElement { @property({ type: String }) - slug?: string; + orgSlug?: string; @property({ type: String }) - collectionId?: string; + collectionSlug?: string; @property({ type: String }) tab: Tab | string = Tab.Replay; get canEditCollection() { - return this.slug === this.orgSlugState && this.appState.isCrawler; + return this.orgSlug === this.orgSlugState && this.appState.isCrawler; } private readonly tabLabels: Record< @@ -48,19 +48,22 @@ export class Collection extends BtrixElement { }; private readonly orgCollections = new Task(this, { - task: async ([slug]) => { - if (!slug) throw new Error("slug required"); - const org = await this.fetchCollections({ slug }); + task: async ([orgSlug]) => { + if (!orgSlug) throw new Error("orgSlug required"); + const org = await this.fetchCollections({ orgSlug }); return org; }, - args: () => [this.slug] as const, + args: () => [this.orgSlug] as const, }); private readonly collection = new Task(this, { - task: async ([slug, collectionId]) => { - if (!slug || !collectionId) - throw new Error("slug and collection required"); - const collection = await this.fetchCollection({ slug, collectionId }); + task: async ([orgSlug, collectionSlug]) => { + if (!orgSlug || !collectionSlug) + throw new Error("orgSlug and collection required"); + const collection = await this.fetchCollection({ + orgSlug, + collectionSlug, + }); if (!collection.crawlCount && (this.tab as unknown) === Tab.Replay) { this.tab = Tab.About; @@ -68,7 +71,7 @@ export class Collection extends BtrixElement { return collection; }, - args: () => [this.slug, this.collectionId] as const, + args: () => [this.orgSlug, this.collectionSlug] as const, }); render() { @@ -84,11 +87,11 @@ export class Collection extends BtrixElement { breadcrumbs: org ? [ { - href: `/${RouteNamespace.PublicOrgs}/${this.slug}`, + href: `/${RouteNamespace.PublicOrgs}/${this.orgSlug}`, content: org.name, }, { - href: `/${RouteNamespace.PublicOrgs}/${this.slug}`, + href: `/${RouteNamespace.PublicOrgs}/${this.orgSlug}`, content: msg("Collections"), }, { @@ -103,8 +106,8 @@ export class Collection extends BtrixElement { () => html` `, @@ -170,7 +173,7 @@ export class Collection extends BtrixElement { { - const resp = await fetch(`/api/public/orgs/${slug}/collections`, { + const resp = await fetch(`/api/public/orgs/${orgSlug}/collections`, { headers: { "Content-Type": "application/json" }, }); @@ -281,14 +284,14 @@ export class Collection extends BtrixElement { } private async fetchCollection({ - slug, - collectionId, + orgSlug, + collectionSlug, }: { - slug: string; - collectionId: string; + orgSlug: string; + collectionSlug: string; }): Promise { const resp = await fetch( - `/api/public/orgs/${slug}/collections/${collectionId}`, + `/api/public/orgs/${orgSlug}/collections/${collectionSlug}`, { headers: { "Content-Type": "application/json" }, }, diff --git a/frontend/src/pages/org/collections-list.ts b/frontend/src/pages/org/collections-list.ts index 9e7761c041..4a1a1d0c7c 100644 --- a/frontend/src/pages/org/collections-list.ts +++ b/frontend/src/pages/org/collections-list.ts @@ -110,7 +110,7 @@ export class CollectionsList extends BtrixElement { }); private getShareLink(collection: Collection) { - return `${window.location.protocol}//${window.location.hostname}${window.location.port ? `:${window.location.port}` : ""}/${collection.access === CollectionAccess.Private ? `${RouteNamespace.PrivateOrgs}/${this.orgSlugState}/collections/view` : `${RouteNamespace.PublicOrgs}/${this.orgSlugState}/collections`}/${collection.id}`; + return `${window.location.protocol}//${window.location.hostname}${window.location.port ? `:${window.location.port}` : ""}/${collection.access === CollectionAccess.Private ? `${RouteNamespace.PrivateOrgs}/${this.orgSlugState}/collections/view` : `${RouteNamespace.PublicOrgs}/${this.orgSlugState}/collections`}/${collection.slug}`; } private get hasSearchStr() { diff --git a/frontend/src/pages/org/profile.ts b/frontend/src/pages/org/profile.ts index 2e0a373447..fdcbb73a10 100644 --- a/frontend/src/pages/org/profile.ts +++ b/frontend/src/pages/org/profile.ts @@ -12,13 +12,13 @@ import type { OrgData, PublicOrgCollections } from "@/types/org"; @customElement("btrix-org-profile") export class OrgProfile extends BtrixElement { @property({ type: String }) - slug?: string; + orgSlug?: string; @state() private isPrivatePreview = false; get canEditOrg() { - return this.slug === this.orgSlugState && this.appState.isAdmin; + return this.orgSlug === this.orgSlugState && this.appState.isAdmin; } private readonly orgCollections = new Task(this, { @@ -27,11 +27,11 @@ export class OrgProfile extends BtrixElement { const org = await this.fetchCollections({ slug }); return org; }, - args: () => [this.slug] as const, + args: () => [this.orgSlug] as const, }); render() { - if (!this.slug) { + if (!this.orgSlug) { return this.renderError(); } @@ -196,7 +196,7 @@ export class OrgProfile extends BtrixElement {
@@ -270,7 +270,7 @@ export class OrgProfile extends BtrixElement { private async getUserOrg(): Promise { try { const userInfo = this.userInfo || (await this.api.fetch("/users/me")); - const userOrg = userInfo?.orgs.find((org) => org.slug === this.slug); + const userOrg = userInfo?.orgs.find((org) => org.slug === this.orgSlug); if (!userOrg) { return null; diff --git a/frontend/src/routes.ts b/frontend/src/routes.ts index 598de394aa..a6b20cde1a 100644 --- a/frontend/src/routes.ts +++ b/frontend/src/routes.ts @@ -36,7 +36,7 @@ export const ROUTES = { ].join(""), publicOrgs: `/${RouteNamespace.PublicOrgs}(/)`, publicOrgProfile: `/${RouteNamespace.PublicOrgs}/:slug(/)`, - publicCollection: `/${RouteNamespace.PublicOrgs}/:slug/collections/:collectionId(/:collectionTab)`, + publicCollection: `/${RouteNamespace.PublicOrgs}/:slug/collections/:collectionSlug(/:collectionTab)`, users: "/users", usersInvite: "/users/invite", crawls: "/crawls", diff --git a/frontend/src/types/collection.ts b/frontend/src/types/collection.ts index 47968031e7..1c0887bd52 100644 --- a/frontend/src/types/collection.ts +++ b/frontend/src/types/collection.ts @@ -8,6 +8,7 @@ export enum CollectionAccess { export const publicCollectionSchema = z.object({ id: z.string(), + slug: z.string(), oid: z.string(), name: z.string(), caption: z.string().nullable(), From 2d241189f7302a3622208191343430b634dcd338 Mon Sep 17 00:00:00 2001 From: sua yoo Date: Mon, 13 Jan 2025 19:29:18 -0800 Subject: [PATCH 08/18] update breadcrumbs --- frontend/src/pages/collections/collection.ts | 28 +++++++++----------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/frontend/src/pages/collections/collection.ts b/frontend/src/pages/collections/collection.ts index aeb610b101..2b30f66c5d 100644 --- a/frontend/src/pages/collections/collection.ts +++ b/frontend/src/pages/collections/collection.ts @@ -1,5 +1,5 @@ import { localized, msg, str } from "@lit/localize"; -import { Task } from "@lit/task"; +import { Task, TaskStatus } from "@lit/task"; import { html, type TemplateResult } from "lit"; import { customElement, property } from "lit/decorators.js"; import { ifDefined } from "lit/directives/if-defined.js"; @@ -84,21 +84,17 @@ export class Collection extends BtrixElement { private readonly renderComplete = (collection: PublicCollection) => { const org = this.orgCollections.value?.org; const header: Parameters[0] = { - breadcrumbs: org - ? [ - { - href: `/${RouteNamespace.PublicOrgs}/${this.orgSlug}`, - content: org.name, - }, - { - href: `/${RouteNamespace.PublicOrgs}/${this.orgSlug}`, - content: msg("Collections"), - }, - { - content: collection.name, - }, - ] - : [], + breadcrumbs: + this.orgCollections.status > TaskStatus.PENDING + ? org + ? [ + { + href: `/${RouteNamespace.PublicOrgs}/${this.orgSlug}`, + content: org.name, + }, + ] + : undefined + : [], title: collection.name || "", actions: html` ${when( From dd1edf5fb195c909c546d4369a9a16004709adfb Mon Sep 17 00:00:00 2001 From: sua yoo Date: Mon, 13 Jan 2025 19:29:58 -0800 Subject: [PATCH 09/18] fix replay --- frontend/src/pages/collections/collection.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/src/pages/collections/collection.ts b/frontend/src/pages/collections/collection.ts index 2b30f66c5d..573d3717f9 100644 --- a/frontend/src/pages/collections/collection.ts +++ b/frontend/src/pages/collections/collection.ts @@ -183,7 +183,7 @@ export class Collection extends BtrixElement { private renderReplay(collection: PublicCollection) { const replaySource = new URL( - `/api/orgs/${collection.oid}/collections/${this.collectionSlug}/public/replay.json`, + `/api/orgs/${collection.oid}/collections/${collection.id}/public/replay.json`, window.location.href, ).href; From 88602f5f8390c37b193278469e4bb3e7c09be7fd Mon Sep 17 00:00:00 2001 From: sua yoo Date: Mon, 13 Jan 2025 19:37:51 -0800 Subject: [PATCH 10/18] add redirect --- frontend/src/features/collections/share-collection.ts | 2 +- frontend/src/pages/collections/collection.ts | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/frontend/src/features/collections/share-collection.ts b/frontend/src/features/collections/share-collection.ts index 08d6f28aa6..6b7da671ea 100644 --- a/frontend/src/features/collections/share-collection.ts +++ b/frontend/src/features/collections/share-collection.ts @@ -62,7 +62,7 @@ export class ShareCollection extends BtrixElement { return `${baseUrl}/${ this.collection.access === CollectionAccess.Private ? `${RouteNamespace.PrivateOrgs}/${this.orgSlugState}/collections/view/${this.collectionId}` - : `${RouteNamespace.PublicOrgs}/${this.orgSlugState}/collections/${this.collection.slug}` + : `${RouteNamespace.PublicOrgs}/${this.orgSlug}/collections/${this.collection.slug}` }`; } return ""; diff --git a/frontend/src/pages/collections/collection.ts b/frontend/src/pages/collections/collection.ts index 573d3717f9..078b043c54 100644 --- a/frontend/src/pages/collections/collection.ts +++ b/frontend/src/pages/collections/collection.ts @@ -69,6 +69,13 @@ export class Collection extends BtrixElement { this.tab = Tab.About; } + if (collection.slug !== this.collectionSlug) { + // Redirect to newest slug + this.navigate.to( + `/${RouteNamespace.PublicOrgs}/${this.orgSlug}/collections/${collection.slug}`, + ); + } + return collection; }, args: () => [this.orgSlug, this.collectionSlug] as const, From 80764f68f22998668f8919ee8849ab89f31c9078 Mon Sep 17 00:00:00 2001 From: sua yoo Date: Mon, 13 Jan 2025 20:09:37 -0800 Subject: [PATCH 11/18] update org slug --- frontend/src/pages/org/collection-detail.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/frontend/src/pages/org/collection-detail.ts b/frontend/src/pages/org/collection-detail.ts index 7e9472d7f3..d108c16ec7 100644 --- a/frontend/src/pages/org/collection-detail.ts +++ b/frontend/src/pages/org/collection-detail.ts @@ -136,6 +136,7 @@ export class CollectionDetail extends BtrixElement { : nothing, actions: html` { From 5bdca9ffbbc8244adf54da53e58b6c5e36543275 Mon Sep 17 00:00:00 2001 From: sua yoo Date: Mon, 13 Jan 2025 20:18:13 -0800 Subject: [PATCH 12/18] track collection by slug --- frontend/docs/docs/deploy/customization.md | 3 +-- .../src/features/collections/share-collection.ts | 16 ++++++++-------- frontend/src/index.ts | 6 +++--- frontend/src/utils/analytics.ts | 3 +-- 4 files changed, 13 insertions(+), 15 deletions(-) diff --git a/frontend/docs/docs/deploy/customization.md b/frontend/docs/docs/deploy/customization.md index fd3a8eeb20..66260caa8e 100644 --- a/frontend/docs/docs/deploy/customization.md +++ b/frontend/docs/docs/deploy/customization.md @@ -238,8 +238,7 @@ type btrixEvent = ( extra?: { props?: { org_slug: string | null; - collection_id?: string | null; - collection_name?: string | null; + collection_slug?: string | null; logged_in?: boolean; }; }, diff --git a/frontend/src/features/collections/share-collection.ts b/frontend/src/features/collections/share-collection.ts index 6b7da671ea..e1d5712608 100644 --- a/frontend/src/features/collections/share-collection.ts +++ b/frontend/src/features/collections/share-collection.ts @@ -120,12 +120,13 @@ export class ShareCollection extends BtrixElement { @click=${() => { void this.clipboardController.copy(this.shareLink); - track(AnalyticsTrackEvent.CopyShareCollectionLink, { - org_slug: this.orgSlug, - collection_id: this.collectionId, - collection_name: this.collection?.name, - logged_in: !!this.authState, - }); + if (this.collection?.access === CollectionAccess.Public) { + track(AnalyticsTrackEvent.CopyShareCollectionLink, { + org_slug: this.orgSlug, + collection_slug: this.collection.slug, + logged_in: !!this.authState, + }); + } }} > { track(AnalyticsTrackEvent.DownloadPublicCollection, { org_slug: this.orgSlug, - collection_id: this.collectionId, - collection_name: this.collection?.name, + collection_slug: this.collection?.slug, }); }} > diff --git a/frontend/src/index.ts b/frontend/src/index.ts index 2dc7afe7a5..7054a37300 100644 --- a/frontend/src/index.ts +++ b/frontend/src/index.ts @@ -310,14 +310,14 @@ export class App extends BtrixElement { } trackPageView() { - const { slug, collectionId } = this.viewState.params; + const { slug, collectionSlug } = this.viewState.params; const pageViewProps: AnalyticsTrackProps = { org_slug: slug || null, logged_in: !!this.authState, }; - if (collectionId) { - pageViewProps.collection_id = collectionId; + if (collectionSlug) { + pageViewProps.collection_slug = collectionSlug; } pageView(pageViewProps); diff --git a/frontend/src/utils/analytics.ts b/frontend/src/utils/analytics.ts index 4df9fc6496..eacdb33faf 100644 --- a/frontend/src/utils/analytics.ts +++ b/frontend/src/utils/analytics.ts @@ -9,8 +9,7 @@ import { AnalyticsTrackEvent } from "../trackEvents"; export type AnalyticsTrackProps = { org_slug: string | null; - collection_id?: string | null; - collection_name?: string | null; + collection_slug?: string | null; logged_in?: boolean; }; From c0bc55917594d4fafae5ab8d46625bcf35524b23 Mon Sep 17 00:00:00 2001 From: sua yoo Date: Mon, 13 Jan 2025 20:23:38 -0800 Subject: [PATCH 13/18] use redirect value --- frontend/src/pages/collections/collection.ts | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/frontend/src/pages/collections/collection.ts b/frontend/src/pages/collections/collection.ts index 078b043c54..10392940d3 100644 --- a/frontend/src/pages/collections/collection.ts +++ b/frontend/src/pages/collections/collection.ts @@ -65,17 +65,16 @@ export class Collection extends BtrixElement { collectionSlug, }); - if (!collection.crawlCount && (this.tab as unknown) === Tab.Replay) { - this.tab = Tab.About; - } - - if (collection.slug !== this.collectionSlug) { - // Redirect to newest slug + if (collection.redirectToSlug) { this.navigate.to( - `/${RouteNamespace.PublicOrgs}/${this.orgSlug}/collections/${collection.slug}`, + `/${RouteNamespace.PublicOrgs}/${this.orgSlug}/collections/${collection.redirectToSlug}`, ); } + if (!collection.crawlCount && (this.tab as unknown) === Tab.Replay) { + this.tab = Tab.About; + } + return collection; }, args: () => [this.orgSlug, this.collectionSlug] as const, @@ -292,7 +291,7 @@ export class Collection extends BtrixElement { }: { orgSlug: string; collectionSlug: string; - }): Promise { + }): Promise { const resp = await fetch( `/api/public/orgs/${orgSlug}/collections/${collectionSlug}`, { From d9478ad06f70489db913d5d7a9986ecfecd4f8b0 Mon Sep 17 00:00:00 2001 From: sua yoo Date: Mon, 13 Jan 2025 20:33:16 -0800 Subject: [PATCH 14/18] add link to copy share --- frontend/src/pages/org/dashboard.ts | 45 +++++++++++++++++++++-------- 1 file changed, 33 insertions(+), 12 deletions(-) diff --git a/frontend/src/pages/org/dashboard.ts b/frontend/src/pages/org/dashboard.ts index 8315504a49..1e22e677cd 100644 --- a/frontend/src/pages/org/dashboard.ts +++ b/frontend/src/pages/org/dashboard.ts @@ -1,7 +1,7 @@ import { localized, msg } from "@lit/localize"; import { Task } from "@lit/task"; import type { SlSelectEvent } from "@shoelace-style/shoelace"; -import { html, nothing, type PropertyValues, type TemplateResult } from "lit"; +import { html, type PropertyValues, type TemplateResult } from "lit"; import { customElement, property, state } from "lit/decorators.js"; import { ifDefined } from "lit/directives/if-defined.js"; import { when } from "lit/directives/when.js"; @@ -9,6 +9,7 @@ import { when } from "lit/directives/when.js"; import type { SelectNewDialogEvent } from "."; import { BtrixElement } from "@/classes/BtrixElement"; +import { ClipboardController } from "@/controllers/clipboard"; import { pageHeading } from "@/layouts/page"; import { pageHeader } from "@/layouts/pageHeader"; import { RouteNamespace } from "@/routes"; @@ -289,17 +290,37 @@ export class Dashboard extends BtrixElement { ? msg("Visit Public Profile") : msg("Preview Public Profile")} - ${this.org && !this.org.enablePublicProfile - ? html` - - - - ${msg("Update Org Visibility")} - - ` - : nothing} + ${when(this.org, (org) => + org.enablePublicProfile + ? html` + { + ClipboardController.copyToClipboard( + `${window.location.protocol}//${window.location.hostname}${ + window.location.port + ? `:${window.location.port}` + : "" + }/${RouteNamespace.PublicOrgs}/${this.orgSlugState}`, + ); + this.notify.toast({ + message: msg("Link copied"), + }); + }} + > + + ${msg("Copy Link to Profile")} + + ` + : html` + + + + ${msg("Update Org Visibility")} + + `, + )} `, From a09316e245bb01bef59474103b274b6506a58600 Mon Sep 17 00:00:00 2001 From: Tessa Walsh Date: Tue, 14 Jan 2025 10:59:15 -0500 Subject: [PATCH 15/18] Rename migration to 0039 --- backend/btrixcloud/db.py | 2 +- ...ration_0038_coll_slugs.py => migration_0039_coll_slugs.py} | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) rename backend/btrixcloud/migrations/{migration_0038_coll_slugs.py => migration_0039_coll_slugs.py} (94%) diff --git a/backend/btrixcloud/db.py b/backend/btrixcloud/db.py index 9f0bab0446..504830f068 100644 --- a/backend/btrixcloud/db.py +++ b/backend/btrixcloud/db.py @@ -17,7 +17,7 @@ from .migrations import BaseMigration -CURR_DB_VERSION = "0038" +CURR_DB_VERSION = "0039" # ============================================================================ diff --git a/backend/btrixcloud/migrations/migration_0038_coll_slugs.py b/backend/btrixcloud/migrations/migration_0039_coll_slugs.py similarity index 94% rename from backend/btrixcloud/migrations/migration_0038_coll_slugs.py rename to backend/btrixcloud/migrations/migration_0039_coll_slugs.py index 362f0afebe..6fa4cd01b0 100644 --- a/backend/btrixcloud/migrations/migration_0038_coll_slugs.py +++ b/backend/btrixcloud/migrations/migration_0039_coll_slugs.py @@ -1,12 +1,12 @@ """ -Migration 0038 -- collection access +Migration 0039 -- collection slugs """ from btrixcloud.migrations import BaseMigration from btrixcloud.utils import slug_from_name -MIGRATION_VERSION = "0038" +MIGRATION_VERSION = "0039" class Migration(BaseMigration): From 943f91bdb27709566825b990c8f94c20ec743b17 Mon Sep 17 00:00:00 2001 From: Tessa Walsh Date: Tue, 14 Jan 2025 14:35:19 -0500 Subject: [PATCH 16/18] Change id typing for collections to always be UUID --- backend/btrixcloud/colls.py | 4 ++-- backend/btrixcloud/models.py | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/backend/btrixcloud/colls.py b/backend/btrixcloud/colls.py index dc9584469d..00e6ad1df8 100644 --- a/backend/btrixcloud/colls.py +++ b/backend/btrixcloud/colls.py @@ -1109,7 +1109,7 @@ async def get_public_collection( coll, redirect = await colls.get_collection_by_slug(coll_slug) return await colls.get_public_collection_out( - cast(UUID, coll.id), org, redirect=redirect, allow_unlisted=True + coll.id, org, redirect=redirect, allow_unlisted=True ) @app.get( @@ -1136,7 +1136,7 @@ async def download_public_collection( if coll.allowPublicDownload is False: raise HTTPException(status_code=403, detail="not_allowed") - return await colls.download_collection(cast(UUID, coll.id), org) + return await colls.download_collection(coll.id, org) @app.get( "/orgs/{oid}/collections/{coll_id}/urls", diff --git a/backend/btrixcloud/models.py b/backend/btrixcloud/models.py index 458384ca0b..43695c1efb 100644 --- a/backend/btrixcloud/models.py +++ b/backend/btrixcloud/models.py @@ -1235,6 +1235,7 @@ class CollAccessType(str, Enum): class Collection(BaseMongoModel): """Org collection structure""" + id: UUID name: str = Field(..., min_length=1) slug: str = Field(..., min_length=1) oid: UUID @@ -1288,6 +1289,7 @@ class CollIn(BaseModel): class CollOut(BaseMongoModel): """Collection output model with annotations.""" + id: UUID name: str slug: str oid: UUID @@ -1323,6 +1325,7 @@ class CollOut(BaseMongoModel): class PublicCollOut(BaseMongoModel): """Collection output model with annotations.""" + id: UUID name: str slug: str oid: UUID From f013e1f13dd353ed406e5f8a40eae12a4c5e2c64 Mon Sep 17 00:00:00 2001 From: Tessa Walsh Date: Wed, 15 Jan 2025 16:32:31 -0500 Subject: [PATCH 17/18] Remove new slug from previousSlugs of other collections in same org --- backend/btrixcloud/colls.py | 22 +++++++++++++++++++--- backend/test/test_collections.py | 26 ++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 3 deletions(-) diff --git a/backend/btrixcloud/colls.py b/backend/btrixcloud/colls.py index 00e6ad1df8..34b3d36713 100644 --- a/backend/btrixcloud/colls.py +++ b/backend/btrixcloud/colls.py @@ -121,17 +121,18 @@ async def add_collection(self, oid: UUID, coll_in: CollIn): crawl_ids = coll_in.crawlIds if coll_in.crawlIds else [] coll_id = uuid4() created = dt_now() - modified = dt_now() + + slug = coll_in.slug or slug_from_name(coll_in.name) coll = Collection( id=coll_id, oid=oid, name=coll_in.name, - slug=coll_in.slug or slug_from_name(coll_in.name), + slug=slug, description=coll_in.description, caption=coll_in.caption, created=created, - modified=modified, + modified=created, access=coll_in.access, defaultThumbnailName=coll_in.defaultThumbnailName, allowPublicDownload=coll_in.allowPublicDownload, @@ -139,6 +140,8 @@ async def add_collection(self, oid: UUID, coll_in: CollIn): try: await self.collections.insert_one(coll.to_dict()) org = await self.orgs.get_org_by_id(oid) + await self.clear_org_previous_slugs_matching_slug(slug, org) + if crawl_ids: await self.crawl_ops.add_to_collection(crawl_ids, coll_id, org) await self.update_collection_counts_and_tags(coll_id) @@ -177,6 +180,7 @@ async def update_collection( if name_update and not slug_update: slug = slug_from_name(name_update) query["slug"] = slug + slug_update = slug query["modified"] = dt_now() @@ -198,8 +202,20 @@ async def update_collection( if not result: raise HTTPException(status_code=404, detail="collection_not_found") + if slug_update: + await self.clear_org_previous_slugs_matching_slug(slug_update, org) + return {"updated": True} + async def clear_org_previous_slugs_matching_slug( + self, slug: str, org: Organization + ): + """Clear new slug from previousSlugs array of other collections in same org""" + await self.collections.update_many( + {"oid": org.id, "previousSlugs": slug}, + {"$pull": {"previousSlugs": slug}}, + ) + async def add_crawls_to_collection( self, coll_id: UUID, crawl_ids: List[str], org: Organization ) -> CollOut: diff --git a/backend/test/test_collections.py b/backend/test/test_collections.py index 7debed0ff2..de0fa84faa 100644 --- a/backend/test/test_collections.py +++ b/backend/test/test_collections.py @@ -1434,6 +1434,32 @@ def test_get_public_collection_slug_redirect(admin_auth_headers, default_org_id) assert coll["slug"] == new_slug assert coll["redirectToSlug"] == new_slug + # Rename second public collection slug to now-unused PUBLIC_COLLECTION_SLUG + r = requests.patch( + f"{API_PREFIX}/orgs/{default_org_id}/collections/{_second_public_coll_id}", + headers=admin_auth_headers, + json={ + "slug": PUBLIC_COLLECTION_SLUG, + }, + ) + assert r.status_code == 200 + assert r.json()["updated"] + + # Delete second public collection + r = requests.delete( + f"{API_PREFIX}/orgs/{default_org_id}/collections/{_second_public_coll_id}", + headers=admin_auth_headers, + ) + assert r.status_code == 200 + assert r.json()["success"] + + # Verify that trying to go to PUBLIC_COLLECTION_SLUG now 404s instead of taking + # us to the collection that had the slug before it was reassigned + r = requests.get( + f"{API_PREFIX}/public/orgs/{default_org_slug}/collections/{PUBLIC_COLLECTION_SLUG}" + ) + assert r.status_code == 404 + def test_delete_collection(crawler_auth_headers, default_org_id, crawler_crawl_id): # Delete second collection From bb0db23c944c303b90c2752c07d9d6ce5630be74 Mon Sep 17 00:00:00 2001 From: Ilya Kreymer Date: Wed, 15 Jan 2025 22:29:09 -0800 Subject: [PATCH 18/18] simplify: remove redirectToSlug from backend and frontend, frontend can determine if redirect is needed by comparing the slug in the API request to the 'slug' in the response, and redirect if they're different --- backend/btrixcloud/colls.py | 20 +++++++------------- backend/btrixcloud/models.py | 1 - backend/test/test_collections.py | 2 -- frontend/src/pages/collections/collection.ts | 6 +++--- 4 files changed, 10 insertions(+), 19 deletions(-) diff --git a/backend/btrixcloud/colls.py b/backend/btrixcloud/colls.py index 34b3d36713..4c1efe719c 100644 --- a/backend/btrixcloud/colls.py +++ b/backend/btrixcloud/colls.py @@ -311,13 +311,13 @@ async def get_collection( async def get_collection_by_slug( self, coll_slug: str, public_or_unlisted_only: bool = False - ) -> Tuple[Collection, bool]: - """Get collection by slug, as well as bool indicating if redirected from previous slug""" + ) -> Collection: + """Get collection by slug""" try: result = await self.get_collection_raw_by_slug( coll_slug, public_or_unlisted_only=public_or_unlisted_only ) - return Collection.from_dict(result), False + return Collection.from_dict(result) # pylint: disable=broad-exception-caught except Exception: pass @@ -327,7 +327,7 @@ async def get_collection_by_slug( previous_slugs=True, public_or_unlisted_only=public_or_unlisted_only, ) - return Collection.from_dict(result), True + return Collection.from_dict(result) async def get_collection_out( self, @@ -355,7 +355,6 @@ async def get_public_collection_out( self, coll_id: UUID, org: Organization, - redirect: bool = False, allow_unlisted: bool = False, ) -> PublicCollOut: """Get PublicCollOut by id""" @@ -377,9 +376,6 @@ async def get_public_collection_out( org, self.storage_ops ) - if redirect: - result["redirectToSlug"] = result.get("slug", "") - return PublicCollOut.from_dict(result) async def list_collections( @@ -1122,11 +1118,9 @@ async def get_public_collection( # pylint: disable=raise-missing-from raise HTTPException(status_code=404, detail="collection_not_found") - coll, redirect = await colls.get_collection_by_slug(coll_slug) + coll = await colls.get_collection_by_slug(coll_slug) - return await colls.get_public_collection_out( - coll.id, org, redirect=redirect, allow_unlisted=True - ) + return await colls.get_public_collection_out(coll.id, org, allow_unlisted=True) @app.get( "/public/orgs/{org_slug}/collections/{coll_slug}/download", @@ -1145,7 +1139,7 @@ async def download_public_collection( raise HTTPException(status_code=404, detail="collection_not_found") # Make sure collection exists and is public/unlisted - coll, _ = await colls.get_collection_by_slug( + coll = await colls.get_collection_by_slug( coll_slug, public_or_unlisted_only=True ) diff --git a/backend/btrixcloud/models.py b/backend/btrixcloud/models.py index 43695c1efb..02184b6fa3 100644 --- a/backend/btrixcloud/models.py +++ b/backend/btrixcloud/models.py @@ -1351,7 +1351,6 @@ class PublicCollOut(BaseMongoModel): defaultThumbnailName: Optional[str] = None allowPublicDownload: bool = True - redirectToSlug: Optional[str] = None # ============================================================================ diff --git a/backend/test/test_collections.py b/backend/test/test_collections.py index de0fa84faa..751b554ffe 100644 --- a/backend/test/test_collections.py +++ b/backend/test/test_collections.py @@ -1151,7 +1151,6 @@ def test_get_public_collection(default_org_id): assert coll["crawlCount"] > 0 assert coll["pageCount"] > 0 assert coll["totalSize"] > 0 - assert coll.get("redirectToSlug") is None for field in NON_PUBLIC_COLL_FIELDS: assert field not in coll @@ -1432,7 +1431,6 @@ def test_get_public_collection_slug_redirect(admin_auth_headers, default_org_id) assert coll["id"] == _public_coll_id assert coll["oid"] == default_org_id assert coll["slug"] == new_slug - assert coll["redirectToSlug"] == new_slug # Rename second public collection slug to now-unused PUBLIC_COLLECTION_SLUG r = requests.patch( diff --git a/frontend/src/pages/collections/collection.ts b/frontend/src/pages/collections/collection.ts index 10392940d3..b7d6b24314 100644 --- a/frontend/src/pages/collections/collection.ts +++ b/frontend/src/pages/collections/collection.ts @@ -65,9 +65,9 @@ export class Collection extends BtrixElement { collectionSlug, }); - if (collection.redirectToSlug) { + if (collection.slug !== collectionSlug) { this.navigate.to( - `/${RouteNamespace.PublicOrgs}/${this.orgSlug}/collections/${collection.redirectToSlug}`, + `/${RouteNamespace.PublicOrgs}/${this.orgSlug}/collections/${collection.slug}`, ); } @@ -291,7 +291,7 @@ export class Collection extends BtrixElement { }: { orgSlug: string; collectionSlug: string; - }): Promise { + }): Promise { const resp = await fetch( `/api/public/orgs/${orgSlug}/collections/${collectionSlug}`, {