diff --git a/config/i18n.json b/config/i18n.json index ebaa46120..94015339a 100644 --- a/config/i18n.json +++ b/config/i18n.json @@ -1503,14 +1503,15 @@ "ExternalSourcePlaceholder": "Paste wish list URL here", "Header": "Wish List", "Import": "Load Wish List Rolls", - "ImportFailed": "No wish list information found.", - "ImportError": "Error loading wish list: {{error}}", + "ImportFailed": "None of your wish lists contained any valid rolls.", + "ImportError": "Error loading wish list from \"{{url}}\": {{error}}", "ImportNoFile": "No file selected.", "InvalidExternalSource": "Please enter a valid URL for your external wish list source. The URL must start with one of the following:", "JustAnotherTeam": "Just Another Team", "LastUpdated": "Last updated: {{lastUpdatedDate}} at {{lastUpdatedTime}}", "Num": "{{num, number}} rolls in your wish list", "NumRolls": "{{num, number}} rolls", + "DupeRolls": " (+{{num, number}} ignored dupes)", "PreMadeFiles": "Use A Pre-Made Wish List", "Refresh": "Refresh Wishlist", "SourceAlreadyAdded": "Wish List already added", diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 2ec431131..2f6848367 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -1,7 +1,8 @@ ## Next * Added a wishlist refresh button in Settings to help with wishlist development. Note that GitHub can take upwards of 10min to actually reflect your changes. Refreshing won't speed up GitHub. - +* Having a broken wish list in your settings will no longer prevent removing other wish lists. + ## 8.50.1 (2024-12-12) * Notes now appear in the tooltips on item tiles. diff --git a/src/app/settings/WishListSettings.tsx b/src/app/settings/WishListSettings.tsx index 81d8fc08a..902f626aa 100644 --- a/src/app/settings/WishListSettings.tsx +++ b/src/app/settings/WishListSettings.tsx @@ -1,5 +1,6 @@ import { settingSelector } from 'app/dim-api/selectors'; import { ConfirmButton } from 'app/dim-ui/ConfirmButton'; +import ExternalLink from 'app/dim-ui/ExternalLink'; import { PressTip } from 'app/dim-ui/PressTip'; import { I18nKey, t } from 'app/i18next-t'; import { showNotification } from 'app/notifications/notifications'; @@ -32,6 +33,8 @@ export default function WishListSettings() { dispatch(fetchWishList()); }, [dispatch]); + // TODO: add a "local" source that can coexist with other sources? + const activeWishlistUrls = settingsWishListSource ? settingsWishListSource.split('|').map((url) => url.trim()) : []; @@ -46,7 +49,10 @@ export default function WishListSettings() { showNotification({ type: 'error', title: t('WishListRoll.Header'), - body: t('WishListRoll.ImportError', { error: errorMessage(e) }), + body: t('WishListRoll.ImportError', { + url: reloadWishListSource ?? '', + error: errorMessage(e), + }), }); } }; @@ -55,7 +61,7 @@ export default function WishListSettings() { const reader = new FileReader(); reader.onload = async () => { if (reader.result && typeof reader.result === 'string') { - const wishListAndInfo = toWishList([undefined, reader.result]); + const wishListAndInfo = toWishList([[undefined, reader.result]]); if (wishListAndInfo.wishListRolls.length) { dispatch(clearWishLists()); } @@ -150,10 +156,12 @@ export default function WishListSettings() { return ( changeUrl(url, checked)} /> @@ -166,6 +174,7 @@ export default function WishListSettings() { title={loadedData?.title} description={loadedData?.description} rollsCount={loadedData?.numRolls} + dupeRollsCount={loadedData?.dupeRolls} onRemove={() => changeUrl(url, false)} /> ); @@ -175,11 +184,13 @@ export default function WishListSettings() { {disabledBuiltinLists.map((list) => ( changeUrl(list.url, checked)} /> ))} @@ -198,23 +209,32 @@ export default function WishListSettings() { function BuiltinWishlist({ name, + url, title, description, rollsCount, + dupeRollsCount, checked, onChange, }: { name: I18nKey; + url: string; title: string | undefined; description: string | undefined; rollsCount: number | undefined; + dupeRollsCount: number | undefined; checked: boolean; onChange: (checked: boolean) => void; }) { return (
- {rollsCount !== undefined && t('WishListRoll.NumRolls', { num: rollsCount })} + + {rollsCount !== undefined && t('WishListRoll.NumRolls', { num: rollsCount })} + {dupeRollsCount !== undefined && + dupeRollsCount > 0 && + t('WishListRoll.DupeRolls', { num: dupeRollsCount })} + {(title || description) && (
{title} @@ -231,12 +251,14 @@ function UrlWishlist({ title, description, rollsCount, + dupeRollsCount, onRemove, }: { url: string; title: string | undefined; description: string | undefined; rollsCount: number | undefined; + dupeRollsCount: number | undefined; onRemove: () => void; }) { return ( @@ -246,7 +268,12 @@ function UrlWishlist({ {!title &&
{url}
} - {rollsCount !== undefined && t('WishListRoll.NumRolls', { num: rollsCount })} + + {rollsCount !== undefined && t('WishListRoll.NumRolls', { num: rollsCount })} + {dupeRollsCount !== undefined && + dupeRollsCount > 0 && + t('WishListRoll.DupeRolls', { num: dupeRollsCount })} + {description &&
{description}
}
); diff --git a/src/app/wishlists/types.ts b/src/app/wishlists/types.ts index 587aced37..4269bde6b 100644 --- a/src/app/wishlists/types.ts +++ b/src/app/wishlists/types.ts @@ -52,6 +52,8 @@ export interface WishListInfo { url: string | undefined; title?: string; description?: string; - /** The number of rolls from this wish list that actually made it in. */ + /** The number of rolls from this wish list that actually made it in (e.g. were valid and unique). */ numRolls: number; + /** The number of rolls in this list that were duplicates of other lists. */ + dupeRolls: number; } diff --git a/src/app/wishlists/wishlist-fetch.ts b/src/app/wishlists/wishlist-fetch.ts index 4156b2a0d..931b69204 100644 --- a/src/app/wishlists/wishlist-fetch.ts +++ b/src/app/wishlists/wishlist-fetch.ts @@ -6,6 +6,7 @@ import { setSettingAction } from 'app/settings/actions'; import { settingsReady } from 'app/settings/settings'; import { get } from 'app/storage/idb-keyval'; import { ThunkResult } from 'app/store/types'; +import { errorMessage } from 'app/utils/errors'; import { errorLog, infoLog } from 'app/utils/log'; import { once } from 'es-toolkit'; import { loadWishLists, touchWishLists } from './actions'; @@ -26,7 +27,7 @@ function hoursAgo(dateToCheck?: Date): number { } /** - * this performs both the initial fetch (after setting a new wishlist) (when arg0 exists) + * this performs both the initial fetch (after setting a new wishlist) (when newWishlistSource exists) * and subsequent fetches (checking for updates) (arg-less) */ export function fetchWishList(newWishlistSource?: string, manualRefresh?: boolean): ThunkResult { @@ -48,9 +49,6 @@ export function fetchWishList(newWishlistSource?: string, manualRefresh?: boolea return; } - // Pipe | seperated URLs - const wishlistUrlsToFetch = validateWishListURLs(wishlistToFetch); - const { lastFetched: wishListLastUpdated, wishListAndInfo: { source: loadedWishListSource, wishListRolls: loadedWishListRolls }, @@ -71,38 +69,48 @@ export function fetchWishList(newWishlistSource?: string, manualRefresh?: boolea return; } - let wishListTexts: string[]; - try { - wishListTexts = await Promise.all( - wishlistUrlsToFetch.map(async (url) => { - const res = await fetch(url); - if (res.status < 200 || res.status >= 300) { - throw await toHttpStatusError(res); - } - return res.text(); - }), - ); - - // if this is a new wishlist, set the setting now that we know it's fetchable - if (newWishlistSource) { - dispatch(setSettingAction('wishListSource', newWishlistSource)); + // Pipe | separated URLs + const wishlistUrlsToFetch = validateWishListURLs(wishlistToFetch); + const wishListResults = await Promise.allSettled( + wishlistUrlsToFetch.map(async (url) => { + const res = await fetch(url); + if (res.status < 200 || res.status >= 300) { + throw await toHttpStatusError(res); + } + return res.text(); + }), + ); + + const wishLists: [url: string, text: string][] = []; + let hasSuccess = false; + for (let i = 0; i < wishlistUrlsToFetch.length; i++) { + const url = wishlistUrlsToFetch[i]; + const result = wishListResults[i]; + if (result.status === 'rejected') { + showNotification({ + type: 'warning', + title: t('WishListRoll.Header'), + body: t('WishListRoll.ImportError', { url, error: errorMessage(result.reason) }), + }); + errorLog(TAG, 'Unable to load wish list', url, result.reason); + return; + } else if (result.status === 'fulfilled') { + hasSuccess = true; + wishLists.push([url, result.value]); } - } catch (e) { - showNotification({ - type: 'warning', - title: t('WishListRoll.Header'), - body: t('WishListRoll.ImportFailed'), - }); - errorLog(TAG, 'Unable to load wish list', e); + } + + if (!hasSuccess) { + // Give up if we couldn't fetch any of the lists return; } - const wishLists: [string, string][] = wishlistUrlsToFetch.map((url, idx) => [ - url, - wishListTexts[idx], - ]); + // if this is a new wishlist, set the setting now that we know at least one list is fetchable + if (newWishlistSource && hasSuccess) { + dispatch(setSettingAction('wishListSource', newWishlistSource)); + } - const wishListAndInfo = toWishList(...wishLists); + const wishListAndInfo = toWishList(wishLists); wishListAndInfo.source = wishlistToFetch; // Only update if the length changed. The wish list may actually be different - we don't do a deep check - diff --git a/src/app/wishlists/wishlist-file.test.ts b/src/app/wishlists/wishlist-file.test.ts index 809067697..6d2e4ac42 100644 --- a/src/app/wishlists/wishlist-file.test.ts +++ b/src/app/wishlists/wishlist-file.test.ts @@ -15,5 +15,5 @@ const cases: [wishlist: string, result: WishListRoll][] = [ ]; test.each(cases)('parse wishlist line: %s', (wishlist, result) => { - expect(toWishList([undefined, wishlist]).wishListRolls[0]).toStrictEqual(result); + expect(toWishList([[undefined, wishlist]]).wishListRolls[0]).toStrictEqual(result); }); diff --git a/src/app/wishlists/wishlist-file.ts b/src/app/wishlists/wishlist-file.ts index 7a23c7f16..3d4dc35ab 100644 --- a/src/app/wishlists/wishlist-file.ts +++ b/src/app/wishlists/wishlist-file.ts @@ -24,9 +24,7 @@ const notesLabel = '//notes:'; * one or more wish list text files, deduplicating within * and between lists. */ -export function toWishList( - ...files: [url: string | undefined, contents: string][] -): WishListAndInfo { +export function toWishList(files: [url: string | undefined, contents: string][]): WishListAndInfo { const stopTimer = timer(TAG, 'Parse wish list'); try { const wishList: WishListAndInfo = { @@ -42,9 +40,8 @@ export function toWishList( title: undefined, description: undefined, numRolls: 0, + dupeRolls: 0, }; - let dupes = 0; - let blockNotes: string | undefined = undefined; const lines = fileText.split('\n'); @@ -74,14 +71,13 @@ export function toWishList( wishList.wishListRolls.push(roll); info.numRolls++; } else { - dupes++; + info.dupeRolls++; } } } } - - if (dupes > 0) { - warnLog(TAG, 'Discarded', dupes, 'duplicate rolls from wish list', url); + if (info.dupeRolls > 0) { + warnLog(TAG, 'Discarded', info.dupeRolls, 'duplicate rolls from wish list', url); } wishList.infos.push(info); } diff --git a/src/locale/en.json b/src/locale/en.json index e1d828ec2..e12ff6170 100644 --- a/src/locale/en.json +++ b/src/locale/en.json @@ -1483,12 +1483,13 @@ "Clear": "Clear Wish List", "CopiedLine": "Wish List roll copied to clipboard", "CopyLine": "Copy Selected Perks as Wish List Roll", + "DupeRolls": " (+{{num, number}} ignored dupes)", "ExternalSource": "Add another wish list", "ExternalSourcePlaceholder": "Paste wish list URL here", "Header": "Wish List", "Import": "Load Wish List Rolls", - "ImportError": "Error loading wish list: {{error}}", - "ImportFailed": "No wish list information found.", + "ImportError": "Error loading wish list from \"{{url}}\": {{error}}", + "ImportFailed": "None of your wish lists contained any valid rolls.", "ImportNoFile": "No file selected.", "InvalidExternalSource": "Please enter a valid URL for your external wish list source. The URL must start with one of the following:", "JustAnotherTeam": "Just Another Team",