Skip to content
This repository has been archived by the owner on Jan 9, 2024. It is now read-only.

Commit

Permalink
Revert "feat(federation): resolve Item by itemId (#796)" (#797)
Browse files Browse the repository at this point in the history
This reverts commit 539c087.
  • Loading branch information
kschelonka authored May 23, 2023
1 parent 539c087 commit 9c79067
Show file tree
Hide file tree
Showing 15 changed files with 76 additions and 49 deletions.
3 changes: 2 additions & 1 deletion schema.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -614,8 +614,9 @@ type PendingItem @key(fields: "url") {
status: PendingItemStatus
}

type Item @key(fields: "itemId") {
type Item @key(fields: "givenUrl") {
"key field to identify the Item entity in the Parser service"
givenUrl: Url!
itemId: String! @shareable

#Note more properties exist here but are defined in another service.
Expand Down
1 change: 1 addition & 0 deletions src/businessEvents/itemsEventEmitter.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ describe('ItemsEventEmitter', () => {
status: 'UNREAD',
isFavorite: true,
isArchived: false,
item: { givenUrl: 'https://www.youtube.com/watch?v=dQw4w9WgXcQ' },
};

const payload: BasicItemEventPayloadWithContext = {
Expand Down
3 changes: 3 additions & 0 deletions src/businessEvents/unifiedEventKinesisHandler.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ describe('UnifiedEventHandler', () => {
isFavorite: true,
isArchived: false,
status: 'UNREAD',
item: {
givenUrl: 'https://www.youtube.com/watch?v=dQw4w9WgXcQ',
},
};

it('should log an error if there are failed messages after retrying', async () => {
Expand Down
6 changes: 6 additions & 0 deletions src/dataLoader/savedItemsDataLoader.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ describe('savedItem data loader', function () {
isFavorite: false,
isArchived: false,
status: 'UNREAD',
item: {
givenUrl: 'abc.com',
},
},
{
id: '2',
Expand All @@ -24,6 +27,9 @@ describe('savedItem data loader', function () {
isFavorite: false,
isArchived: false,
status: 'DELETED',
item: {
givenUrl: 'def.com',
},
},
];

Expand Down
4 changes: 2 additions & 2 deletions src/resolvers/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,9 @@ const resolvers = {
},
Item: {
savedItem: async (item: Item, args, context: IContext) => {
return await context.dataLoaders.savedItemsById.load(item.itemId);
return await context.dataLoaders.savedItemsByUrl.load(item.givenUrl);
},
// This is basically a passthrough so that the itemId is available
// This is basically a passthrough so that the givenUrl is available
// on the parent when the savedItem entity is resolved
// Possible to resolve savedItem on this reference resolver instead,
// but this maintains our pattern of separation of entity resolvers
Expand Down
3 changes: 3 additions & 0 deletions src/server/context.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ describe('context', () => {
isFavorite: false,
status: 'UNREAD',
isArchived: false,
item: {
givenUrl: 'dont-care.com',
},
};
describe('constructor', () => {
const sentryScopeSpy = sinon.spy(Sentry, 'configureScope');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ describe('UpsertSavedItem Mutation', () => {
_version
item {
... on Item {
itemId
givenUrl
}
}
tags {
Expand All @@ -188,7 +188,7 @@ describe('UpsertSavedItem Mutation', () => {
expect(data.isArchived).is.false;
expect(data._deletedAt).is.null;
expect(data._version).is.null;
expect(data.item.itemId).equals('8');
expect(data.item.givenUrl).equals(variables.url);
expect(data.tags[0].name).equals('zebra');
expect(data.archivedAt).is.null;
expect(data.favoritedAt).is.null;
Expand Down Expand Up @@ -236,7 +236,7 @@ describe('UpsertSavedItem Mutation', () => {
item {
... on Item {
__typename
itemId
givenUrl
}
... on PendingItem {
__typename
Expand All @@ -255,6 +255,7 @@ describe('UpsertSavedItem Mutation', () => {
expect(mutationResult).is.not.null;
const data = mutationResult.body.data?.upsertSavedItem;
expect(data.id).to.equal('1');
expect(data.item.givenUrl).is.undefined;
expect(data.item.url).to.equal(givenUrl);
expect(data.item.itemId).to.equal('1');
expect(data.item.__typename).to.equal('PendingItem');
Expand Down
30 changes: 16 additions & 14 deletions src/test/graphql/queries/item.integration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ describe('item', () => {

const itemFragment = `
fragment ItemFields on Item {
itemId
givenUrl
savedItem {
id
url
Expand All @@ -22,8 +22,8 @@ describe('item', () => {
`;
const GET_SAVED_ITEM = `
${itemFragment}
query getSaveFromItem($itemId: String!) {
_entities(representations: { itemId: $itemId, __typename: "Item" }) {
query getSaveFromItem($givenUrl: String!) {
_entities(representations: { givenUrl: $givenUrl, __typename: "Item" }) {
... on Item {
...ItemFields
}
Expand All @@ -34,11 +34,11 @@ describe('item', () => {
// than to just duplicate it
const GET_TWO_SAVED_ITEMS = `
${itemFragment}
query getSavesFromItems($itemId1: String!, $itemId2: String!) {
query getSavesFromItems($givenUrl1: String!, $givenUrl2: String!) {
_entities(
representations: [
{ itemId: $itemId1, __typename: "Item" }
{ itemId: $itemId2, __typename: "Item" }
{ givenUrl: $givenUrl1, __typename: "Item" }
{ givenUrl: $givenUrl2, __typename: "Item" }
]
) {
... on Item {
Expand Down Expand Up @@ -113,7 +113,7 @@ describe('item', () => {
it('resolves more than one savedItem from multiple entities', async () => {
const expected = [
{
itemId: '1',
givenUrl: 'https://www.youtube.com/watch?v=aWJ_7akYFhg',
savedItem: {
url: 'https://www.youtube.com/watch?v=aWJ_7akYFhg',
isFavorite: true,
Expand All @@ -122,7 +122,7 @@ describe('item', () => {
},
},
{
itemId: '999',
givenUrl: 'https://www.youtube.com/watch?v=OZaL86RDGIU',
savedItem: {
url: 'https://www.youtube.com/watch?v=OZaL86RDGIU',
isFavorite: false,
Expand All @@ -133,8 +133,8 @@ describe('item', () => {
];
const variables = {
userId: '1',
itemId1: '1',
itemId2: '999',
givenUrl1: 'https://www.youtube.com/watch?v=aWJ_7akYFhg',
givenUrl2: 'https://www.youtube.com/watch?v=OZaL86RDGIU',
};

const res = await request(app).post(url).set(headers).send({
Expand All @@ -149,7 +149,7 @@ describe('item', () => {
});
it('resolves savedItem field from entity representation', async () => {
const expected = {
itemId: '1',
givenUrl: 'https://www.youtube.com/watch?v=aWJ_7akYFhg',
savedItem: {
url: 'https://www.youtube.com/watch?v=aWJ_7akYFhg',
isFavorite: true,
Expand All @@ -159,7 +159,7 @@ describe('item', () => {
};
const variables = {
userId: '1',
itemId: '1',
givenUrl: 'https://www.youtube.com/watch?v=aWJ_7akYFhg',
};

const res = await request(app).post(url).set(headers).send({
Expand All @@ -176,7 +176,7 @@ describe('item', () => {
it('returns null if the save does not exist', async () => {
const variables = {
userId: '1',
itemId: '2',
givenUrl: 'https://www.youtube.com/watch?v=Tpbo25iBvfU',
};

const res = await request(app).post(url).set(headers).send({
Expand All @@ -187,7 +187,9 @@ describe('item', () => {
expect(res.body.errors).toBeUndefined;
const entities = res.body.data._entities;
expect(entities.length).toEqual(1);
expect(entities[0].itemId).toEqual('2');
expect(entities[0].givenUrl).toEqual(
'https://www.youtube.com/watch?v=Tpbo25iBvfU'
);
expect(entities[0].savedItem).toBeNull;
});
});
2 changes: 2 additions & 0 deletions src/test/graphql/queries/pocketSave-item.integration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ describe('PocketSave.Item', () => {
}
... on Item {
itemId
givenUrl
__typename
}
}
Expand Down Expand Up @@ -117,6 +118,7 @@ describe('PocketSave.Item', () => {
});
const expected = {
itemId: '55',
givenUrl: 'https://www.youtube.com/watch?v=nsNMP6_Q0Js',
__typename: 'Item',
};
expect(res.body.errors).toBeUndefined();
Expand Down
8 changes: 5 additions & 3 deletions src/test/graphql/queries/savedItem.integration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ describe('getSavedItemByItemId', () => {
});
expect(res.body.data?._entities[0].savedItemById).to.be.null;
});
it('should resolve item ID', async () => {
it('should resolve item url', async () => {
const variables = {
userId: '1',
itemId: '1',
Expand All @@ -153,7 +153,7 @@ describe('getSavedItemByItemId', () => {
favoritedAt
item {
... on Item {
itemId
givenUrl
}
}
}
Expand All @@ -165,7 +165,9 @@ describe('getSavedItemByItemId', () => {
query: GET_SAVED_ITEM_ITEM,
variables,
});
expect(res.body.data?._entities[0].savedItemById.item.itemId).to.equal('1');
expect(res.body.data?._entities[0].savedItemById.item.givenUrl).to.equal(
'http://abc'
);
});

it('should have _deletedAt field if item is deleted', async () => {
Expand Down
44 changes: 22 additions & 22 deletions src/test/graphql/queries/savedItems.integration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ describe('getSavedItems', () => {
url
item {
... on Item {
itemId
givenUrl
}
}
}
Expand Down Expand Up @@ -165,14 +165,14 @@ describe('getSavedItems', () => {
'http://ijk'
);
expect(
res.body.data?._entities[0].savedItems.edges[0].node.item.itemId
).to.equal('3');
res.body.data?._entities[0].savedItems.edges[0].node.item.givenUrl
).to.equal('http://ijk');
expect(res.body.data?._entities[0].savedItems.edges[1].node.url).to.equal(
'http://def'
);
expect(
res.body.data?._entities[0].savedItems.edges[1].node.item.itemId
).to.equal('2');
res.body.data?._entities[0].savedItems.edges[1].node.item.givenUrl
).to.equal('http://def');
expect(res.body.data?._entities[0].savedItems.pageInfo.hasNextPage).to.be
.true;
expect(res.body.data?._entities[0].savedItems.pageInfo.hasPreviousPage).to
Expand All @@ -195,8 +195,8 @@ describe('getSavedItems', () => {
'http://abc'
);
expect(
res.body.data?._entities[0].savedItems.edges[0].node.item.itemId
).to.equal('1');
res.body.data?._entities[0].savedItems.edges[0].node.item.givenUrl
).to.equal('http://abc');
expect(res.body.data?._entities[0].savedItems.pageInfo.hasNextPage).to.be
.false;
expect(res.body.data?._entities[0].savedItems.pageInfo.hasPreviousPage).to
Expand Down Expand Up @@ -248,8 +248,8 @@ describe('getSavedItems', () => {
});
expect(res.body.data?._entities[0].savedItems.edges.length).to.equal(1);
expect(
res.body.data?._entities[0].savedItems.edges[0].node.item.itemId
).to.equal('3');
res.body.data?._entities[0].savedItems.edges[0].node.item.givenUrl
).to.equal('http://ijk');
expect(res.body.data?._entities[0].savedItems.pageInfo.hasPreviousPage).to
.be.false;
expect(res.body.data?._entities[0].savedItems.pageInfo.hasNextPage).to.be
Expand Down Expand Up @@ -406,7 +406,7 @@ describe('getSavedItems', () => {
url
item {
... on Item {
itemId
givenUrl
}
}
}
Expand Down Expand Up @@ -466,47 +466,47 @@ describe('getSavedItems', () => {
{
sortBy: 'CREATED_AT',
sortOrder: 'DESC',
expectedIds: ['3', '2'],
expectedUrls: ['http://ijk', 'http://def'],
},
{
sortBy: 'CREATED_AT',
sortOrder: 'ASC',
expectedIds: ['1', '2'],
expectedUrls: ['http://abc', 'http://def'],
},
{
sortBy: 'UPDATED_AT',
sortOrder: 'DESC',
expectedIds: ['2', '1'],
expectedUrls: ['http://def', 'http://abc'],
},
{
sortBy: 'UPDATED_AT',
sortOrder: 'ASC',
expectedIds: ['3', '1'],
expectedUrls: ['http://ijk', 'http://abc'],
},
{
sortBy: 'FAVORITED_AT',
sortOrder: 'DESC',
expectedIds: ['2', '3'],
expectedUrls: ['http://def', 'http://ijk'],
},
{
sortBy: 'FAVORITED_AT',
// Note that this will put non-favorite items first (since they are set to time 0)
sortOrder: 'ASC',
expectedIds: ['1', '3'],
expectedUrls: ['http://abc', 'http://ijk'],
},
{
sortBy: 'ARCHIVED_AT',
sortOrder: 'DESC',
expectedIds: ['3', '2'],
expectedUrls: ['http://ijk', 'http://def'],
},
{
sortBy: 'ARCHIVED_AT',
sortOrder: 'ASC',
expectedIds: ['1', '2'],
expectedUrls: ['http://abc', 'http://def'],
},
])(
' by $sortBy, $sortOrder works',
async ({ sortBy, sortOrder, expectedIds }) => {
async ({ sortBy, sortOrder, expectedUrls }) => {
const variables = {
id: '1',
pagination: { first: 2 },
Expand All @@ -517,10 +517,10 @@ describe('getSavedItems', () => {
variables,
});
expect(res.body.errors).to.be.undefined;
const ids = res.body.data?._entities[0].savedItems.edges.map(
(edge) => edge.node.item.itemId
const urls = res.body.data?._entities[0].savedItems.edges.map(
(edge) => edge.node.item.givenUrl
);
expect(expectedIds).to.deep.equal(ids);
expect(expectedUrls).to.deep.equal(urls);
}
);
});
Expand Down
Loading

0 comments on commit 9c79067

Please sign in to comment.