Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refetch aggregate queries on record creation/update/deletion of record #8885

Merged
merged 7 commits into from
Dec 5, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { getObjectTypename } from '@/object-record/cache/utils/getObjectTypename
import { RecordGqlOperationGqlRecordFields } from '@/object-record/graphql/types/RecordGqlOperationGqlRecordFields';
import { generateDepthOneRecordGqlFields } from '@/object-record/graphql/utils/generateDepthOneRecordGqlFields';
import { useCreateManyRecordsMutation } from '@/object-record/hooks/useCreateManyRecordsMutation';
import { useRefetchAggregateQueries } from '@/object-record/hooks/useRefetchAggregateQueries';
import { ObjectRecord } from '@/object-record/types/ObjectRecord';
import { getCreateManyRecordsMutationResponseField } from '@/object-record/utils/getCreateManyRecordsMutationResponseField';
import { sanitizeRecordInput } from '@/object-record/utils/sanitizeRecordInput';
Expand Down Expand Up @@ -51,6 +52,10 @@ export const useCreateManyRecords = <

const { objectMetadataItems } = useObjectMetadataItems();

const { refetchAggregateQueries } = useRefetchAggregateQueries({
objectMetadataNamePlural: objectMetadataItem.namePlural,
});

const createManyRecords = async (
recordsToCreate: Partial<CreatedObjectRecord>[],
upsert?: boolean,
Expand Down Expand Up @@ -141,6 +146,7 @@ export const useCreateManyRecords = <
throw error;
});

await refetchAggregateQueries();
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this will seek to refetch aggregate queries even if there were no AggregateQueries performed before (like on table view for instance). I think it's ok but it does trigger a warning (yellow) in the console.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return createdObjects.data?.[mutationResponseField] ?? [];
Comment on lines +149 to 150
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: consider wrapping refetch in try/catch to handle potential refetch failures gracefully

};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { getObjectTypename } from '@/object-record/cache/utils/getObjectTypename
import { RecordGqlOperationGqlRecordFields } from '@/object-record/graphql/types/RecordGqlOperationGqlRecordFields';
import { generateDepthOneRecordGqlFields } from '@/object-record/graphql/utils/generateDepthOneRecordGqlFields';
import { useCreateOneRecordMutation } from '@/object-record/hooks/useCreateOneRecordMutation';
import { useRefetchAggregateQueries } from '@/object-record/hooks/useRefetchAggregateQueries';
import { ObjectRecord } from '@/object-record/types/ObjectRecord';
import { getCreateOneRecordMutationResponseField } from '@/object-record/utils/getCreateOneRecordMutationResponseField';
import { sanitizeRecordInput } from '@/object-record/utils/sanitizeRecordInput';
Expand Down Expand Up @@ -55,6 +56,10 @@ export const useCreateOneRecord = <

const { objectMetadataItems } = useObjectMetadataItems();

const { refetchAggregateQueries } = useRefetchAggregateQueries({
objectMetadataNamePlural: objectMetadataItem.namePlural,
});

const createOneRecord = async (input: Partial<CreatedObjectRecord>) => {
setLoading(true);

Expand Down Expand Up @@ -131,6 +136,7 @@ export const useCreateOneRecord = <
throw error;
});

