From 4583babecbabb2a9f74fe98509cb0546872c1b72 Mon Sep 17 00:00:00 2001 From: Tessa Walsh Date: Thu, 16 Jan 2025 01:44:32 -0500 Subject: [PATCH] feat: Add slug to collections and use it in public collection URLs (#2301) Resolves https://github.com/webrecorder/browsertrix/issues/2298 ## Changes - Slugs added to collections, can be specified separately when creating or updating collections or else is based off of supplied collection name - Migration added to backfill slugs for existing collections - Redirect collection to newest slug if changed - Adds option to copy public profile link to "Public Collections" action menu - Show "Back to " link instead of breadcrumbs --------- Co-authored-by: sua yoo Co-authored-by: Ilya Kreymer --- backend/btrixcloud/colls.py | 129 +++++++++++++++--- backend/btrixcloud/db.py | 2 +- .../migrations/migration_0039_coll_slugs.py | 38 ++++++ backend/btrixcloud/models.py | 10 ++ backend/btrixcloud/orgs.py | 2 + backend/btrixcloud/utils.py | 13 +- backend/test/test_collections.py | 101 ++++++++++++-- frontend/docs/docs/deploy/customization.md | 3 +- frontend/src/classes/BtrixElement.ts | 2 +- .../features/collections/collections-grid.ts | 4 +- .../features/collections/share-collection.ts | 46 ++++--- .../features/crawl-workflows/workflow-list.ts | 2 +- frontend/src/index.ts | 28 ++-- frontend/src/pages/collections/collection.ts | 89 ++++++------ frontend/src/pages/log-in.ts | 5 +- .../pages/org/archived-item-detail/ui/qa.ts | 2 +- frontend/src/pages/org/collection-detail.ts | 1 + frontend/src/pages/org/collections-list.ts | 2 +- frontend/src/pages/org/dashboard.ts | 51 +++++-- frontend/src/pages/org/index.ts | 8 +- frontend/src/pages/org/profile.ts | 12 +- .../pages/org/settings/components/profile.ts | 4 +- frontend/src/pages/org/settings/settings.ts | 4 +- frontend/src/routes.ts | 2 +- frontend/src/types/collection.ts | 1 + frontend/src/utils/LiteElement.ts | 2 +- frontend/src/utils/analytics.ts | 3 +- 27 files changed, 414 insertions(+), 152 deletions(-) create mode 100644 backend/btrixcloud/migrations/migration_0039_coll_slugs.py diff --git a/backend/btrixcloud/colls.py b/backend/btrixcloud/colls.py index f6c13dad59..4c1efe719c 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( @@ -111,16 +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=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, @@ -128,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) @@ -139,9 +153,10 @@ 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 - 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") async def update_collection( self, coll_id: UUID, org: Organization, update: UpdateColl @@ -152,23 +167,55 @@ 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 + slug_update = 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") + 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: @@ -234,6 +281,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: @@ -241,6 +309,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 + ) -> 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) + # 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) + async def get_collection_out( self, coll_id: UUID, @@ -264,7 +352,10 @@ 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, + allow_unlisted: bool = False, ) -> PublicCollOut: """Get PublicCollOut by id""" result = await self.get_collection_raw(coll_id) @@ -1012,13 +1103,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) @@ -1027,16 +1118,18 @@ 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 = await colls.get_collection_by_slug(coll_slug) + + return await colls.get_public_collection_out(coll.id, org, 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) @@ -1046,12 +1139,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(coll.id, org) @app.get( "/orgs/{oid}/collections/{coll_id}/urls", 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_0039_coll_slugs.py b/backend/btrixcloud/migrations/migration_0039_coll_slugs.py new file mode 100644 index 0000000000..6fa4cd01b0 --- /dev/null +++ b/backend/btrixcloud/migrations/migration_0039_coll_slugs.py @@ -0,0 +1,38 @@ +""" +Migration 0039 -- collection slugs +""" + +from btrixcloud.migrations import BaseMigration +from btrixcloud.utils import slug_from_name + + +MIGRATION_VERSION = "0039" + + +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..02184b6fa3 100644 --- a/backend/btrixcloud/models.py +++ b/backend/btrixcloud/models.py @@ -1235,7 +1235,9 @@ 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 description: Optional[str] = None caption: Optional[str] = None @@ -1264,12 +1266,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]] = [] @@ -1284,7 +1289,9 @@ class CollIn(BaseModel): class CollOut(BaseMongoModel): """Collection output model with annotations.""" + id: UUID name: str + slug: str oid: UUID description: Optional[str] = None caption: Optional[str] = None @@ -1318,7 +1325,9 @@ class CollOut(BaseMongoModel): class PublicCollOut(BaseMongoModel): """Collection output model with annotations.""" + id: UUID name: str + slug: str oid: UUID description: Optional[str] = None caption: Optional[str] = None @@ -1349,6 +1358,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/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( diff --git a/backend/btrixcloud/utils.py b/backend/btrixcloud/utils.py index 468c892421..8a555a4c7c 100644 --- a/backend/btrixcloud/utils.py +++ b/backend/btrixcloud/utils.py @@ -171,15 +171,16 @@ 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" + allowed_fields = ("name", "slug", "subscription.subId") + 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 allowed_fields: + return field + + return "name" def get_origin(headers) -> str: diff --git a/backend/test/test_collections.py b/backend/test/test_collections.py index 7b274bf8fe..751b554ffe 100644 --- a/backend/test/test_collections.py +++ b/backend/test/test_collections.py @@ -9,12 +9,16 @@ 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" UPDATED_CAPTION = "Updated caption" +SECOND_PUBLIC_COLL_SLUG = "second-public-collection" NON_PUBLIC_COLL_FIELDS = ( "tags", @@ -74,6 +78,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 +103,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 +137,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 +213,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 +244,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 +340,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 +363,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 +542,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 +559,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 @@ -811,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", }, @@ -888,6 +909,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 @@ -1111,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() @@ -1122,6 +1144,7 @@ def test_get_public_collection(default_org_id): assert coll["name"] assert coll["created"] assert coll["modified"] + assert coll["slug"] == PUBLIC_COLLECTION_SLUG assert coll["resources"] assert coll["dateEarliest"] assert coll["dateLatest"] @@ -1154,22 +1177,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/{UPDATED_SLUG}" ) assert r.status_code == 404 assert r.json()["detail"] == "collection_not_found" @@ -1189,7 +1212,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() @@ -1200,6 +1223,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"] == SECOND_PUBLIC_COLL_SLUG assert coll["resources"] assert coll["dateEarliest"] assert coll["dateLatest"] @@ -1229,7 +1253,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() @@ -1240,6 +1264,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"] @@ -1314,7 +1339,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 @@ -1332,7 +1357,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 @@ -1365,7 +1390,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 @@ -1382,6 +1407,58 @@ 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 + + # 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 r = requests.delete( 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/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..740a803680 100644 --- a/frontend/src/features/collections/collections-grid.ts +++ b/frontend/src/features/collections/collections-grid.ts @@ -61,7 +61,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..e1d5712608 100644 --- a/frontend/src/features/collections/share-collection.ts +++ b/frontend/src/features/collections/share-collection.ts @@ -40,7 +40,7 @@ enum Tab { @customElement("btrix-share-collection") export class ShareCollection extends BtrixElement { @property({ type: String }) - slug = ""; + orgSlug = ""; @property({ type: String }) collectionId = ""; @@ -59,7 +59,11 @@ 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/${this.collectionId}` + : `${RouteNamespace.PublicOrgs}/${this.orgSlug}/collections/${this.collection.slug}` + }`; } return ""; } @@ -116,12 +120,13 @@ export class ShareCollection extends BtrixElement { @click=${() => { void this.clipboardController.copy(this.shareLink); - track(AnalyticsTrackEvent.CopyShareCollectionLink, { - org_slug: this.slug, - 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, + }); + } }} > + ${when(this.orgSlug && this.collection, (collection) => collection.access === CollectionAccess.Public && collection.allowPublicDownload ? html` { track(AnalyticsTrackEvent.DownloadPublicCollection, { - org_slug: this.slug, - collection_id: this.collectionId, - collection_name: this.collection?.name, + org_slug: this.orgSlug, + collection_slug: this.collection?.slug, }); }} > @@ -252,7 +256,11 @@ export class ShareCollection extends BtrixElement {
    - ${when(showSettings && this.collection, this.renderSettings)} + ${when( + showSettings && this.collection, + this.renderSettings, + this.renderShareLink, + )}
    @@ -282,10 +290,6 @@ export class ShareCollection extends BtrixElement { ); }} > - ${when( - this.collection?.access != CollectionAccess.Private, - this.renderShareLink, - )} ${when( this.org && !this.org.enablePublicProfile && @@ -299,6 +303,10 @@ export class ShareCollection extends BtrixElement { `, )} + ${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")}
    org.slug === slug)); } @@ -306,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); @@ -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; @@ -816,20 +820,20 @@ export class App extends BtrixElement { case "publicOrgProfile": return html``; 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 1b54f6467a..b7d6b24314 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"; @@ -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.orgSlug && this.appState.isCrawler; + return this.orgSlug === this.orgSlugState && this.appState.isCrawler; } private readonly tabLabels: Record< @@ -48,19 +48,28 @@ 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.slug !== collectionSlug) { + this.navigate.to( + `/${RouteNamespace.PublicOrgs}/${this.orgSlug}/collections/${collection.slug}`, + ); + } if (!collection.crawlCount && (this.tab as unknown) === Tab.Replay) { this.tab = Tab.About; @@ -68,7 +77,7 @@ export class Collection extends BtrixElement { return collection; }, - args: () => [this.slug, this.collectionId] as const, + args: () => [this.orgSlug, this.collectionSlug] as const, }); render() { @@ -81,21 +90,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.slug}`, - content: org.name, - }, - { - href: `/${RouteNamespace.PublicOrgs}/${this.slug}`, - 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( @@ -103,8 +108,8 @@ export class Collection extends BtrixElement { () => html` `, @@ -170,7 +175,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 +286,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/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/collection-detail.ts b/frontend/src/pages/org/collection-detail.ts index afa13146a3..c9ce01e8a9 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` { diff --git a/frontend/src/pages/org/collections-list.ts b/frontend/src/pages/org/collections-list.ts index 546a749e53..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.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.slug}`; } private get hasSearchStr() { diff --git a/frontend/src/pages/org/dashboard.ts b/frontend/src/pages/org/dashboard.ts index 1978ef25f1..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"; @@ -62,7 +63,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,24 +283,44 @@ export class Dashboard extends BtrixElement { ${msg("Manage Collections")} ${this.org?.enablePublicProfile ? 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")} + + `, + )} `, @@ -307,7 +328,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..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.orgSlug && 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/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)}