diff --git a/frontend/src/metabase/collections/utils.js b/frontend/src/metabase/collections/utils.js index 8ccaa58c857ac08d560e7d419cc9c1d11d3ed734..7735f6d31c2d5af1375b6cc89a2cd9d65af70c2a 100644 --- a/frontend/src/metabase/collections/utils.js +++ b/frontend/src/metabase/collections/utils.js @@ -11,6 +11,7 @@ function preparePersonalCollection(c) { return { ...c, name: t`Your personal collection`, + originalName: c.name, }; } diff --git a/frontend/src/metabase/lib/urls.js b/frontend/src/metabase/lib/urls.js index af13ada31c558cac9e95e6b6e807cf4bbba08fc1..8a562280ffc01a1eddfd0a3de66d6b4c2e389106 100644 --- a/frontend/src/metabase/lib/urls.js +++ b/frontend/src/metabase/lib/urls.js @@ -124,18 +124,40 @@ export function tableRowsQuery(databaseId, tableId, metricId, segmentId) { return question(null, query); } +function slugifyPersonalCollection(collection) { + // Current user's personal collection name is replaced with "Your personal collection" + // `originalName` keeps the original name like "John Doe's Personal Collection" + const name = collection.originalName || collection.name; + + // Will keep single quote characters, + // so "John's" can be converted to `john-s` instead of `johns` + let slug = slugg(name, { + toStrip: /["â€â€œ]/g, + }); + + // If can't build a slug out of user's name (e.g. if it contains only emojis) + // removes the "s-" part of a slug + if (slug === "s-personal-collection") { + slug = slug.replace("s-", ""); + } + + return slug; +} + export function collection(collection) { - if ( - !collection || - collection.id === null || - typeof collection.id === "string" - ) { + const isSystemCollection = + !collection || collection.id === null || typeof collection.id === "string"; + + if (isSystemCollection) { const id = collection && collection.id ? collection.id : "root"; return `/collection/${id}`; } - const slug = collection.slug - ? collection.slug.split("_").join("-") + + const isPersonalCollection = typeof collection.personal_owner_id === "number"; + const slug = isPersonalCollection + ? slugifyPersonalCollection(collection) : slugg(collection.name); + return appendSlug(`/collection/${collection.id}`, slug); } diff --git a/frontend/test/metabase/lib/urls.unit.spec.js b/frontend/test/metabase/lib/urls.unit.spec.js index ecdd23a7c6a5a979b5cbd5905a8b1cb581a595be..c1e1fd797d4ecd202e3a742d25d295304a57fdd9 100644 --- a/frontend/test/metabase/lib/urls.unit.spec.js +++ b/frontend/test/metabase/lib/urls.unit.spec.js @@ -78,10 +78,6 @@ describe("urls", () => { }); it("returns correct url", () => { - expect(collection({ id: 1, slug: "first_collection" })).toBe( - "/collection/1-first-collection", - ); - expect(collection({ id: 1, name: "First collection" })).toBe( "/collection/1-first-collection", ); @@ -94,6 +90,37 @@ describe("urls", () => { }), ).toBe("/collection/1-first-collection"); }); + + it("handles possessives correctly", () => { + expect( + collection({ + id: 1, + name: "John Doe's Personal Collection", + personal_owner_id: 1, + }), + ).toBe("/collection/1-john-doe-s-personal-collection"); + }); + + it("omits possessive form if name can't be turned into a slug", () => { + expect( + collection({ + id: 1, + name: "ðŸŽ's Personal Collection", + personal_owner_id: 1, + }), + ).toBe("/collection/1-personal-collection"); + }); + + it("uses originalName to build a slug if present", () => { + expect( + collection({ + id: 1, + name: "Your personal collection", + originalName: "John Doe's Personal Collection", + personal_owner_id: 1, + }), + ).toBe("/collection/1-john-doe-s-personal-collection"); + }); }); describe("extractEntityId", () => { @@ -212,16 +239,28 @@ describe("urls", () => { return slug ? `${path}-${slug}` : path; } - it(`should handle ${caseName} correctly`, () => { + it(`should handle ${caseName} correctly for database browse URLs`, () => { expect(browseDatabase(entity)).toBe( expectedUrl("/browse/1", expectedString), ); - expect(collection(entity)).toBe( + }); + + it(`should handle ${caseName} correctly for collection URLs`, () => { + // collection objects have not transliterated slugs separated by underscores + // this makes sure they don't affect the slug builder + const collectionOwnSlug = entity.name.split(" ").join("_"); + expect(collection({ ...entity, slug: collectionOwnSlug })).toBe( expectedUrl("/collection/1", expectedString), ); + }); + + it(`should handle ${caseName} correctly for dashboard URLs`, () => { expect(dashboard(entity)).toBe( expectedUrl("/dashboard/1", expectedString), ); + }); + + it(`should handle ${caseName} correctly for question URLs`, () => { expect(question(entity)).toBe( expectedUrl("/question/1", expectedString), );