Skip to content

Commit cbb317b

Browse files
Fix user fetching limit causing indefinite loading when named version creator is not in first 100 users (#193)
* Fetch next user batch if more users are present and current view has not loaded all user * Add changelog
1 parent 393f508 commit cbb317b

4 files changed

Lines changed: 59 additions & 17 deletions

File tree

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
{
2+
"changes": [
3+
{
4+
"packageName": "@itwin/manage-versions-react",
5+
"comment": "Fetch next user batch if current view has user that are not fetched yet.",
6+
"type": "patch"
7+
}
8+
],
9+
"packageName": "@itwin/manage-versions-react"
10+
}

packages/modules/manage-versions/src/clients/changesetClient.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,13 +43,16 @@ export class ChangesetClient {
4343
.then((resp) => resp.changesets);
4444
}
4545

46-
public async getUsers(imodelId: string): Promise<User[]> {
46+
public async getUsers(
47+
imodelId: string,
48+
requestOptions: RequestOptions = {}
49+
): Promise<{ users: User[]; hasMore: boolean }> {
4750
return this._http
4851
.get(
4952
`${UrlBuilder.buildGetUsersUrl(
5053
imodelId,
5154
this._serverEnvironmentPrefix
52-
)}`,
55+
)}${UrlBuilder.getQuery(requestOptions)}`,
5356
{
5457
headers: {
5558
[HttpHeaderNames.Prefer]: "return=representation",
@@ -58,7 +61,10 @@ export class ChangesetClient {
5861
},
5962
}
6063
)
61-
.then((resp) => resp.users);
64+
.then((resp) => ({
65+
users: resp.users,
66+
hasMore: !!resp._links?.next?.href,
67+
}));
6268
}
6369

6470
public async getChangesetCheckpoint(

packages/modules/manage-versions/src/components/ManageVersions/ManageVersions.test.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ describe("ManageVersions", () => {
6565
jest.clearAllMocks();
6666
mockGetVersions.mockResolvedValue(MockedVersionList());
6767
mockGetChangesets.mockResolvedValue(MockedChangesetList());
68-
mockGetUsers.mockResolvedValue(MockedUsers());
68+
mockGetUsers.mockResolvedValue({ users: MockedUsers(), hasMore: false });
6969
});
7070

7171
it("should show versions table with data", async () => {

packages/modules/manage-versions/src/components/ManageVersions/ManageVersions.tsx

Lines changed: 39 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ export enum ManageVersionsTabs {
111111

112112
const NAMED_VERSION_TOP = 100;
113113
const CHANGESET_TOP = 100;
114+
const USERS_TOP = 100;
114115

115116
const initialChangeset: Changeset = {
116117
id: "",
@@ -231,18 +232,26 @@ const ManageVersionsComponent = (props: ManageVersionsProps) => {
231232
const usersRef = React.useRef<{ [key: string]: string } | undefined>(
232233
undefined
233234
);
235+
const hasMoreUsersRef = React.useRef<boolean>(true);
234236

235-
const getUsers = React.useCallback(async () => {
236-
const users = await changesetClient.getUsers(imodelId);
237-
const userMapData = users.reduce((acc, user) => {
238-
const userFullName = [user.givenName, user.surname].filter(Boolean);
239-
acc[user.id] = userFullName.length
240-
? userFullName.join(" ")
241-
: user.displayName;
242-
return acc;
243-
}, {} as { [key: string]: string });
244-
usersRef.current = userMapData;
245-
}, [changesetClient, imodelId]);
237+
const getUsers = React.useCallback(
238+
async (skip?: number) => {
239+
const { users, hasMore } = await changesetClient.getUsers(imodelId, {
240+
top: USERS_TOP,
241+
skip,
242+
});
243+
const userMapData = users.reduce((acc, user) => {
244+
const userFullName = [user.givenName, user.surname].filter(Boolean);
245+
acc[user.id] = userFullName.length
246+
? userFullName.join(" ")
247+
: user.displayName;
248+
return acc;
249+
}, {} as { [key: string]: string });
250+
usersRef.current = { ...usersRef.current, ...userMapData };
251+
hasMoreUsersRef.current = hasMore;
252+
},
253+
[changesetClient, imodelId]
254+
);
246255

247256
React.useEffect(() => {
248257
setCurrentTab(currentTab);
@@ -446,9 +455,26 @@ const ManageVersionsComponent = (props: ManageVersionsProps) => {
446455

447456
React.useEffect(() => {
448457
const loadUsers = async () => {
449-
await getUsers();
458+
if (!hasMoreUsersRef.current) {
459+
return;
460+
}
461+
462+
if (!usersRef.current) {
463+
hasMoreUsersRef.current = false;
464+
await getUsers();
465+
} else {
466+
const hasMissingUsers =
467+
versionsTableData?.some((td) => !td.version.createdBy) ||
468+
changesets?.some((cs) => !cs.createdBy);
469+
470+
if (hasMissingUsers) {
471+
hasMoreUsersRef.current = false;
472+
await getUsers(Object.keys(usersRef.current).length);
473+
}
474+
}
450475
};
451-
if (!usersRef.current) {
476+
477+
if (!usersRef.current || versionsTableData || changesets) {
452478
loadUsers()
453479
.then(() => {
454480
const updatedVersionsTableData = versionsTableData?.map((td) => {

0 commit comments

Comments
 (0)