fix(opds): hide popular catalog after adding it to My Catalogs (#4782) (#4787)

Adding a built-in popular catalog (e.g. Project Gutenberg) to My Catalogs left it still rendering in the Popular Catalogs section, so it looked like a duplicate. Only the card's Add button was hidden; the card itself stayed. Filter added (and disabled) entries out of the Popular list entirely via a new pure helper getUnaddedPopularCatalogs, which matches by normalized URL (trim + lowercase) to mirror the store's findByUrl dedup. The section already auto-hides when the list is empty, so it disappears once all are added. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-25 16:12:41 +08:00
parent 99b9adfe85
commit fb943987eb
3 changed files with 107 additions and 50 deletions
@@ -56,8 +56,9 @@ import {
  MIME,
  validateOPDSURL,
  getOPDSNavLink,
+  getUnaddedPopularCatalogs,
 } from '@/app/opds/utils/opdsUtils';
-import type { OPDSBaseLink } from '@/types/opds';
+import type { OPDSBaseLink, OPDSCatalog } from '@/types/opds';
 import { fetchWithAuth } from '@/app/opds/utils/opdsReq';

 const mockFetchWithAuth = vi.mocked(fetchWithAuth);
@@ -781,4 +782,44 @@ describe('opdsUtils', () => {
      expect(getOPDSNavLink([{ type: 'application/opds+json' }])).toBeUndefined();
    });
  });
+
+  describe('getUnaddedPopularCatalogs', () => {
+    const popular: OPDSCatalog[] = [
+      { id: 'gutenberg', name: 'Project Gutenberg', url: 'https://m.gutenberg.org/ebooks.opds/' },
+      {
+        id: 'standardebooks',
+        name: 'Standard Ebooks',
+        url: 'https://standardebooks.org/feeds/opds',
+      },
+    ];
+
+    it('returns all popular catalogs when none are added', () => {
+      expect(getUnaddedPopularCatalogs(popular, [])).toEqual(popular);
+    });
+
+    it('drops a popular catalog already present in the added list', () => {
+      const added: OPDSCatalog[] = [
+        { id: '1', name: 'Project Gutenberg', url: 'https://m.gutenberg.org/ebooks.opds/' },
+      ];
+      const result = getUnaddedPopularCatalogs(popular, added);
+      expect(result.map((c) => c.id)).toEqual(['standardebooks']);
+    });
+
+    it('matches by normalized URL (case and surrounding whitespace)', () => {
+      const added: OPDSCatalog[] = [
+        { id: '1', name: 'Gutenberg', url: '  HTTPS://M.GUTENBERG.ORG/EBOOKS.OPDS/  ' },
+      ];
+      const result = getUnaddedPopularCatalogs(popular, added);
+      expect(result.map((c) => c.id)).toEqual(['standardebooks']);
+    });
+
+    it('excludes disabled popular catalogs', () => {
+      const withDisabled: OPDSCatalog[] = [
+        ...popular,
+        { id: 'manybooks', name: 'ManyBooks', url: 'https://manybooks.net/opds/', disabled: true },
+      ];
+      const result = getUnaddedPopularCatalogs(withDisabled, []);
+      expect(result.map((c) => c.id)).toEqual(['gutenberg', 'standardebooks']);
+    });
+  });
 });
@@ -29,7 +29,7 @@ import { eventDispatcher } from '@/utils/event';
 import { SectionTitle } from '@/components/settings/primitives';
 import { deleteSubscriptionState, loadSubscriptionState } from '@/services/opds';
 import type { OPDSSubscriptionState } from '@/services/opds/types';