await refetchAggregateQueries();
return createdObject.data?.[mutationResponseField] ?? null;
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { getRecordNodeFromRecord } from '@/object-record/cache/utils/getRecordNo
import { updateRecordFromCache } from '@/object-record/cache/utils/updateRecordFromCache';
import { DEFAULT_MUTATION_BATCH_SIZE } from '@/object-record/constants/DefaultMutationBatchSize';
import { useDeleteManyRecordsMutation } from '@/object-record/hooks/useDeleteManyRecordsMutation';
import { useRefetchAggregateQueries } from '@/object-record/hooks/useRefetchAggregateQueries';
import { ObjectRecord } from '@/object-record/types/ObjectRecord';
import { getDeleteManyRecordsMutationResponseField } from '@/object-record/utils/getDeleteManyRecordsMutationResponseField';
import { useRecoilValue } from 'recoil';
Expand Down Expand Up @@ -51,6 +52,10 @@ export const useDeleteManyRecords = ({

const { objectMetadataItems } = useObjectMetadataItems();

const { refetchAggregateQueries } = useRefetchAggregateQueries({
objectMetadataNamePlural: objectMetadataItem.namePlural,
});

const mutationResponseField = getDeleteManyRecordsMutationResponseField(
objectMetadataItem.namePlural,
);
Expand Down Expand Up @@ -194,7 +199,7 @@ export const useDeleteManyRecords = ({
await sleep(options.delayInMsBetweenRequests);
}
}

await refetchAggregateQueries();
ijreilly marked this conversation as resolved.
Show resolved Hide resolved
return deletedRecords;
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { useGetRecordFromCache } from '@/object-record/cache/hooks/useGetRecordF
import { getRecordNodeFromRecord } from '@/object-record/cache/utils/getRecordNodeFromRecord';
import { updateRecordFromCache } from '@/object-record/cache/utils/updateRecordFromCache';
import { useDeleteOneRecordMutation } from '@/object-record/hooks/useDeleteOneRecordMutation';
import { useRefetchAggregateQueries } from '@/object-record/hooks/useRefetchAggregateQueries';
import { ObjectRecord } from '@/object-record/types/ObjectRecord';
import { getDeleteOneRecordMutationResponseField } from '@/object-record/utils/getDeleteOneRecordMutationResponseField';
import { capitalize } from '~/utils/string/capitalize';
Expand Down Expand Up @@ -35,6 +36,10 @@ export const useDeleteOneRecord = ({

const { objectMetadataItems } = useObjectMetadataItems();

const { refetchAggregateQueries } = useRefetchAggregateQueries({
objectMetadataNamePlural: objectMetadataItem.namePlural,
});

const mutationResponseField =
getDeleteOneRecordMutationResponseField(objectNameSingular);

Expand Down Expand Up @@ -126,6 +131,7 @@ export const useDeleteOneRecord = ({
throw error;
});

await refetchAggregateQueries();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: refetchAggregateQueries() should be wrapped in a try/catch to handle potential refetch failures gracefully

return deletedRecord.data?.[mutationResponseField] ?? null;
},
[
Expand All @@ -135,6 +141,7 @@ export const useDeleteOneRecord = ({
mutationResponseField,
objectMetadataItem,
objectMetadataItems,
refetchAggregateQueries,
],
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { useObjectMetadataItems } from '@/object-metadata/hooks/useObjectMetadat
import { useGetRecordFromCache } from '@/object-record/cache/hooks/useGetRecordFromCache';
import { DEFAULT_MUTATION_BATCH_SIZE } from '@/object-record/constants/DefaultMutationBatchSize';
import { useDestroyManyRecordsMutation } from '@/object-record/hooks/useDestroyManyRecordsMutation';
import { useRefetchAggregateQueries } from '@/object-record/hooks/useRefetchAggregateQueries';
import { getDestroyManyRecordsMutationResponseField } from '@/object-record/utils/getDestroyManyRecordsMutationResponseField';
import { useRecoilValue } from 'recoil';
import { isDefined } from '~/utils/isDefined';
Expand Down Expand Up @@ -48,6 +49,10 @@ export const useDestroyManyRecords = ({

const { objectMetadataItems } = useObjectMetadataItems();

const { refetchAggregateQueries } = useRefetchAggregateQueries({
objectMetadataNamePlural: objectMetadataItem.namePlural,
});

const mutationResponseField = getDestroyManyRecordsMutationResponseField(
objectMetadataItem.namePlural,
);
Expand Down Expand Up @@ -127,6 +132,7 @@ export const useDestroyManyRecords = ({
}
}

await refetchAggregateQueries();
Comment on lines 134 to +135
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: refetchAggregateQueries could fail silently here - consider handling errors or at least logging them

return destroyedRecords;
};

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import { getAggregateQueryName } from '@/object-record/utils/getAggregateQueryName';
import { useIsFeatureEnabled } from '@/workspace/hooks/useIsFeatureEnabled';
import { useApolloClient } from '@apollo/client';

export const useRefetchAggregateQueries = ({
objectMetadataNamePlural,
}: {
objectMetadataNamePlural: string;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: objectMetadataNamePlural should be validated - getAggregateQueryName will throw if this is empty

}) => {
const apolloClient = useApolloClient();
const isAggregateQueryEnabled = useIsFeatureEnabled(
'IS_AGGREGATE_QUERY_ENABLED',
);
const refetchAggregateQueries = async () => {
const queryName = getAggregateQueryName(objectMetadataNamePlural);

if (isAggregateQueryEnabled) {
await apolloClient.refetchQueries({
include: [queryName],
});
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: refetchQueries could fail - needs try/catch with error handling

};

return {
refetchAggregateQueries,
};
};
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { useGetRecordFromCache } from '@/object-record/cache/hooks/useGetRecordF
import { getRecordNodeFromRecord } from '@/object-record/cache/utils/getRecordNodeFromRecord';
import { updateRecordFromCache } from '@/object-record/cache/utils/updateRecordFromCache';
import { generateDepthOneRecordGqlFields } from '@/object-record/graphql/utils/generateDepthOneRecordGqlFields';
import { useRefetchAggregateQueries } from '@/object-record/hooks/useRefetchAggregateQueries';
import { useUpdateOneRecordMutation } from '@/object-record/hooks/useUpdateOneRecordMutation';
import { ObjectRecord } from '@/object-record/types/ObjectRecord';
import { getUpdateOneRecordMutationResponseField } from '@/object-record/utils/getUpdateOneRecordMutationResponseField';
Expand Down Expand Up @@ -45,6 +46,10 @@ export const useUpdateOneRecord = <

const { objectMetadataItems } = useObjectMetadataItems();

const { refetchAggregateQueries } = useRefetchAggregateQueries({
objectMetadataNamePlural: objectMetadataItem.namePlural,
});

const updateOneRecord = async ({
idToUpdate,
updateOneRecordInput,
Expand Down Expand Up @@ -116,7 +121,7 @@ export const useUpdateOneRecord = <
idToUpdate,
input: sanitizedInput,
},
update: (cache, { data }) => {
update: async (cache, { data }) => {
ijreilly marked this conversation as resolved.
Show resolved Hide resolved
const record = data?.[mutationResponseField];

if (!record || !cachedRecord) return;
Expand All @@ -128,6 +133,8 @@ export const useUpdateOneRecord = <
updatedRecord: record,
objectMetadataItems,
});

await refetchAggregateQueries();
ijreilly marked this conversation as resolved.
Show resolved Hide resolved
},
})
.catch((error: Error) => {
ijreilly marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -152,6 +159,7 @@ export const useUpdateOneRecord = <
throw error;
});

await refetchAggregateQueries();
return updatedRecord?.data?.[mutationResponseField] ?? null;
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { RecordBoardColumnHotkeyScope } from '@/object-record/record-board/types
import { Dropdown } from '@/ui/layout/dropdown/components/Dropdown';

type RecordBoardColumnHeaderAggregateDropdownProps = {
aggregateValue: string | number;
aggregateValue?: string | number;
aggregateLabel?: string;
objectMetadataItem: ObjectMetadataItem;
dropdownId: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,16 @@ export const RecordBoardColumnHeaderAggregateDropdownButton = ({
tooltip,
}: {
dropdownId: string;
value: string | number;
value?: string | number;
tooltip?: string;
}) => {
return (
<div id={dropdownId}>
<StyledButton>
<Tag text={formatNumber(Number(value))} color={'transparent'} />
<Tag
text={value ? formatNumber(Number(value)) : '-'}
color={'transparent'}
/>
<AppTooltip
anchorSelect={`#${dropdownId}`}
content={tooltip}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export const RecordBoardColumnHeaderAggregateDropdownFieldsContent = () => {

if (!fieldMetadata) return null;
return (
<DropdownMenuItemsContainer>
<DropdownMenuItemsContainer key={fieldId}>
<MenuItem
key={fieldId}
onClick={() => {
ijreilly marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ export const useAggregateManyRecordsForRecordBoardColumn = () => {
);

return {
aggregateValue: value ?? recordCount,
aggregateValue: isAggregateQueryEnabled ? value : recordCount,
aggregateLabel: isDefined(value) ? label : undefined,
};
};
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import gql from 'graphql-tag';

import { ObjectMetadataItem } from '@/object-metadata/types/ObjectMetadataItem';
import { RecordGqlFields } from '@/object-record/graphql/types/RecordGqlFields';
import { getAggregateQueryName } from '@/object-record/utils/getAggregateQueryName';
import { capitalize } from '~/utils/string/capitalize';

export const generateAggregateQuery = ({
Expand All @@ -17,7 +18,7 @@ export const generateAggregateQuery = ({
.join('\n ');

return gql`
query AggregateMany${capitalize(objectMetadataItem.namePlural)}($filter: ${capitalize(
query ${getAggregateQueryName(objectMetadataItem.namePlural)}($filter: ${capitalize(
objectMetadataItem.nameSingular,
)}FilterInput) {
${objectMetadataItem.namePlural}(filter: $filter) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { capitalize } from '~/utils/string/capitalize';

export const getAggregateQueryName = (
objectMetadataNamePlural: string,
): string => {
if (!objectMetadataNamePlural) {
throw new Error('objectMetadataNamePlural is required');
ijreilly marked this conversation as resolved.
Show resolved Hide resolved
}
return `AggregateMany${capitalize(objectMetadataNamePlural)}`;
ijreilly marked this conversation as resolved.
Show resolved Hide resolved
};
Loading