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.

Sign up for GitHub

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

UI: Fix controls and parameters on tag-filtered stories #30038

Merged
merged 1 commit into from
Dec 13, 2024
Merged

UI: Fix controls and parameters on tag-filtered stories #30038

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions code/core/src/manager-api/lib/stories.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,7 @@ export const transformStoryIndexToStoriesHash = (
.reduce(addItem, orphanHash);
};

/** Now we need to patch in the existing prepared stories */
export const addPreparedStories = (newHash: API_IndexHash, oldHash?: API_IndexHash) => {
if (!oldHash) {
return newHash;
Expand Down
26 changes: 22 additions & 4 deletions code/core/src/manager-api/modules/refs.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,15 @@ export const init: ModuleFn<SubAPI, SubState> = (
},
changeRefVersion: async (id, url) => {
const { versions, title } = api.getRefs()[id];
const ref: API_SetRefData = { id, url, versions, title, index: {}, expanded: true };
const ref: API_SetRefData = {
id,
url,
versions,
title,
index: {},
filteredIndex: {},
expanded: true,
};

await api.setRef(id, { ...ref, type: 'unknown' }, false);
await api.checkRef(ref);
Expand Down Expand Up @@ -292,6 +300,7 @@ export const init: ModuleFn<SubAPI, SubState> = (
// eslint-disable-next-line @typescript-eslint/naming-convention
let internal_index: StoryIndex | undefined;
let index: API_IndexHash | undefined;
let filteredIndex: API_IndexHash | undefined;
const { filters } = store.getState();
const { storyMapper = defaultStoryMapper } = provider.getConfig();
const ref = api.getRefs()[id];
Expand All @@ -304,19 +313,28 @@ export const init: ModuleFn<SubAPI, SubState> = (
: storyIndex;

// @ts-expect-error (could be undefined)
index = transformStoryIndexToStoriesHash(storyIndex, {
filteredIndex = transformStoryIndexToStoriesHash(storyIndex, {
provider,
docsOptions,
filters,
status: {},
});
// @ts-expect-error (could be undefined)
index = transformStoryIndexToStoriesHash(storyIndex, {
provider,
docsOptions,
filters: {},
status: {},
});
}

if (index) {
index = addRefIds(index, ref);
}

await api.updateRef(id, { ...ref, ...rest, index, internal_index });
if (filteredIndex) {
filteredIndex = addRefIds(filteredIndex, ref);
}
await api.updateRef(id, { ...ref, ...rest, index, filteredIndex, internal_index });
},

updateRef: async (id, data) => {
Expand Down
79 changes: 55 additions & 24 deletions code/core/src/manager-api/modules/stories.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -568,41 +568,61 @@ export const init: ModuleFn<SubAPI, SubState> = ({
// The story index we receive on fetchStoryIndex is not, but all the prepared fields are optional
// so we can cast one to the other easily enough
setIndex: async (input) => {
const { index: oldHash, status, filters } = store.getState();
const newHash = transformStoryIndexToStoriesHash(input, {
const { filteredIndex: oldFilteredHash, index: oldHash, status, filters } = store.getState();
const newFilteredHash = transformStoryIndexToStoriesHash(input, {
provider,
docsOptions,
status,
filters,
});
const newHash = transformStoryIndexToStoriesHash(input, {
provider,
docsOptions,
status,
filters: {},
});

// Now we need to patch in the existing prepared stories
const output = addPreparedStories(newHash, oldHash);

await store.setState({ internal_index: input, index: output, indexError: undefined });
await store.setState({
internal_index: input,
filteredIndex: addPreparedStories(newFilteredHash, oldFilteredHash),
index: addPreparedStories(newHash, oldHash),
indexError: undefined,
});
},
// FIXME: is there a bug where filtered stories get added back in on updateStory???
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: This FIXME comment identifies a real issue - filtered stories could get re-added during updates. Consider adding a check to prevent this.

updateStory: async (
storyId: StoryId,
update: StoryUpdate,
ref?: API_ComposedRef
): Promise<void> => {
if (!ref) {
const { index } = store.getState();
if (!index) {
return;
const { index, filteredIndex } = store.getState();
if (index) {
index[storyId] = {
...index[storyId],
...update,
} as API_StoryEntry;
}
if (filteredIndex) {
filteredIndex[storyId] = {
...filteredIndex[storyId],
...update,
} as API_StoryEntry;
}
if (index || filteredIndex) {
await store.setState({ index, filteredIndex });
}
shilman marked this conversation as resolved.
Show resolved Hide resolved
} else {
const { id: refId, index, filteredIndex }: any = ref;
index[storyId] = {
...index[storyId],
...update,
} as API_StoryEntry;
await store.setState({ index });
} else {
const { id: refId, index }: any = ref;
index[storyId] = {
...index[storyId],
filteredIndex[storyId] = {
...filteredIndex[storyId],
...update,
} as API_StoryEntry;
await fullAPI.updateRef(refId, { index });
await fullAPI.updateRef(refId, { index, filteredIndex });
}
},
updateDocs: async (
Expand All @@ -611,22 +631,33 @@ export const init: ModuleFn<SubAPI, SubState> = ({
ref?: API_ComposedRef
): Promise<void> => {
if (!ref) {
const { index } = store.getState();
if (!index) {
return;
const { index, filteredIndex } = store.getState();
if (index) {
index[docsId] = {
...index[docsId],
...update,
} as API_DocsEntry;
}
if (filteredIndex) {
filteredIndex[docsId] = {
...filteredIndex[docsId],
...update,
} as API_DocsEntry;
}
if (index || filteredIndex) {
await store.setState({ index, filteredIndex });
}
} else {
const { id: refId, index, filteredIndex }: any = ref;
index[docsId] = {
...index[docsId],
...update,
} as API_DocsEntry;
await store.setState({ index });
} else {
const { id: refId, index }: any = ref;
index[docsId] = {
...index[docsId],
filteredIndex[docsId] = {
...filteredIndex[docsId],
...update,
} as API_DocsEntry;
await fullAPI.updateRef(refId, { index });
await fullAPI.updateRef(refId, { index, filteredIndex });
}
},
setPreviewInitialized: async (ref) => {
Expand Down
28 changes: 20 additions & 8 deletions code/core/src/manager-api/tests/refs.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,7 @@ describe('Refs API', () => {
{
"refs": {
"fake": {
"filteredIndex": undefined,
"id": "fake",
"index": undefined,
"indexError": {
Expand Down Expand Up @@ -360,6 +361,7 @@ describe('Refs API', () => {
{
"refs": {
"fake": {
"filteredIndex": undefined,
"id": "fake",
"index": undefined,
"indexError": {
Expand Down Expand Up @@ -504,6 +506,7 @@ describe('Refs API', () => {
{
"refs": {
"fake": {
"filteredIndex": {},
"id": "fake",
"index": {},
"internal_index": {
Expand All @@ -522,6 +525,7 @@ describe('Refs API', () => {
{
"refs": {
"fake": {
"filteredIndex": {},
"id": "fake",
"index": {},
"internal_index": {
Expand Down Expand Up @@ -601,6 +605,7 @@ describe('Refs API', () => {
{
"refs": {
"fake": {
"filteredIndex": {},
"id": "fake",
"index": {},
"internal_index": {
Expand Down Expand Up @@ -682,6 +687,7 @@ describe('Refs API', () => {
{
"refs": {
"fake": {
"filteredIndex": {},
"id": "fake",
"index": {},
"internal_index": {
Expand Down Expand Up @@ -763,6 +769,7 @@ describe('Refs API', () => {
{
"refs": {
"fake": {
"filteredIndex": undefined,
"id": "fake",
"index": undefined,
"internal_index": undefined,
Expand Down Expand Up @@ -905,6 +912,7 @@ describe('Refs API', () => {
{
"refs": {
"fake": {
"filteredIndex": undefined,
"id": "fake",
"index": undefined,
"internal_index": undefined,
Expand Down Expand Up @@ -987,6 +995,7 @@ describe('Refs API', () => {
{
"refs": {
"fake": {
"filteredIndex": {},
"id": "fake",
"index": {},
"internal_index": {
Expand Down Expand Up @@ -1068,6 +1077,7 @@ describe('Refs API', () => {
{
"refs": {
"fake": {
"filteredIndex": {},
"id": "fake",
"index": {},
"internal_index": {
Expand Down Expand Up @@ -1227,18 +1237,20 @@ describe('Refs API', () => {
},
};

const transformOptions = {
provider: provider as any,
docsOptions: {},
filters: {},
status: {},
};
const initialState: Partial<State> = {
refs: {
fake: {
id: 'fake',
url: 'https://example.com',
previewInitialized: true,
index: transformStoryIndexToStoriesHash(index, {
provider: provider as any,
docsOptions: {},
filters: {},
status: {},
}),
index: transformStoryIndexToStoriesHash(index, transformOptions),
filteredIndex: transformStoryIndexToStoriesHash(index, transformOptions),
internal_index: index,
},
},
Expand All @@ -1261,10 +1273,10 @@ describe('Refs API', () => {

await api.setRef('fake', { storyIndex: index });

await expect(api.getRefs().fake.index).toEqual(
await expect(api.getRefs().fake.filteredIndex).toEqual(
expect.objectContaining({ 'a--1': expect.anything() })
);
await expect(api.getRefs().fake.index).not.toEqual(
await expect(api.getRefs().fake.filteredIndex).not.toEqual(
expect.objectContaining({ 'a--2': expect.anything() })
);
});
Expand Down
20 changes: 13 additions & 7 deletions code/core/src/manager-api/tests/stories.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -765,10 +765,15 @@ describe('stories API', () => {
source: '',
sourceLocation: '',
type: '',
ref: { id: 'refId', index: { 'a--1': { args: { a: 'b' } } } } as any,
ref: {
id: 'refId',
index: { 'a--1': { args: { a: 'b' } } },
filteredIndex: { 'a--1': { args: { a: 'b' } } },
} as any,
});
provider.channel.emit(STORY_ARGS_UPDATED, { storyId: 'a--1', args: { foo: 'bar' } });
expect(fullAPI.updateRef).toHaveBeenCalledWith('refId', {
filteredIndex: { 'a--1': { args: { foo: 'bar' } } },
index: { 'a--1': { args: { foo: 'bar' } } },
});
});
Expand Down Expand Up @@ -1539,6 +1544,7 @@ describe('stories API', () => {
})
);
});

it('updates state', async () => {
const moduleArgs = createMockModuleArgs({});
const { api } = initStories(moduleArgs as unknown as ModuleArgs);
Expand All @@ -1565,9 +1571,9 @@ describe('stories API', () => {
await api.setIndex({ v: 5, entries: navigationEntries });
await api.experimental_setFilter('myCustomFilter', (item: any) => item.id.startsWith('a'));

const { index } = store.getState();
const { filteredIndex } = store.getState();

expect(index).toMatchInlineSnapshot(`
expect(filteredIndex).toMatchInlineSnapshot(`
{
"a": {
"children": [
Expand Down Expand Up @@ -1624,7 +1630,7 @@ describe('stories API', () => {
);

// empty, because there are no stories with status
expect(store.getState().index).toMatchInlineSnapshot('{}');
expect(store.getState().filteredIndex).toMatchInlineSnapshot('{}');

// setting status should update the index
await api.experimental_updateStatus('a-addon-id', {
Expand All @@ -1636,7 +1642,7 @@ describe('stories API', () => {
'a--2': { status: 'success', title: 'a addon title', description: '' },
});

expect(store.getState().index).toMatchInlineSnapshot(`
expect(store.getState().filteredIndex).toMatchInlineSnapshot(`
{
"a": {
"children": [
Expand Down Expand Up @@ -1676,9 +1682,9 @@ describe('stories API', () => {

await api.setIndex({ v: 5, entries: navigationEntries });

const { index } = store.getState();
const { filteredIndex } = store.getState();

expect(index).toMatchInlineSnapshot(`
expect(filteredIndex).toMatchInlineSnapshot(`
{
"a": {
"children": [
Expand Down
Loading
Loading