Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
5 changes: 5 additions & 0 deletions .changeset/lazy-donkeys-shake.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'tinacms': patch
---

Skip TinaCloud identity requests when no auth token is stored. Logged-out admin loads no longer produce misleading 401/CORS console errors; a clear console message now points at the login popup console instead. Also fixes an unawaited auth guard in GetDocument, stops the document view from loading forever when that guard rejects the request, and fixes an unhandled promise rejection when the project settings request fails.
5 changes: 5 additions & 0 deletions .changeset/tall-jars-smile.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'tinacms': patch
---

Fix the collection list and collection search hanging on the loading screen when the session check says the user is not signed in. Both now settle and render instead of spinning until the page is reloaded.
108 changes: 108 additions & 0 deletions packages/tinacms/src/admin/components/GetCollection.hooks.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
import { renderHook, waitFor } from '@testing-library/react';
import type { TinaCMS } from '@tinacms/toolkit';
import { afterEach, describe, expect, it, vi } from 'vitest';
import { TinaAdminApi } from '../api';
import { useGetCollection, useSearchCollection } from './GetCollection';

const loadedFolder = { loading: false, fullyQualifiedName: '' };

const buildCms = (isAuthenticated: boolean, searchResults = []) =>
({
api: {
tina: {
schema: { getCollection: () => ({ name: 'post', fields: [] }) },
authProvider: {
isAuthenticated: vi.fn().mockResolvedValue(isAuthenticated),
},
},
search: {
supportsClientSideIndexing: () => false,
query: vi.fn().mockResolvedValue({ results: searchResults }),
},
},
alerts: { error: vi.fn() },
}) as unknown as TinaCMS;

describe('useGetCollection', () => {
afterEach(() => {
vi.restoreAllMocks();
});

it('fetches the collection when authenticated', async () => {
const fetchCollection = vi
.spyOn(TinaAdminApi.prototype, 'fetchCollection')
.mockResolvedValue({ name: 'post' });

const cms = buildCms(true);

const { result } = renderHook(() =>
useGetCollection(cms, 'post', true, loadedFolder)
);

await waitFor(() => expect(result.current.loading).toBe(false));
expect(fetchCollection).toHaveBeenCalled();
expect(result.current.collection).toEqual({ name: 'post' });
});

it('skips the request and stops loading when not authenticated', async () => {
const fetchCollection = vi.spyOn(TinaAdminApi.prototype, 'fetchCollection');

const cms = buildCms(false);

const { result } = renderHook(() =>
useGetCollection(cms, 'post', true, loadedFolder)
);

await waitFor(() => expect(result.current.loading).toBe(false));
expect(fetchCollection).not.toHaveBeenCalled();
expect(result.current.collection).toBeUndefined();
});

it('keeps loading while the folder is still resolving', async () => {
const fetchCollection = vi.spyOn(TinaAdminApi.prototype, 'fetchCollection');

const cms = buildCms(true);
const pendingFolder = { loading: true, fullyQualifiedName: '' };

const { result } = renderHook(() =>
useGetCollection(cms, 'post', true, pendingFolder)
);

await waitFor(() => expect(fetchCollection).not.toHaveBeenCalled());
expect(result.current.loading).toBe(true);
});

it('surfaces a fetch failure instead of loading forever', async () => {
vi.spyOn(TinaAdminApi.prototype, 'fetchCollection').mockRejectedValue(
new Error('boom')
);
vi.spyOn(console, 'error').mockImplementation(() => {});

const cms = buildCms(true);

const { result } = renderHook(() =>
useGetCollection(cms, 'post', true, loadedFolder)
);

await waitFor(() => expect(result.current.loading).toBe(false));
expect(result.current.error).toEqual(new Error('boom'));
});
});