-import { validateOPDSURL } from '../utils/opdsUtils';
+import { getUnaddedPopularCatalogs, validateOPDSURL } from '../utils/opdsUtils';
 import { FailedDownloadsDialog } from './FailedDownloadsDialog';
 import {
  formatOPDSCustomHeadersInput,
@@ -129,7 +129,11 @@ export function CatalogManager({ inSubPage = false }: CatalogManagerProps = {})
  const [headerError, setHeaderError] = useState('');
  const [proxyConsentError, setProxyConsentError] = useState('');
  const [isValidating, setIsValidating] = useState(false);
-  const popularCatalogs = appService?.isOnlineCatalogsAccessible ? POPULAR_CATALOGS : [];
+  // Only surface popular catalogs the user hasn't already added; otherwise an
+  // added entry would render in both sections and read as a duplicate (#4782).
+  const popularCatalogs = appService?.isOnlineCatalogsAccessible
+    ? getUnaddedPopularCatalogs(POPULAR_CATALOGS, catalogs)
+    : [];
  const [subscriptionStates, setSubscriptionStates] = useState<
    Record<string, OPDSSubscriptionState>
  >({});
@@ -593,53 +597,46 @@ export function CatalogManager({ inSubPage = false }: CatalogManagerProps = {})
      <section className={clsx('text-base', popularCatalogs.length === 0 && 'hidden')}>
        <SectionTitle className='mb-3'>{_('Popular Catalogs')}</SectionTitle>
        <div className='grid grid-cols-1 gap-3 sm:grid-cols-2'>
-          {popularCatalogs
-            .filter((catalog) => !catalog.disabled)
-            .map((catalog) => {
-              const isAdded = catalogs.some((c) => c.url === catalog.url);
-              return (
-                <div
-                  key={catalog.id}
-                  className='card eink-bordered bg-base-100 border-base-200 flex flex-col border'
-                >
-                  <div className='flex flex-1 flex-col gap-2.5 p-4'>
-                    <h4>
-                      <button
-                        type='button'
-                        onClick={() => handleOpenCatalog(catalog)}
-                        className='flex w-full min-w-0 items-center gap-1.5 rounded-sm text-start text-sm font-semibold transition-colors duration-150 hover:underline focus-visible:underline focus-visible:outline-none'
-                      >
-                        {catalog.icon && <span className='flex-shrink-0'>{catalog.icon}</span>}
-                        <span className='truncate'>{catalog.name}</span>
-                      </button>
-                    </h4>
-                    {catalog.description && (
-                      <p className='text-base-content/70 line-clamp-2 text-xs leading-relaxed'>
-                        {catalog.description}
-                      </p>
-                    )}
-                    <div className='border-base-200 mt-auto flex items-center justify-end gap-1 border-t pt-3'>
-                      {!isAdded && (
-                        <button
-                          onClick={() => handleAddPopularCatalog(catalog)}
-                          className='hover:bg-base-200 focus-visible:ring-base-content/15 inline-flex items-center gap-1 rounded-md px-2 py-1 text-xs font-medium transition-colors duration-150 focus-visible:outline-none focus-visible:ring-2'
-                        >
-                          <IoAdd className='h-4 w-4' />
-                          {_('Add')}
-                        </button>
-                      )}
-                      <button
-                        onClick={() => handleOpenCatalog(catalog)}
-                        className='hover:bg-base-200 focus-visible:ring-base-content/15 inline-flex items-center gap-0.5 rounded-md px-2 py-1 text-xs font-medium transition-colors duration-150 focus-visible:outline-none focus-visible:ring-2'
-                      >
-                        {_('Browse')}
-                        <MdChevronRight className='h-4 w-4' />
-                      </button>
-                    </div>
-                  </div>
+          {popularCatalogs.map((catalog) => (
+            <div
+              key={catalog.id}
+              className='card eink-bordered bg-base-100 border-base-200 flex flex-col border'
+            >
+              <div className='flex flex-1 flex-col gap-2.5 p-4'>
+                <h4>
+                  <button
+                    type='button'
+                    onClick={() => handleOpenCatalog(catalog)}
+                    className='flex w-full min-w-0 items-center gap-1.5 rounded-sm text-start text-sm font-semibold transition-colors duration-150 hover:underline focus-visible:underline focus-visible:outline-none'
+                  >
+                    {catalog.icon && <span className='flex-shrink-0'>{catalog.icon}</span>}
+                    <span className='truncate'>{catalog.name}</span>
+                  </button>
+                </h4>
+                {catalog.description && (
+                  <p className='text-base-content/70 line-clamp-2 text-xs leading-relaxed'>
+                    {catalog.description}
+                  </p>
+                )}
+                <div className='border-base-200 mt-auto flex items-center justify-end gap-1 border-t pt-3'>
+                  <button
+                    onClick={() => handleAddPopularCatalog(catalog)}
+                    className='hover:bg-base-200 focus-visible:ring-base-content/15 inline-flex items-center gap-1 rounded-md px-2 py-1 text-xs font-medium transition-colors duration-150 focus-visible:outline-none focus-visible:ring-2'
+                  >
+                    <IoAdd className='h-4 w-4' />
+                    {_('Add')}
+                  </button>
+                  <button
+                    onClick={() => handleOpenCatalog(catalog)}
+                    className='hover:bg-base-200 focus-visible:ring-base-content/15 inline-flex items-center gap-0.5 rounded-md px-2 py-1 text-xs font-medium transition-colors duration-150 focus-visible:outline-none focus-visible:ring-2'
+                  >
+                    {_('Browse')}
+                    <MdChevronRight className='h-4 w-4' />
+                  </button>
                </div>
-              );
-            })}
+              </div>
+            </div>
+          ))}
        </div>
      </section>

@@ -1,6 +1,6 @@
 import { isOPDSCatalog } from 'foliate-js/opds.js';
 import { replace as expandURITemplate, getVariables } from 'foliate-js/uri-template.js';
-import { OPDSBaseLink } from '@/types/opds';
+import { OPDSBaseLink, OPDSCatalog } from '@/types/opds';
 import { EXTS } from '@/libs/document';
 import { fetchWithAuth } from './opdsReq';

@@ -319,6 +319,25 @@ export const validateOPDSURL = async (
  }
 };

+/**
+ * Filter the built-in "popular" OPDS catalogs down to those the user hasn't
+ * already added to their personal list. Matching is by normalized URL (trim +
+ * lowercase), mirroring the store's `findByUrl` dedup so case/whitespace
+ * differences still hide a popular entry once it's been added. Disabled
+ * popular entries are always excluded. Without this an added popular catalog
+ * would keep rendering in the Popular section and look like a duplicate
+ * (issue #4782).
+ */
+export const getUnaddedPopularCatalogs = (
+  popularCatalogs: OPDSCatalog[],
+  addedCatalogs: OPDSCatalog[],
+): OPDSCatalog[] => {
+  const addedUrls = new Set(addedCatalogs.map((c) => c.url.trim().toLowerCase()));
+  return popularCatalogs.filter(
+    (catalog) => !catalog.disabled && !addedUrls.has(catalog.url.trim().toLowerCase()),
+  );
+};
+
 export const getFileExtFromPath = (pathname: string, delimiter = '/'): string => {
  const parts = pathname.split(delimiter);
  for (const ext of Object.values(EXTS)) {