Skip to content

Commit

Permalink
Sync stripe tables (twentyhq#5475)
Browse files Browse the repository at this point in the history
Stripe tables do not support `hasNextPage` and `totalCount`. This may be
because of stripe wrapper do not properly support `COUNT` request.
Waiting on pg_graphql answer
[here](supabase/pg_graphql#519).

This PR:
- removes `totalCount` and `hasNextPage` form queries for remote
objects. Even if it works for postgres, this may really be inefficient
- adapt the `fetchMore` functions so it works despite `hasNextPage`
missing
- remove `totalCount` display for remotes
- fix `orderBy`

---------

Co-authored-by: Thomas Trompette <[email protected]>
  • Loading branch information
thomtrp and Thomas Trompette authored May 22, 2024
1 parent 35c1f97 commit 2e79bcc
Show file tree
Hide file tree
Showing 22 changed files with 191 additions and 87 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { RecordGqlRefEdge } from '@/object-record/cache/types/RecordGqlRefEdge';
import { getEdgeTypename } from '@/object-record/cache/utils/getEdgeTypename';
import { isObjectRecordConnectionWithRefs } from '@/object-record/cache/utils/isObjectRecordConnectionWithRefs';
import { RecordGqlNode } from '@/object-record/graphql/types/RecordGqlNode';
import { isDefined } from '~/utils/isDefined';

/*
TODO: for now new records are added to all cached record lists, no matter what the variables (filters, orderBy, etc.) are.
Expand Down Expand Up @@ -61,11 +62,10 @@ export const triggerCreateRecordsOptimisticEffect = ({
rootQueryCachedObjectRecordConnection,
);

const rootQueryCachedRecordTotalCount =
readField<number>(
'totalCount',
rootQueryCachedObjectRecordConnection,
) || 0;
const rootQueryCachedRecordTotalCount = readField<number | undefined>(
'totalCount',
rootQueryCachedObjectRecordConnection,
);

const nextRootQueryCachedRecordEdges = rootQueryCachedRecordEdges
? [...rootQueryCachedRecordEdges]
Expand Down Expand Up @@ -113,7 +113,9 @@ export const triggerCreateRecordsOptimisticEffect = ({
return {
...rootQueryCachedObjectRecordConnection,
edges: nextRootQueryCachedRecordEdges,
totalCount: rootQueryCachedRecordTotalCount + 1,
totalCount: isDefined(rootQueryCachedRecordTotalCount)
? rootQueryCachedRecordTotalCount + 1
: undefined,
};
},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,10 @@ export const triggerDeleteRecordsOptimisticEffect = ({
rootQueryCachedObjectRecordConnection,
);

const totalCount =
readField<number>(
'totalCount',
rootQueryCachedObjectRecordConnection,
) || 0;
const totalCount = readField<number | undefined>(
'totalCount',
rootQueryCachedObjectRecordConnection,
);

const nextCachedEdges =
cachedEdges?.filter((cachedEdge) => {
Expand All @@ -77,7 +76,9 @@ export const triggerDeleteRecordsOptimisticEffect = ({
return {
...rootQueryCachedObjectRecordConnection,
edges: nextCachedEdges,
totalCount: totalCount - recordIdsToDelete.length,
totalCount: isDefined(totalCount)
? totalCount - recordIdsToDelete.length
: undefined,
};
},
},
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import { ObjectMetadataItem } from '@/object-metadata/types/ObjectMetadataItem';

export const hasPositionField = (objectMetadataItem: ObjectMetadataItem) =>
!objectMetadataItem.isRemote;
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import { ObjectMetadataItem } from '@/object-metadata/types/ObjectMetadataItem';

export const isAggregationEnabled = (objectMetadataItem: ObjectMetadataItem) =>
!objectMetadataItem.isRemote;
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ export const useFindDuplicateRecords = <T extends ObjectRecord = ObjectRecord>({
return {
objectMetadataItem,
records,
totalCount: objectRecordConnection?.totalCount || 0,
totalCount: objectRecordConnection?.totalCount,
loading,
error,
queryStateIdentifier: findDuplicateQueryStateIdentifier,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { useRecoilValue } from 'recoil';

import { useObjectMetadataItem } from '@/object-metadata/hooks/useObjectMetadataItem';
import { objectMetadataItemsState } from '@/object-metadata/states/objectMetadataItemsState';
import { isAggregationEnabled } from '@/object-metadata/utils/isAggregationEnabled';
import { mapObjectMetadataToGraphQLQuery } from '@/object-metadata/utils/mapObjectMetadataToGraphQLQuery';
import { getFindDuplicateRecordsQueryResponseField } from '@/object-record/utils/getFindDuplicateRecordsQueryResponseField';
import { capitalize } from '~/utils/string/capitalize';
Expand Down Expand Up @@ -33,11 +34,11 @@ export const useFindDuplicateRecordsQuery = ({
cursor
}
pageInfo {
hasNextPage
${isAggregationEnabled(objectMetadataItem) ? 'hasNextPage' : ''}
startCursor
endCursor
}
totalCount
${isAggregationEnabled(objectMetadataItem) ? 'totalCount' : ''}
}
}
`;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { useRecoilState, useRecoilValue, useSetRecoilState } from 'recoil';
import { currentWorkspaceMemberState } from '@/auth/states/currentWorkspaceMemberState';
import { useObjectMetadataItem } from '@/object-metadata/hooks/useObjectMetadataItem';
import { ObjectMetadataItemIdentifier } from '@/object-metadata/types/ObjectMetadataItemIdentifier';
import { isAggregationEnabled } from '@/object-metadata/utils/isAggregationEnabled';
import { getRecordsFromRecordConnection } from '@/object-record/cache/utils/getRecordsFromRecordConnection';
import { RecordGqlConnection } from '@/object-record/graphql/types/RecordGqlConnection';
import { RecordGqlEdge } from '@/object-record/graphql/types/RecordGqlEdge';
Expand Down Expand Up @@ -122,7 +123,8 @@ export const useFindManyRecords = <T extends ObjectRecord = ObjectRecord>({
});

const fetchMoreRecords = useCallback(async () => {
if (hasNextPage) {
// Remote objects does not support hasNextPage. We cannot rely on it to fetch more records.
if (hasNextPage || (!isAggregationEnabled(objectMetadataItem) && !error)) {
setIsFetchingMoreObjects(true);

try {
Expand All @@ -137,11 +139,11 @@ export const useFindManyRecords = <T extends ObjectRecord = ObjectRecord>({
const nextEdges =
fetchMoreResult?.[objectMetadataItem.namePlural]?.edges;

let newEdges: RecordGqlEdge[] = [];
let newEdges: RecordGqlEdge[] = previousEdges ?? [];

if (isNonEmptyArray(previousEdges) && isNonEmptyArray(nextEdges)) {
if (isNonEmptyArray(nextEdges)) {
newEdges = filterUniqueRecordEdgesByCursor([
...(prev?.[objectMetadataItem.namePlural]?.edges ?? []),
...newEdges,
...(fetchMoreResult?.[objectMetadataItem.namePlural]?.edges ??
[]),
]);
Expand Down Expand Up @@ -199,21 +201,21 @@ export const useFindManyRecords = <T extends ObjectRecord = ObjectRecord>({
}
}, [
hasNextPage,
objectMetadataItem,
error,
setIsFetchingMoreObjects,
fetchMore,
filter,
orderBy,
lastCursor,
objectMetadataItem.namePlural,
objectMetadataItem.nameSingular,
onCompleted,
data,
onCompleted,
setLastCursor,
setHasNextPage,
enqueueSnackBar,
]);

const totalCount = data?.[objectMetadataItem.namePlural].totalCount ?? 0;
const totalCount = data?.[objectMetadataItem.namePlural]?.totalCount;

const records = useMemo(
() =>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { FieldMetadataItem } from '@/object-metadata/types/FieldMetadataItem';
import { ObjectMetadataItem } from '@/object-metadata/types/ObjectMetadataItem';
import { Sort } from '@/object-record/object-sort-dropdown/types/Sort';
import { SortDefinition } from '@/object-record/object-sort-dropdown/types/SortDefinition';
import { turnSortsIntoOrderBy } from '@/object-record/object-sort-dropdown/utils/turnSortsIntoOrderBy';
Expand All @@ -8,12 +10,30 @@ const sortDefinition: SortDefinition = {
iconName: 'icon',
};

const objectMetadataItem: ObjectMetadataItem = {
id: 'object1',
fields: [],
createdAt: '2021-01-01',
updatedAt: '2021-01-01',
nameSingular: 'object1',
namePlural: 'object1s',
icon: 'icon',
isActive: true,
isSystem: false,
isCustom: false,
isRemote: false,
labelPlural: 'object1s',
labelSingular: 'object1',
};

describe('turnSortsIntoOrderBy', () => {
it('should sort by recordPosition if no sorts', () => {
const fields = [{ id: 'field1', name: 'createdAt' }];
expect(turnSortsIntoOrderBy([], fields)).toEqual({
position: 'AscNullsFirst',
});
const fields = [{ id: 'field1', name: 'createdAt' }] as FieldMetadataItem[];
expect(turnSortsIntoOrderBy({ ...objectMetadataItem, fields }, [])).toEqual(
{
position: 'AscNullsFirst',
},
);
});

it('should create OrderByField with single sort', () => {
Expand All @@ -24,8 +44,10 @@ describe('turnSortsIntoOrderBy', () => {
definition: sortDefinition,
},
];
const fields = [{ id: 'field1', name: 'field1' }];
expect(turnSortsIntoOrderBy(sorts, fields)).toEqual({
const fields = [{ id: 'field1', name: 'field1' }] as FieldMetadataItem[];
expect(
turnSortsIntoOrderBy({ ...objectMetadataItem, fields }, sorts),
).toEqual({
field1: 'AscNullsFirst',
position: 'AscNullsFirst',
});
Expand All @@ -47,8 +69,10 @@ describe('turnSortsIntoOrderBy', () => {
const fields = [
{ id: 'field1', name: 'field1' },
{ id: 'field2', name: 'field2' },
];
expect(turnSortsIntoOrderBy(sorts, fields)).toEqual({
] as FieldMetadataItem[];
expect(
turnSortsIntoOrderBy({ ...objectMetadataItem, fields }, sorts),
).toEqual({
field1: 'AscNullsFirst',
field2: 'DescNullsLast',
position: 'AscNullsFirst',
Expand All @@ -63,8 +87,21 @@ describe('turnSortsIntoOrderBy', () => {
definition: sortDefinition,
},
];
expect(turnSortsIntoOrderBy(sorts, [])).toEqual({
expect(turnSortsIntoOrderBy(objectMetadataItem, sorts)).toEqual({
position: 'AscNullsFirst',
});
});

it('should not return position for remotes', () => {
const sorts: Sort[] = [
{
fieldMetadataId: 'invalidField',
direction: 'asc',
definition: sortDefinition,
},
];
expect(
turnSortsIntoOrderBy({ ...objectMetadataItem, isRemote: true }, sorts),
).toEqual({});
});
});
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { ObjectMetadataItem } from '@/object-metadata/types/ObjectMetadataItem';
import { OrderBy } from '@/object-metadata/types/OrderBy';
import { hasPositionField } from '@/object-metadata/utils/hasPositionColumn';
import { RecordGqlOperationOrderBy } from '@/object-record/graphql/types/RecordGqlOperationOrderBy';
import { Field } from '~/generated/graphql';
import { mapArrayToObject } from '~/utils/array/mapArrayToObject';
Expand All @@ -8,9 +10,10 @@ import { isUndefinedOrNull } from '~/utils/isUndefinedOrNull';
import { Sort } from '../types/Sort';

export const turnSortsIntoOrderBy = (
objectMetadataItem: ObjectMetadataItem,
sorts: Sort[],
fields: Pick<Field, 'id' | 'name'>[],
): RecordGqlOperationOrderBy => {
const fields: Pick<Field, 'id' | 'name'>[] = objectMetadataItem?.fields ?? [];
const fieldsById = mapArrayToObject(fields, ({ id }) => id);
const sortsOrderBy = Object.fromEntries(
sorts
Expand All @@ -29,8 +32,12 @@ export const turnSortsIntoOrderBy = (
.filter(isDefined),
);

return {
...sortsOrderBy,
position: 'AscNullsFirst',
};
if (hasPositionField(objectMetadataItem)) {
return {
...sortsOrderBy,
position: 'AscNullsFirst',
};
}

return sortsOrderBy;
};
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@ import { useFavorites } from '@/favorites/hooks/useFavorites';
import { ObjectMetadataItem } from '@/object-metadata/types/ObjectMetadataItem';
import { useDeleteManyRecords } from '@/object-record/hooks/useDeleteManyRecords';
import { useExecuteQuickActionOnOneRecord } from '@/object-record/hooks/useExecuteQuickActionOnOneRecord';
import { useExportTableData } from '@/object-record/record-index/options/hooks/useExportTableData';
import {
displayedExportProgress,
useExportTableData,
} from '@/object-record/record-index/options/hooks/useExportTableData';
import { recordStoreFamilyState } from '@/object-record/record-store/states/recordStoreFamilyState';
import { ConfirmationModal } from '@/ui/layout/modal/components/ConfirmationModal';
import { actionBarEntriesState } from '@/ui/navigation/action-bar/states/actionBarEntriesState';
Expand Down Expand Up @@ -111,7 +114,7 @@ export const useRecordActionBar = ({
const baseActions: ContextMenuEntry[] = useMemo(
() => [
{
label: `${progress === undefined ? `Export` : `Export (${progress}%)`}`,
label: displayedExportProgress(progress),
Icon: IconFileExport,
accent: 'default',
onClick: () => download(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,7 @@ export const useLoadRecordIndexBoard = ({
recordIndexFilters,
objectMetadataItem?.fields ?? [],
);
const orderBy = !objectMetadataItem.isRemote
? turnSortsIntoOrderBy(recordIndexSorts, objectMetadataItem?.fields ?? [])
: undefined;
const orderBy = turnSortsIntoOrderBy(objectMetadataItem, recordIndexSorts);

const recordIndexIsCompactModeActive = useRecoilValue(
recordIndexIsCompactModeActiveState,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,7 @@ export const useFindManyParams = (
objectMetadataItem?.fields ?? [],
);

if (objectMetadataItem?.isRemote) {
return { objectNameSingular, filter };
}

const orderBy = turnSortsIntoOrderBy(
tableSorts,
objectMetadataItem?.fields ?? [],
);
const orderBy = turnSortsIntoOrderBy(objectMetadataItem, tableSorts);

return { objectNameSingular, filter, orderBy };
};
Expand Down Expand Up @@ -71,7 +64,7 @@ export const useLoadRecordIndexTable = (objectNameSingular: string) => {
currentWorkspace?.activationStatus === 'active'
? records
: SIGN_IN_BACKGROUND_MOCK_COMPANIES,
totalCount: totalCount || 0,
totalCount: totalCount,
loading,
fetchMoreRecords,
queryStateIdentifier,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@ import {
} from 'twenty-ui';

import { RECORD_INDEX_OPTIONS_DROPDOWN_ID } from '@/object-record/record-index/options/constants/RecordIndexOptionsDropdownId';
import { useExportTableData } from '@/object-record/record-index/options/hooks/useExportTableData';
import {
displayedExportProgress,
useExportTableData,
} from '@/object-record/record-index/options/hooks/useExportTableData';
import { useRecordIndexOptionsForBoard } from '@/object-record/record-index/options/hooks/useRecordIndexOptionsForBoard';
import { useRecordIndexOptionsForTable } from '@/object-record/record-index/options/hooks/useRecordIndexOptionsForTable';
import { TableOptionsHotkeyScope } from '@/object-record/record-table/types/TableOptionsHotkeyScope';
Expand Down Expand Up @@ -123,7 +126,7 @@ export const RecordIndexOptionsDropdownContent = ({
<MenuItem
onClick={download}
LeftIcon={IconFileExport}
text={progress === undefined ? `Export` : `Export (${progress}%)`}
text={displayedExportProgress(progress)}
/>
</DropdownMenuItemsContainer>
)}
Expand Down
Loading

0 comments on commit 2e79bcc

Please sign in to comment.