🐛 Fixed keeping existing attribution in recommendations (#19945)

ref https://linear.app/tryghost/issue/ENG-799

- recommendations were being stripped of query parameters and hash
fragments before save
- in particular, query parameters for attribution such as ?ref were not
being stored
This commit is contained in:
Sag 2024-03-28 13:54:23 +01:00 committed by GitHub
parent 86911be7db
commit 7e2d842db2
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 64 additions and 18 deletions

View File

@ -6,7 +6,6 @@ import {EditOrAddRecommendation, useCheckRecommendation} from '@tryghost/admin-x
import {ErrorMessages, useForm} from '@tryghost/admin-x-framework/hooks';
import {Form, LoadingIndicator, Modal, TextField, dismissAllToasts, formatUrl, showToast} from '@tryghost/admin-x-design-system';
import {RoutingModalProps, useRouting} from '@tryghost/admin-x-framework/routing';
import {trimSearchAndHash} from '../../../../utils/url';
interface AddRecommendationModalProps {
recommendation?: EditOrAddRecommendation,
@ -60,7 +59,6 @@ const AddRecommendationModal: React.FC<RoutingModalProps & AddRecommendationModa
onSave: async () => {
let validatedUrl: URL;
validatedUrl = new URL(formState.url);
validatedUrl = trimSearchAndHash(validatedUrl);
// Use the hostname as fallback title
const defaultTitle = validatedUrl.hostname.replace('www.', '');

View File

@ -109,7 +109,7 @@ export class BookshelfRecommendationRepository extends BookshelfRepository<strin
// Query params, hash fragements, protocol and www are ignored.
const existing = recommendations.find((r) => {
return r.url.hostname.replace('www.', '') === url.hostname.replace('www.', '') &&
r.url.pathname === url.pathname;
r.url.pathname.replace(/\/$/, '') === url.pathname.replace(/\/$/, '');
}) || null;
return existing;

View File

@ -12,7 +12,7 @@ export class InMemoryRecommendationRepository extends InMemoryRepository<string,
// Query params, hash fragements, protocol and www are ignored.
const existing = this.store.find((r) => {
return r.url.hostname.replace('www.', '') === url.hostname.replace('www.', '') &&
r.url.pathname === url.pathname;
r.url.pathname.replace(/\/$/, '') === url.pathname.replace(/\/$/, '');
}) || null;
return existing;

View File

@ -121,18 +121,10 @@ export class Recommendation {
this.description = null;
}
this.url = this.cleanURL(this.url);
this.createdAt.setMilliseconds(0);
this.updatedAt?.setMilliseconds(0);
}
cleanURL(url: URL) {
url.search = '';
url.hash = '';
return url;
};
static create(data: RecommendationCreateData) {
const id = data.id ?? ObjectId().toString();

View File

@ -114,7 +114,35 @@ describe('BookshelfRecommendationRepository', function () {
sinon.assert.calledOnce(stub);
});
it('getByUrl returns if matching hostname', async function () {
it('getByUrl returns null if not matching path', async function () {
const repository = new BookshelfRecommendationRepository({} as any, {
sentry: undefined
});
const recommendation = Recommendation.create({
id: 'id',
title: 'title',
description: 'description',
excerpt: 'excerpt',
featuredImage: new URL('https://example.com'),
favicon: new URL('https://example.com'),
url: new URL('https://example.com/other-path'),
oneClickSubscribe: true,
createdAt: new Date('2021-01-01'),
updatedAt: new Date('2021-01-02')
});
const stub = sinon.stub(repository, 'getAll').returns(Promise.resolve([
recommendation
]));
const entity = await repository.getByUrl(new URL('https://www.example.com/path'));
assert.equal(
entity,
null
);
sinon.assert.calledOnce(stub);
});
it('getByUrl returns if matching hostname and pathname', async function () {
const repository = new BookshelfRecommendationRepository({} as any, {
sentry: undefined
});
@ -142,7 +170,7 @@ describe('BookshelfRecommendationRepository', function () {
sinon.assert.calledOnce(stub);
});
it('getByUrl returns null if not matching path', async function () {
it('getByUrl returns if matching hostname and pathname, but not query params', async function () {
const repository = new BookshelfRecommendationRepository({} as any, {
sentry: undefined
});
@ -153,7 +181,35 @@ describe('BookshelfRecommendationRepository', function () {
excerpt: 'excerpt',
featuredImage: new URL('https://example.com'),
favicon: new URL('https://example.com'),
url: new URL('https://example.com/other-path'),
url: new URL('https://example.com/path'),
oneClickSubscribe: true,
createdAt: new Date('2021-01-01'),
updatedAt: new Date('2021-01-02')
});
const stub = sinon.stub(repository, 'getAll').returns(Promise.resolve([
recommendation
]));
const entity = await repository.getByUrl(new URL('https://www.example.com/path/?query=param'));
assert.equal(
entity,
recommendation
);
sinon.assert.calledOnce(stub);
});
it('getByUrl returns if matching hostname and pathname, but not hash fragments', async function () {
const repository = new BookshelfRecommendationRepository({} as any, {
sentry: undefined
});
const recommendation = Recommendation.create({
id: 'id',
title: 'title',
description: 'description',
excerpt: 'excerpt',
featuredImage: new URL('https://example.com'),
favicon: new URL('https://example.com'),
url: new URL('https://example.com/path/#section1'),
oneClickSubscribe: true,
createdAt: new Date('2021-01-01'),
updatedAt: new Date('2021-01-02')
@ -165,7 +221,7 @@ describe('BookshelfRecommendationRepository', function () {
assert.equal(
entity,
null
recommendation
);
sinon.assert.calledOnce(stub);
});

View File

@ -118,7 +118,7 @@ describe('Recommendation', function () {
assert.equal(recommendation.description, null);
});
it('removes search and hash params', function () {
it('keeps search and hash params', function () {
const recommendation = Recommendation.create({
title: 'Test',
description: '',
@ -130,7 +130,7 @@ describe('Recommendation', function () {
updatedAt: new Date('2021-01-01T00:00:05Z')
});
assert.equal(recommendation.url.toString(), 'https://example.com/');
assert.equal(recommendation.url.toString(), 'https://example.com/?query=1#hash');
});
});