Mirror of https://github.com/roostorg/coop github.com/roostorg/coop
0
fork

Configure Feed

Select the types of activity you want to include in your feed.

[Bug Fix] Surface Thread-kind items in user submission history (#284)

* [fix] Surface Thread-kind items in user submission history
Thread items submitted with a schema field role were
silently being dropped from creator-keyed surfaces

* [test] Add unit tests for getCreator across item kinds

Pins the behavior change from the previous commit so the THREAD case
can't silently regress to `undefined` again. Covers:

- CONTENT: returns the creator from the creatorId field role; returns
undefined when the role is unconfigured or the field is missing.
- THREAD: same as CONTENT (this is the regression case the bug fix
targets).
- USER: always returns undefined.

Exports `getCreator` from `makeItemSubmission.ts` for testability;
nothing else in the codebase imports it.

authored by

Juan Mrad and committed by
GitHub
725bad99 d9257a07

+260 -26
+53 -1
client/src/graphql/generated.ts
··· 16689 16689 }; 16690 16690 }; 16691 16691 } 16692 - | { readonly __typename: 'ThreadItem' } 16692 + | { 16693 + readonly __typename: 'ThreadItem'; 16694 + readonly id: string; 16695 + readonly submissionId: string; 16696 + readonly data: JsonObject; 16697 + readonly type: { 16698 + readonly __typename: 'ThreadItemType'; 16699 + readonly id: string; 16700 + readonly name: string; 16701 + readonly baseFields: ReadonlyArray<{ 16702 + readonly __typename: 'BaseField'; 16703 + readonly name: string; 16704 + readonly type: GQLFieldType; 16705 + readonly required: boolean; 16706 + readonly container?: { 16707 + readonly __typename: 'Container'; 16708 + readonly containerType: GQLContainerType; 16709 + readonly keyScalarType?: GQLScalarType | null; 16710 + readonly valueScalarType: GQLScalarType; 16711 + } | null; 16712 + }>; 16713 + readonly schemaFieldRoles: { 16714 + readonly __typename: 'ThreadSchemaFieldRoles'; 16715 + readonly displayName?: string | null; 16716 + readonly createdAt?: string | null; 16717 + readonly creatorId?: string | null; 16718 + }; 16719 + }; 16720 + } 16693 16721 | { readonly __typename: 'UserItem' }; 16694 16722 }>; 16695 16723 }; ··· 35093 35121 displayName 35094 35122 parentId 35095 35123 threadId 35124 + createdAt 35125 + creatorId 35126 + } 35127 + } 35128 + } 35129 + ... on ThreadItem { 35130 + id 35131 + submissionId 35132 + data 35133 + type { 35134 + id 35135 + name 35136 + baseFields { 35137 + name 35138 + type 35139 + required 35140 + container { 35141 + containerType 35142 + keyScalarType 35143 + valueScalarType 35144 + } 35145 + } 35146 + schemaFieldRoles { 35147 + displayName 35096 35148 createdAt 35097 35149 creatorId 35098 35150 }
+57 -21
client/src/webpages/dashboard/mrt/manual_review_job/v2/user/ManualReviewJobLatestSubmissionsWithThreadComponent.tsx
··· 58 58 } 59 59 } 60 60 } 61 + ... on ThreadItem { 62 + id 63 + submissionId 64 + data 65 + type { 66 + id 67 + name 68 + baseFields { 69 + name 70 + type 71 + required 72 + container { 73 + containerType 74 + keyScalarType 75 + valueScalarType 76 + } 77 + } 78 + schemaFieldRoles { 79 + displayName 80 + createdAt 81 + creatorId 82 + } 83 + } 84 + } 61 85 } 62 86 } 63 87 } ··· 105 129 106 130 const dataValues = useMemo(() => { 107 131 return data?.latestItemsCreatedBy.map((itemSubmission) => { 108 - if (itemSubmission.latest.__typename !== 'ContentItem') { 109 - return undefined; 132 + const item = itemSubmission.latest; 133 + if (item.__typename === 'ContentItem') { 134 + const createdAt = getFieldValueForRole(item, 'createdAt'); 135 + const threadId = getFieldValueForRole(item, 'threadId'); 136 + const creatorId = getFieldValueForRole(item, 'creatorId'); 137 + return { 138 + itemId: item.id, 139 + itemData: item.data, 140 + itemTypeId: item.type.id, 141 + itemTypeName: item.type.name, 142 + itemTypeFields: item.type.baseFields, 143 + dateCreated: createdAt, 144 + threadId, 145 + creatorId, 146 + }; 110 147 } 111 - const createdAt = getFieldValueForRole( 112 - itemSubmission.latest, 113 - 'createdAt', 114 - ); 115 - const threadId = getFieldValueForRole(itemSubmission.latest, 'threadId'); 116 - const creatorId = getFieldValueForRole( 117 - itemSubmission.latest, 118 - 'creatorId', 119 - ); 120 - return { 121 - itemId: itemSubmission.latest.id, 122 - itemData: itemSubmission.latest.data, 123 - itemTypeId: itemSubmission.latest.type.id, 124 - itemTypeName: itemSubmission.latest.type.name, 125 - itemTypeFields: itemSubmission.latest.type.baseFields, 126 - dateCreated: createdAt, 127 - threadId, 128 - creatorId, 129 - }; 148 + // For Thread-kind items, the item itself is the thread (it's the 149 + // top-level post, with replies/comments as child Content items pointing 150 + // back at it via their `threadId` role). 151 + if (item.__typename === 'ThreadItem') { 152 + const createdAt = getFieldValueForRole(item, 'createdAt'); 153 + const creatorId = getFieldValueForRole(item, 'creatorId'); 154 + return { 155 + itemId: item.id, 156 + itemData: item.data, 157 + itemTypeId: item.type.id, 158 + itemTypeName: item.type.name, 159 + itemTypeFields: item.type.baseFields, 160 + dateCreated: createdAt, 161 + threadId: { id: item.id, typeId: item.type.id }, 162 + creatorId, 163 + }; 164 + } 165 + return undefined; 130 166 }); 131 167 }, [data]); 132 168
+142
server/services/itemProcessingService/makeItemSubmission.test.ts
··· 1 + import { ScalarTypes, type Field, type FieldType } from '@roostorg/types'; 2 + 3 + import { instantiateOpaqueType } from '../../utils/typescript-types.js'; 4 + import { 5 + type ContentItemType, 6 + type ContentSchemaFieldRoles, 7 + type ThreadItemType, 8 + type ThreadSchemaFieldRoles, 9 + type UserItemType, 10 + type UserSchemaFieldRoles, 11 + } from '../moderationConfigService/index.js'; 12 + import { getCreator } from './makeItemSubmission.js'; 13 + import { type NormalizedItemData } from './toNormalizedItemDataOrErrors.js'; 14 + 15 + // Field roles like `creatorId` resolve to RELATED_ITEM in the schema, which is 16 + // stored as `{ id, typeId }` in normalized data. 17 + const creatorIdField = { 18 + name: 'authorId', 19 + type: ScalarTypes.RELATED_ITEM, 20 + required: false, 21 + container: null, 22 + } as const satisfies Field<FieldType>; 23 + 24 + const baseSchema = [ 25 + creatorIdField, 26 + { 27 + name: 'displayName', 28 + type: ScalarTypes.STRING, 29 + required: false, 30 + container: null, 31 + } as const, 32 + { 33 + name: 'createdAt', 34 + type: ScalarTypes.DATETIME, 35 + required: true, 36 + container: null, 37 + } as const, 38 + ] as const satisfies readonly [Field<FieldType>, ...Field<FieldType>[]]; 39 + 40 + const baseTypeFields = { 41 + id: 'test', 42 + name: 'test', 43 + description: null, 44 + version: '1', 45 + schemaVariant: 'original', 46 + orgId: 'test-org', 47 + schema: baseSchema, 48 + } as const; 49 + 50 + function makeContentItemType( 51 + schemaFieldRoles: ContentSchemaFieldRoles, 52 + ): ContentItemType { 53 + return { ...baseTypeFields, kind: 'CONTENT', schemaFieldRoles }; 54 + } 55 + 56 + function makeThreadItemType( 57 + schemaFieldRoles: ThreadSchemaFieldRoles, 58 + ): ThreadItemType { 59 + return { ...baseTypeFields, kind: 'THREAD', schemaFieldRoles }; 60 + } 61 + 62 + function makeUserItemType( 63 + schemaFieldRoles: UserSchemaFieldRoles, 64 + ): UserItemType { 65 + return { 66 + ...baseTypeFields, 67 + kind: 'USER', 68 + isDefaultUserType: false, 69 + schemaFieldRoles, 70 + }; 71 + } 72 + 73 + const expectedCreator = { id: 'creator-id', typeId: 'creator-type-id' }; 74 + 75 + const itemDataWithCreator = instantiateOpaqueType<NormalizedItemData>({ 76 + authorId: expectedCreator, 77 + displayName: 'a post', 78 + createdAt: '2024-01-01T00:00:00.000Z', 79 + }); 80 + 81 + const itemDataWithoutCreator = instantiateOpaqueType<NormalizedItemData>({ 82 + displayName: 'a post', 83 + createdAt: '2024-01-01T00:00:00.000Z', 84 + }); 85 + 86 + describe('getCreator', () => { 87 + describe('CONTENT items', () => { 88 + test('returns the value of the creatorId field role', () => { 89 + const itemType = makeContentItemType({ 90 + creatorId: 'authorId', 91 + threadId: 'createdAt', 92 + createdAt: 'createdAt', 93 + }); 94 + expect(getCreator(itemType, itemDataWithCreator)).toEqual( 95 + expectedCreator, 96 + ); 97 + }); 98 + 99 + test('returns undefined when the creatorId role is not configured', () => { 100 + const itemType = makeContentItemType({}); 101 + expect(getCreator(itemType, itemDataWithCreator)).toBeUndefined(); 102 + }); 103 + 104 + test('returns undefined when the creatorId field is missing from the data', () => { 105 + const itemType = makeContentItemType({ 106 + creatorId: 'authorId', 107 + threadId: 'createdAt', 108 + createdAt: 'createdAt', 109 + }); 110 + expect(getCreator(itemType, itemDataWithoutCreator)).toBeUndefined(); 111 + }); 112 + }); 113 + 114 + describe('THREAD items', () => { 115 + test('returns the value of the creatorId field role', () => { 116 + const itemType = makeThreadItemType({ creatorId: 'authorId' }); 117 + expect(getCreator(itemType, itemDataWithCreator)).toEqual( 118 + expectedCreator, 119 + ); 120 + }); 121 + 122 + test('returns undefined when the creatorId role is not configured', () => { 123 + const itemType = makeThreadItemType({}); 124 + expect(getCreator(itemType, itemDataWithCreator)).toBeUndefined(); 125 + }); 126 + 127 + test('returns undefined when the creatorId field is missing from the data', () => { 128 + const itemType = makeThreadItemType({ creatorId: 'authorId' }); 129 + expect(getCreator(itemType, itemDataWithoutCreator)).toBeUndefined(); 130 + }); 131 + }); 132 + 133 + describe('USER items', () => { 134 + // User items are not "created by" themselves; we never want to populate 135 + // a creator for them, even if the schema happens to include a field 136 + // pointing at another item. 137 + test('always returns undefined', () => { 138 + const itemType = makeUserItemType({}); 139 + expect(getCreator(itemType, itemDataWithCreator)).toBeUndefined(); 140 + }); 141 + }); 142 + });
+8 -4
server/services/itemProcessingService/makeItemSubmission.ts
··· 93 93 /** 94 94 * The ItemIdentifier for the user that created this item. 95 95 * 96 - * NB: this only applies to content items; user items don't list the user as 97 - * the item's creator, and threads don't (yet) have creators. 96 + * Populated from the item type's `creatorId` field role for both CONTENT 97 + * and THREAD item types. USER items don't list themselves as their own 98 + * creator. 98 99 * 99 100 * NB: this could be generated from the schema and field roles on the 100 101 * itemType, but to support older users who submitted the creator as a ··· 286 287 }; 287 288 } 288 289 289 - function getCreator(itemType: ItemType, itemData: NormalizedItemData) { 290 + export function getCreator( 291 + itemType: ItemType, 292 + itemData: NormalizedItemData, 293 + ) { 290 294 switch (itemType.kind) { 291 - case 'THREAD': 292 295 case 'USER': 293 296 return undefined; 297 + case 'THREAD': 294 298 case 'CONTENT': 295 299 return getFieldValueForRole( 296 300 itemType.schema,