Skip to content

Inactivate known external links #878

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

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: 4 additions & 1 deletion src/components/ContentNode/Reference.vue
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@ export default {
name: 'Reference',
computed: {
isInternal({ url }) {
if (!url) {
return false;
}
if (!url.startsWith('/') && !url.startsWith('#')) {
// If the URL has a scheme, it's not an internal link.
return false;
Expand Down Expand Up @@ -92,7 +95,7 @@ export default {
props: {
url: {
type: String,
required: true,
required: false,
},
kind: {
type: String,
Expand Down
4 changes: 2 additions & 2 deletions src/components/ContentNode/ReferenceExternal.vue
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
-->

<template>
<a v-if="isActive" :href="url"><slot /></a>
<a v-if="url && isActive" :href="url"><slot /></a>
<span v-else><slot /></span>
</template>

Expand All @@ -19,7 +19,7 @@ export default {
props: {
url: {
type: String,
required: true,
required: false,
},
isActive: {
type: Boolean,
Expand Down
4 changes: 2 additions & 2 deletions src/components/ContentNode/ReferenceInternal.vue
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
-->

<template>
<router-link v-if="isActive" :to="url"><slot /></router-link>
<router-link v-if="url && isActive" :to="url"><slot /></router-link>
<span v-else><slot/></span>
</template>

Expand All @@ -19,7 +19,7 @@ export default {
props: {
url: {
type: String,
required: true,
required: false,
},
isActive: {
type: Boolean,
Expand Down
8 changes: 7 additions & 1 deletion src/components/Navigator/NavigatorDataProvider.vue
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
<script>
import { fetchIndexPathsData } from 'docc-render/utils/data';
import { flattenNestedData } from 'docc-render/utils/navigatorData';
import AppStore from 'docc-render/stores/AppStore';
import Language from 'docc-render/constants/Language';
/**
Expand Down Expand Up @@ -88,11 +89,16 @@ export default {
async fetchIndexData() {
try {
this.isFetching = true;
const { interfaceLanguages, references } = await fetchIndexPathsData(
const {
includedArchiveIdentifiers = [],
interfaceLanguages,
references,
} = await fetchIndexPathsData(
{ slug: this.$route.params.locale || '' },
);
this.navigationIndex = Object.freeze(interfaceLanguages);
this.navigationReferences = Object.freeze(references);
AppStore.setIncludedArchiveIdentifiers(includedArchiveIdentifiers);
} catch (e) {
this.errorFetching = true;
} finally {
Expand Down
34 changes: 33 additions & 1 deletion src/mixins/referencesProvider.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
* See https://swift.org/LICENSE.txt for license information
* See https://swift.org/CONTRIBUTORS.txt for Swift project authors
*/
import AppStore from 'docc-render/stores/AppStore';

export default {
// inject the `store`
Expand All @@ -21,8 +22,39 @@ export default {
}),
},
},
data: () => ({ appState: AppStore.state }),
computed: {
// exposes the references for the current page
references: ({ store }) => store.state.references,
references() {
const {
isFromIncludedArchive,
store: {
state: { references: originalRefs = {} },
},
} = this;
// strip the `url` key from refs if their identifier comes from an
// archive that hasn't been included by DocC
return Object.keys(originalRefs).reduce((newRefs, id) => {
const { url, ...refWithoutUrl } = originalRefs[id];
return {
...newRefs,
[id]: isFromIncludedArchive(id) ? originalRefs[id] : refWithoutUrl,
};
}, {});
},
},
methods: {
isFromIncludedArchive(id) {
const { includedArchiveIdentifiers = [] } = this.appState;
// for backwards compatibility purposes, treat all references as being
// from included archives if there is no data for it
if (!includedArchiveIdentifiers.length) {
return true;
}

return includedArchiveIdentifiers.some(archiveId => (
id?.startsWith(`doc://${archiveId}/`)
));
},
},
};
5 changes: 5 additions & 0 deletions src/stores/AppStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,15 @@ export default {
supportsAutoColorScheme,
systemColorScheme: ColorScheme.light,
availableLocales: [],
includedArchiveIdentifiers: [],
},
reset() {
this.state.imageLoadingStrategy = process.env.VUE_APP_TARGET === 'ide'
? ImageLoadingStrategy.eager : ImageLoadingStrategy.lazy;
this.state.preferredColorScheme = Settings.preferredColorScheme || defaultColorScheme;
this.state.supportsAutoColorScheme = supportsAutoColorScheme;
this.state.systemColorScheme = ColorScheme.light;
this.state.includedArchiveIdentifiers = [];
},
setImageLoadingStrategy(strategy) {
this.state.imageLoadingStrategy = strategy;
Expand All @@ -59,6 +61,9 @@ export default {
setSystemColorScheme(value) {
this.state.systemColorScheme = value;
},
setIncludedArchiveIdentifiers(value) {
this.state.includedArchiveIdentifiers = value;
},
syncPreferredColorScheme() {
if (!!Settings.preferredColorScheme
&& Settings.preferredColorScheme !== this.state.preferredColorScheme) {
Expand Down
19 changes: 19 additions & 0 deletions tests/unit/components/Navigator/NavigatorDataProvider.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

import NavigatorDataProvider from '@/components/Navigator/NavigatorDataProvider.vue';
import { shallowMount } from '@vue/test-utils';
import AppStore from 'docc-render/stores/AppStore';
import Language from 'docc-render/constants/Language';
import { TopicTypes } from '@/constants/TopicTypes';
import { fetchIndexPathsData } from '@/utils/data';
Expand Down Expand Up @@ -127,7 +128,13 @@ const references = {
foo: { bar: 'bar' },
};

const includedArchiveIdentifiers = [
'foo',
'bar',
];

const response = {
includedArchiveIdentifiers,
interfaceLanguages: {
[Language.swift.key.url]: [
swiftIndexOne,
Expand Down Expand Up @@ -174,6 +181,10 @@ describe('NavigatorDataProvider', () => {
jest.clearAllMocks();
});

afterEach(() => {
AppStore.reset();
});

it('fetches data when mounting NavigatorDataProvider', async () => {
expect(fetchIndexPathsData).toHaveBeenCalledTimes(0);
createWrapper();
Expand Down Expand Up @@ -552,4 +563,12 @@ describe('NavigatorDataProvider', () => {
},
]);
});

it('sets `includedArchiveIdentifiers` state in the app store', async () => {
expect(AppStore.state.includedArchiveIdentifiers).toEqual([]);
fetchIndexPathsData.mockResolvedValue(response);
createWrapper();
await flushPromises();
expect(AppStore.state.includedArchiveIdentifiers).toEqual(includedArchiveIdentifiers);
});
});
121 changes: 121 additions & 0 deletions tests/unit/mixins/referencesProvider.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
/**
* This source file is part of the Swift.org open source project
*
* Copyright (c) 2024 Apple Inc. and the Swift project authors
* Licensed under Apache License v2.0 with Runtime Library Exception
*
* See https://swift.org/LICENSE.txt for license information
* See https://swift.org/CONTRIBUTORS.txt for Swift project authors
*/
import { shallowMount } from '@vue/test-utils';
import referencesProvider from 'docc-render/mixins/referencesProvider';

const FakeComponentInner = {
name: 'FakeComponentInner',
props: ['references'],
render() {
return null;
},
};

const FakeComponentOuter = {
name: 'FakeComponentOuter',
mixins: [referencesProvider],
render(createElement) {
return createElement(FakeComponentInner, {
props: {
references: this.references,
},
});
},
};

const aa = {
identifier: 'doc://A/documentation/A/a',
url: '/documentation/A/a',
title: 'A.A',
};
const ab = {
identifier: 'doc://A/documentation/A/b',
url: '/documentation/A/b',
title: 'A.B',
};
const bb = {
identifier: 'doc://B/documentation/B/b',
url: '/documentation/B/b',
title: 'B.B',
};
const bbb = {
identifier: 'doc://BB/documentation/BB/b',
url: '/documentation/BB/b',
title: 'BB.B',
};

const references = {
[aa.identifier]: aa,
[ab.identifier]: ab,
[bb.identifier]: bb,
[bbb.identifier]: bbb,
};

const provide = {
store: {
state: { references },
},
};

const createOuter = (opts = { provide }) => shallowMount(FakeComponentOuter, opts);

describe('referencesProvider', () => {
it('provides a store with a default state', () => {
const outer = createOuter({});
const inner = outer.find(FakeComponentInner);
expect(inner.exists()).toBe(true);
expect(inner.props('references')).toEqual({});
});

it('provides references from a store', () => {
const outer = createOuter();
const inner = outer.find(FakeComponentInner);
expect(inner.exists()).toBe(true);
expect(inner.props('references')).toEqual(references);
});

it('removes `url` data for refs with non-empty `includedArchiveIdentifiers` app state', () => {
// empty `includedArchiveIdentifiers` — no changes to refs
const outer = createOuter();
let inner = outer.find(FakeComponentInner);
expect(inner.exists()).toBe(true);
expect(inner.props('references')).toEqual(references);

// `includedArchiveIdentifiers` contains all refs - no changes to refs
outer.setData({
appState: {
includedArchiveIdentifiers: ['A', 'B', 'BB'],
},
});
inner = outer.find(FakeComponentInner);
expect(inner.exists()).toBe(true);
expect(inner.props('references')).toEqual(references);

// `includedArchiveIdentifiers` only contains archive B — remove `url` field
// from all non-B refs
outer.setData({
appState: {
includedArchiveIdentifiers: ['B'],
},
});
inner = outer.find(FakeComponentInner);
expect(inner.exists()).toBe(true);
const refs3 = inner.props('references');
expect(refs3).not.toEqual(references);
expect(refs3[aa.identifier].title).toBe(aa.title);
expect(refs3[aa.identifier].url).toBeFalsy(); // aa `url` is gone now
expect(refs3[ab.identifier].title).toBe(ab.title);
expect(refs3[ab.identifier].url).toBeFalsy(); // ab `url` is gone now
expect(refs3[bb.identifier].title).toBe(bb.title);
expect(refs3[bb.identifier].url).toBe(bb.url); // bb still has `url`
expect(refs3[bbb.identifier].title).toBe(bbb.title);
expect(refs3[bbb.identifier].url).toBeFalsy(); // bbb `url` is gone now
});
});
12 changes: 12 additions & 0 deletions tests/unit/stores/AppStore.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ describe('AppStore', () => {
systemColorScheme: ColorScheme.light,
preferredLocale: null,
availableLocales: [],
includedArchiveIdentifiers: [],
});
});

Expand All @@ -37,6 +38,7 @@ describe('AppStore', () => {
systemColorScheme: ColorScheme.light,
preferredLocale: null,
availableLocales: [],
includedArchiveIdentifiers: [],
});

// restore target
Expand Down Expand Up @@ -72,11 +74,20 @@ describe('AppStore', () => {
});
});

describe('setIncludedArchiveIdentifiers', () => {
it('sets the included archive identifiers', () => {
const includedArchiveIdentifiers = ['foo', 'bar'];
AppStore.setIncludedArchiveIdentifiers(includedArchiveIdentifiers);
expect(AppStore.state.includedArchiveIdentifiers).toEqual(includedArchiveIdentifiers);
});
});

it('resets the state', () => {
AppStore.setImageLoadingStrategy(ImageLoadingStrategy.eager);
AppStore.setPreferredColorScheme(ColorScheme.auto);
AppStore.setSystemColorScheme(ColorScheme.dark);
AppStore.syncPreferredColorScheme();
AppStore.setIncludedArchiveIdentifiers(['a']);
AppStore.reset();

expect(AppStore.state).toEqual({
Expand All @@ -86,6 +97,7 @@ describe('AppStore', () => {
systemColorScheme: ColorScheme.light,
preferredLocale: null,
availableLocales: [],
includedArchiveIdentifiers: [],
});
});
});