Skip to content

Commit

Permalink
remove human-readable automatic slug generation (elastic#132593)
Browse files Browse the repository at this point in the history
* remove human-readable automatic slug generation

* make change non-breaking

* [CI] Auto-commit changed files from 'node scripts/eslint --no-cache --fix'

* remove test

Co-authored-by: streamich <streamich@gmail.com>
Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
3 people committed May 20, 2022
1 parent 6fc2fff commit e857b30
Show file tree
Hide file tree
Showing 8 changed files with 11 additions and 58 deletions.
13 changes: 0 additions & 13 deletions src/plugins/share/README.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -215,19 +215,6 @@ const url = await shortUrls.create({
});
```

You can make the short URL slug human-readable by specifying the
`humanReadableSlug` flag:

```ts
const url = await shortUrls.create({
locator,
params: {
dashboardId: '123',
},
humanReadableSlug: true,
});
```

Or you can manually specify the slug for the short URL using the `slug` option:

```ts
Expand Down
6 changes: 0 additions & 6 deletions src/plugins/share/common/url_service/short_urls/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,12 +79,6 @@ export interface ShortUrlCreateParams<P extends SerializableRecord> {
* URL. This part will be visible to the user, it can have user-friendly text.
*/
slug?: string;

/**
* Whether to generate a slug automatically. If `true`, the slug will be
* a human-readable text consisting of three worlds: "<adjective>-<adjective>-<noun>".
*/
humanReadableSlug?: boolean;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@ describe('create()', () => {
body: expect.any(String),
});
expect(JSON.parse(fetchSpy.mock.calls[0][1].body)).toStrictEqual({
humanReadableSlug: false,
locatorId: LEGACY_SHORT_URL_LOCATOR_ID,
params: {
url: 'https://example.com/foo/bar',
Expand Down Expand Up @@ -173,7 +172,6 @@ describe('createFromLongUrl()', () => {
body: expect.any(String),
});
expect(JSON.parse(fetchSpy.mock.calls[0][1].body)).toStrictEqual({
humanReadableSlug: true,
locatorId: LEGACY_SHORT_URL_LOCATOR_ID,
params: {
url: '/a/b/c',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,15 +59,13 @@ export class BrowserShortUrlClient implements IShortUrlClient {
locator,
params,
slug = undefined,
humanReadableSlug = false,
}: ShortUrlCreateParams<P>): Promise<ShortUrl<P>> {
const { http } = this.dependencies;
const data = await http.fetch<ShortUrlData<P>>('/api/short_url', {
method: 'POST',
body: JSON.stringify({
locatorId: locator.id,
slug,
humanReadableSlug,
params,
}),
});
Expand Down Expand Up @@ -113,7 +111,6 @@ export class BrowserShortUrlClient implements IShortUrlClient {

const result = await this.createWithLocator({
locator,
humanReadableSlug: true,
params: {
url: relativeUrl,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,15 @@ export const registerCreateRoute = (router: IRouter, url: ServerUrlService) => {
minLength: 3,
maxLength: 255,
}),
/**
* @deprecated
*
* This field is deprecated as the API does not support automatic
* human-readable slug generation.
*
* @todo This field will be removed in a future version. It is left
* here for backwards compatibility.
*/
humanReadableSlug: schema.boolean({
defaultValue: false,
}),
Expand All @@ -36,7 +45,7 @@ export const registerCreateRoute = (router: IRouter, url: ServerUrlService) => {
router.handleLegacyErrors(async (ctx, req, res) => {
const savedObjects = (await ctx.core).savedObjects.client;
const shortUrls = url.shortUrls.get({ savedObjects });
const { locatorId, params, slug, humanReadableSlug } = req.body;
const { locatorId, params, slug } = req.body;
const locator = url.locators.get(locatorId);

if (!locator) {
Expand All @@ -51,7 +60,6 @@ export const registerCreateRoute = (router: IRouter, url: ServerUrlService) => {
locator,
params,
slug,
humanReadableSlug,
});

return res.ok({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,19 +128,6 @@ describe('ServerShortUrlClient', () => {
})
).rejects.toThrowError(new UrlServiceError(`Slug "lala" already exists.`, 'SLUG_EXISTS'));
});

test('can automatically generate human-readable slug', async () => {
const { client, locator } = setup();
const shortUrl = await client.create({
locator,
humanReadableSlug: true,
params: {
url: '/app/test#foo/bar/baz',
},
});

expect(shortUrl.data.slug.split('-').length).toBe(3);
});
});

describe('.get()', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@

import type { SerializableRecord } from '@kbn/utility-types';
import { SavedObjectReference } from '@kbn/core/server';
import { generateSlug } from 'random-word-slugs';
import { ShortUrlRecord } from '.';
import type {
IShortUrlClient,
Expand Down Expand Up @@ -60,14 +59,13 @@ export class ServerShortUrlClient implements IShortUrlClient {
locator,
params,
slug = '',
humanReadableSlug = false,
}: ShortUrlCreateParams<P>): Promise<ShortUrl<P>> {
if (slug) {
validateSlug(slug);
}

if (!slug) {
slug = humanReadableSlug ? generateSlug() : randomStr(4);
slug = randomStr(5);
}

const { storage, currentVersion } = this.dependencies;
Expand Down
16 changes: 0 additions & 16 deletions test/api_integration/apis/short_url/create_short_url/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,22 +70,6 @@ export default function ({ getService }: FtrProviderContext) {
expect(response.body.url).to.be('');
});

it('can generate a human-readable slug, composed of three words', async () => {
const response = await supertest.post('/api/short_url').send({
locatorId: 'LEGACY_SHORT_URL_LOCATOR',
params: {},
humanReadableSlug: true,
});

expect(response.status).to.be(200);
expect(typeof response.body.slug).to.be('string');
const words = response.body.slug.split('-');
expect(words.length).to.be(3);
for (const word of words) {
expect(word.length > 0).to.be(true);
}
});

it('can create a short URL with custom slug', async () => {
const rnd = Math.round(Math.random() * 1e6) + 1;
const slug = 'test-slug-' + Date.now() + '-' + rnd;
Expand Down

0 comments on commit e857b30

Please sign in to comment.