describe('useSearchCollection', () => {
afterEach(() => {
vi.restoreAllMocks();
});

it('skips the search and stops loading when not authenticated', async () => {
const cms = buildCms(false);

const { result } = renderHook(() =>
useSearchCollection(cms, 'post', true, loadedFolder, '', 'hello')
);

await waitFor(() => expect(result.current.loading).toBe(false));
expect(cms.api.search.query).not.toHaveBeenCalled();
expect(result.current.collection).toBeUndefined();
});
});
56 changes: 34 additions & 22 deletions packages/tinacms/src/admin/components/GetCollection.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -72,12 +72,16 @@ export const useGetCollection = (
let cancelled = false;

const fetchCollection = async () => {
if ((await api.isAuthenticated()) && !folder.loading && !cancelled) {
const { name, order } = JSON.parse(sortKey || '{}');
const validSortKey = isValidSortKey(name, collectionExtra)
? name
: undefined;
try {
// the effect re-runs once the folder resolves
if (folder.loading) {
return;
}
try {
if ((await api.isAuthenticated()) && !cancelled) {
const { name, order } = JSON.parse(sortKey || '{}');
const validSortKey = isValidSortKey(name, collectionExtra)
? name
: undefined;
const collection = await api.fetchCollection(
collectionName,
includeDocuments,
Expand All @@ -88,15 +92,17 @@ export const useGetCollection = (
filterArgs
);
setCollection(collection);
} catch (error) {
cms.alerts.error(
`[${error.name}] GetCollection failed: ${error.message}`
);
console.error(error);
setCollection(undefined);
setError(error);
}
} catch (error) {
cms.alerts.error(
`[${error.name}] GetCollection failed: ${error.message}`
);
console.error(error);
setCollection(undefined);
setError(error);
}

if (!cancelled) {
setLoading(false);
}
};
Expand Down Expand Up @@ -152,8 +158,12 @@ export const useSearchCollection = (
let cancelled = false;

const searchCollection = async () => {
if ((await api.isAuthenticated()) && !folder.loading && !cancelled) {
try {
// the effect re-runs once the folder resolves
if (folder.loading) {
return;
}
try {
if ((await api.isAuthenticated()) && !cancelled) {
const response = (await cms.api.search.query(search, {
limit: 15,
cursor: after,
Expand Down Expand Up @@ -203,15 +213,17 @@ export const useSearchCollection = (
};

setCollection(collectionData);
} catch (error) {
cms.alerts.error(
`[${error.name}] GetCollection failed: ${error.message}`
);
console.error(error);
setCollection(undefined);
setError(error);
}
} catch (error) {
cms.alerts.error(
`[${error.name}] GetCollection failed: ${error.message}`
);
console.error(error);
setCollection(undefined);
setError(error);
}

if (!cancelled) {
setLoading(false);
}
};
Expand Down
62 changes: 62 additions & 0 deletions packages/tinacms/src/admin/components/GetDocument.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
import { renderHook, waitFor } from '@testing-library/react';
import type { TinaCMS } from '@tinacms/toolkit';
import { afterEach, describe, expect, it, vi } from 'vitest';
import { TinaAdminApi } from '../api';
import { useGetDocument } from './GetDocument';

const buildCms = (isAuthenticated: boolean) =>
({
api: {
tina: {
schema: {},
authProvider: {
isAuthenticated: vi.fn().mockResolvedValue(isAuthenticated),
},
},
},
alerts: { error: vi.fn() },
}) as unknown as TinaCMS;

const renderGetDocument = (cms: TinaCMS) =>
renderHook(() => useGetDocument(cms, 'post', 'hello.mdx'));

describe('useGetDocument', () => {
afterEach(() => {
vi.restoreAllMocks();
});

it('fetches the document when authenticated', async () => {
const fetchDocument = vi
.spyOn(TinaAdminApi.prototype, 'fetchDocument')
.mockResolvedValue({ document: { _values: { title: 'Hello' } } });

const { result } = renderGetDocument(buildCms(true));

await waitFor(() => expect(result.current.loading).toBe(false));
expect(fetchDocument).toHaveBeenCalledWith('post', 'hello.mdx');
expect(result.current.document).toEqual({ _values: { title: 'Hello' } });
expect(result.current.error).toBeUndefined();
});

it('skips the request and stops loading when not authenticated', async () => {
const fetchDocument = vi.spyOn(TinaAdminApi.prototype, 'fetchDocument');

const { result } = renderGetDocument(buildCms(false));

await waitFor(() => expect(result.current.loading).toBe(false));
expect(fetchDocument).not.toHaveBeenCalled();
expect(result.current.document).toBeUndefined();
});

it('surfaces a fetch failure instead of loading forever', async () => {
vi.spyOn(TinaAdminApi.prototype, 'fetchDocument').mockRejectedValue(
new Error('boom')
);
vi.spyOn(console, 'error').mockImplementation(() => {});

const { result } = renderGetDocument(buildCms(true));

await waitFor(() => expect(result.current.loading).toBe(false));
expect(result.current.error).toEqual(new Error('boom'));
});
});
28 changes: 14 additions & 14 deletions packages/tinacms/src/admin/components/GetDocument.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ export const useGetDocument = (
let isCancelled = false; // Add cancellation flag

const fetchDocument = async () => {
if (api.isAuthenticated() && !isCancelled) {
try {
try {
if ((await api.isAuthenticated()) && !isCancelled) {
const response = await api.fetchDocument(
collectionName,
relativePath
Expand All @@ -34,22 +34,22 @@ export const useGetDocument = (
if (!isCancelled) {
setDocument(response.document);
}
} catch (error) {
// Only handle error if the request hasn't been cancelled
if (!isCancelled) {
cms.alerts.error(
`[${error.name}] GetDocument failed: ${error.message}`
);
console.error(error);
setDocument(undefined);
setError(error);
}
}

} catch (error) {
// Only handle error if the request hasn't been cancelled
if (!isCancelled) {
setLoading(false);
cms.alerts.error(
`[${error.name}] GetDocument failed: ${error.message}`
);
console.error(error);
setDocument(undefined);
setError(error);
}
}

if (!isCancelled) {
setLoading(false);
}
};

setLoading(true);
Expand Down
13 changes: 10 additions & 3 deletions packages/tinacms/src/auth/TinaCloudProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -523,8 +523,13 @@ export const TinaCloudProvider = (
}, []);

React.useEffect(() => {
const setupEditorialWorkflow = () => {
client.getProject().then(async (project) => {
const setupEditorialWorkflow = async () => {
try {
const token = await client.authProvider.getToken();
if (!token?.access_token && !token?.id_token) {
return;
}
const project = await client.getProject();
if (project?.features?.includes('editorial-workflow')) {
cms.flags.set('branch-switcher', true);
client.usingEditorialWorkflow = true;
Expand All @@ -536,7 +541,9 @@ export const TinaCloudProvider = (
setCurrentBranch(project.defaultBranch || 'main');
}
}
});
} catch (e) {
console.error('TinaCMS: unable to load project settings', e);
}
};
if (isTinaCloud) {
setupEditorialWorkflow();
Expand Down
Loading
Loading