Skip to content

Commit

Permalink
Correctly handle asset Ids in asset pagination
Browse files Browse the repository at this point in the history
Summary:
The frontend passes in an ID which may have been emitted by get_unique_asset_id, which prepends a prefix before the path starts. Fix the asset key normalizer to account for that case.

Test Plan: New test case

hi
  • Loading branch information
gibsondan committed Aug 8, 2024
1 parent 6985c0d commit 175b69f
Show file tree
Hide file tree
Showing 10 changed files with 160 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ export function useAllAssets({
const fetchAssets = useCallback(async () => {
try {
const data = await fetchPaginatedData({
async fetchData(cursor: string | undefined) {
async fetchData(cursor: string | null | undefined) {
const {data} = await client.query<
AssetCatalogTableQuery,
AssetCatalogTableQueryVariables
Expand All @@ -114,7 +114,7 @@ export function useAllAssets({
}
const assets = data.assetsOrError.nodes;
const hasMoreData = assets.length === batchLimit;
const nextCursor = hasMoreData ? assets[assets.length - 1]!.id : undefined;
const nextCursor = data.assetsOrError.cursor;
return {
data: assets,
cursor: nextCursor,
Expand Down Expand Up @@ -289,6 +289,7 @@ export const ASSET_CATALOG_TABLE_QUERY = gql`
id
...AssetTableFragment
}
cursor
}
...PythonErrorFragment
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,12 @@ jest.mock('idb-lru-cache', () => {

const createMock = ({
nodes,
returnedCursor,
cursor,
limit = 2,
}: {
limit?: number;
returnedCursor: string | null;
cursor?: string;
nodes: Asset[];
}) =>
Expand All @@ -48,6 +50,7 @@ const createMock = ({
data: {
assetsOrError: buildAssetConnection({
nodes,
cursor: returnedCursor,
}),
},
delay: 100,
Expand All @@ -57,14 +60,17 @@ describe('useAllAssets', () => {
it('Paginates correctly', async () => {
const mock = createMock({
nodes: [buildAsset({id: 'asset-id-1'}), buildAsset({id: 'asset-id-2'})],
returnedCursor: 'asset-key-2',
});
const mock2 = createMock({
cursor: 'asset-id-2',
cursor: 'asset-key-2',
nodes: [buildAsset({id: 'asset-id-3'}), buildAsset({id: 'asset-id-4'})],
returnedCursor: 'asset-key-4',
});
const mock3 = createMock({
cursor: 'asset-id-4',
cursor: 'asset-key-4',
nodes: [buildAsset({id: 'asset-id-5'})],
returnedCursor: null,
});

const {result} = renderHook(() => useAllAssets({batchLimit: 2}), {
Expand All @@ -90,14 +96,17 @@ describe('useAllAssets', () => {
});
const mock = createMock({
nodes: [buildAsset({id: 'asset-id-1'}), buildAsset({id: 'asset-id-2'})],
returnedCursor: 'asset-key-2',
});
const mock2 = createMock({
cursor: 'asset-id-2',
cursor: 'asset-key-2',
nodes: [buildAsset({id: 'asset-id-3'}), buildAsset({id: 'asset-id-4'})],
returnedCursor: 'asset-key-4',
});
const mock3 = createMock({
cursor: 'asset-id-4',
cursor: 'asset-key-4',
nodes: [buildAsset({id: 'asset-id-5'})],
returnedCursor: null,
});

const {result} = renderHook(() => useAllAssets({batchLimit: 2}), {
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions js_modules/dagster-ui/packages/ui-core/src/graphql/types.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,8 @@ def get_assets(
definition=asset_nodes_by_asset_key.get(asset_key),
)
for asset_key in asset_keys
]
],
cursor=asset_keys[-1].to_string() if asset_keys else None,
)


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ def resolve_assets(self, graphene_info):
def resolve_assetsOrError(self, graphene_info) -> "GrapheneAssetConnection":
from .roots.assets import GrapheneAssetConnection

return GrapheneAssetConnection(nodes=self._get_assets(graphene_info))
return GrapheneAssetConnection(nodes=self._get_assets(graphene_info), cursor=None)

class Meta:
name = "AssetSelection"
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

class GrapheneAssetConnection(graphene.ObjectType):
nodes = non_null_list(GrapheneAsset)
cursor = graphene.Field(graphene.String)

class Meta:
name = "AssetConnection"
Expand Down
Loading

0 comments on commit 175b69f

Please sign in to comment.