Skip to content

Commit 6e4e01a

Browse files
nsantacruzclaude
andauthored
Hide topics with 2 or fewer sources from A-Z, category browse, and Google [sc-45176] (#3496)
* feat(topics): hide low-source topics from A-Z, category browse, and Google search [sc-45176] Topics with 2 or fewer sources are no longer surfaced on the Topic A-Z page, category/TOC browse cards, or Google (excluded from the sitemap and marked noindex on their own page). They remain fully intact in the database and still appear in other topics' "Related Topics" sidebars. should_display() gains an optional min_sources param (default preserves existing behavior) so the stricter threshold can be applied only at the three affected call sites, leaving the sidebar's annotate_topic_link() path untouched. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * test(topics): parametrize should_display tests Collapses four repetitive test methods into a single pytest.mark.parametrize covering the same cases (default vs raised threshold, curation exemptions, explicit shouldDisplay=False override). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
1 parent ca65de1 commit 6e4e01a

8 files changed

Lines changed: 96 additions & 12 deletions

File tree

reader/views.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
from remote_config.keys import CHATBOT_MAX_INPUT_CHARS, CHATBOT_MAX_PROMPTS, CHATBOT_PROMO_LEARN_MORE_URLS, CHATBOT_PROMO_MAYBE_LATER_JSON, SHOW_JOIN_CHATBOT_BANNER, CHATBOT_PROMO_SESSION_LENGTH_SECONDS
2626
from sefaria.system.context_processors import _is_user_in_experiment
2727
from sefaria.utils.util import get_redirect_to_help_center
28-
from sefaria.constants.model import LIBRARY_MODULE, VOICES_MODULE
28+
from sefaria.constants.model import LIBRARY_MODULE, VOICES_MODULE, MIN_SOURCES_FOR_TOPIC_DISPLAY
2929
from rest_framework.decorators import api_view, permission_classes
3030
from rest_framework.permissions import IsAuthenticated
3131
from django.template.loader import render_to_string
@@ -3519,6 +3519,7 @@ def topic_page(request, slug, test_version=None):
35193519
return render_template(request, 'base.html', props, {
35203520
"title": title,
35213521
"desc": desc,
3522+
"noindex": not topic_obj.should_display(min_sources=MIN_SOURCES_FOR_TOPIC_DISPLAY),
35223523
})
35233524

35243525
@catch_error_as_json
@@ -3528,7 +3529,7 @@ def topics_list_api(request):
35283529
"""
35293530
limit = int(request.GET.get("limit", 1000))
35303531
minify = bool(int(request.GET.get("minify", 1)))
3531-
all_topics = get_all_topics(limit, active_module=request.active_module)
3532+
all_topics = get_all_topics(limit, active_module=request.active_module, min_sources=MIN_SOURCES_FOR_TOPIC_DISPLAY)
35323533
all_topics_json = []
35333534
for topic in all_topics:
35343535
topic_json = topic.contents(minify=minify, with_html=True)

sefaria/constants/model.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,4 +33,7 @@
3333

3434
# Module constants that correspond to DOMAIN_MODULES keys
3535
LIBRARY_MODULE = "library"
36-
VOICES_MODULE = "voices"
36+
VOICES_MODULE = "voices"
37+
38+
# Topics with fewer sources than this are hidden from the A-Z listing, category browse pages, and Google search
39+
MIN_SOURCES_FOR_TOPIC_DISPLAY = 3

sefaria/helper/tests/topic_test.py

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
from sefaria.model.text import library
44
from sefaria.model.place import Place
55
from sefaria.helper import topic
6+
from sefaria.conftest import mock_topics_pool
67

78

89
@pytest.fixture(autouse=True, scope='module')
@@ -215,6 +216,50 @@ def test_update_topic(some_topic):
215216
topic.update_topic(some_topic, slug='abc')
216217

217218

219+
@pytest.fixture(scope='module')
220+
def topics_with_varying_source_counts(django_db_setup, django_db_blocker):
221+
# Topic.get_pools() is globally mocked in sefaria/conftest.py to read from mock_topics_pool
222+
# rather than the real DjangoTopic pool tables, so pool membership for test topics is granted
223+
# the same way: by adding their slugs to that dict.
224+
with django_db_blocker.unblock():
225+
specs = [
226+
('zero', 0, {}),
227+
('two', 2, {}),
228+
('three', 3, {}),
229+
('curated', 1, {"description": {"en": "A hand-written description"}}),
230+
]
231+
by_suffix = {}
232+
for suffix, num_sources, extra_attrs in specs:
233+
t = Topic({'slug': "", "numSources": num_sources, **extra_attrs})
234+
title = f"get_all_topics test {suffix}"
235+
t.add_primary_titles(title, title[::-1])
236+
t.set_slug_to_primary_title()
237+
t.save()
238+
mock_topics_pool[t.slug] = ['library']
239+
by_suffix[suffix] = t
240+
yield by_suffix
241+
for t in by_suffix.values():
242+
mock_topics_pool.pop(t.slug, None)
243+
t.delete()
244+
245+
246+
def test_get_all_topics_min_sources(topics_with_varying_source_counts):
247+
from django.core.cache import caches
248+
caches['default'].clear() # get_all_topics is cached for 24h; bust it so fixture topics are picked up
249+
250+
by_suffix = topics_with_varying_source_counts
251+
relevant_slugs = {t.slug for t in by_suffix.values()}
252+
253+
default_result = {t.slug for t in topic.get_all_topics(limit=0) if t.slug in relevant_slugs}
254+
# unchanged legacy behavior: raw query only requires numSources > 0, no min_sources filtering applied
255+
assert default_result == {by_suffix['two'].slug, by_suffix['three'].slug, by_suffix['curated'].slug}
256+
257+
strict_result = {t.slug for t in topic.get_all_topics(limit=0, min_sources=3) if t.slug in relevant_slugs}
258+
# 'two' has only 2 sources and no exemption, so it's hidden; 'curated' has 1 source but a
259+
# published description, so the curation exemption keeps it visible; 'three' clears the bar outright
260+
assert strict_result == {by_suffix['three'].slug, by_suffix['curated'].slug}
261+
262+
218263
def test_get_topic_omits_orphaned_ref_links():
219264
"""A RefTopicLink whose text was deleted from the library (its ref no longer
220265
resolves) must be omitted from the get_topic response. Otherwise the orphaned source

sefaria/helper/topic.py

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -256,14 +256,17 @@ def annotate_topic_link(link: dict, link_topic_dict: dict) -> Union[dict, None]:
256256

257257

258258
@django_cache(timeout=24 * 60 * 60)
259-
def get_all_topics(limit=1000, displayableOnly=True, active_module=LIBRARY_MODULE):
259+
def get_all_topics(limit=1000, displayableOnly=True, active_module=LIBRARY_MODULE, min_sources=None):
260260
query = {"shouldDisplay": {"$ne": False}, "numSources": {"$gt": 0}} if displayableOnly else {}
261261
topic_list = TopicSet(query, limit=limit, sort=[('numSources', -1)])
262-
262+
263263
# Get the actual pool name that should be used for this active_module
264264
expected_pool_name = get_topic_pool_name_for_module(active_module)
265-
266-
return [t for t in topic_list if expected_pool_name in t.get_pools()]
265+
266+
topics = [t for t in topic_list if expected_pool_name in t.get_pools()]
267+
if min_sources is not None:
268+
topics = [t for t in topics if t.should_display(min_sources=min_sources)]
269+
return topics
267270

268271
@django_cache(timeout=24 * 60 * 60)
269272
def get_num_library_topics():

sefaria/model/tests/topic_test.py

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,36 @@ def test_sanitize(self):
209209
assert "<script>" not in t.description["he"]
210210
assert "<script>" not in t.slug
211211

212+
@pytest.mark.parametrize(('num_sources', 'min_sources', 'description', 'data_source', 'should_display_flag', 'expected'), [
213+
(1, 1, None, None, None, True), # default threshold matches legacy '> 0' behavior
214+
(0, 1, None, None, None, False), # default threshold matches legacy '> 0' behavior
215+
(2, 1, None, None, None, True), # below the raised threshold, but default threshold still passes
216+
(2, 3, None, None, None, False), # raised threshold hides low-source topics
217+
(3, 3, None, None, None, True), # raised threshold: topic clears the bar
218+
(1, 3, {"en": "A hand-written description"}, None, None, True), # curation exemption: published description
219+
(0, 3, None, "sefaria", None, True), # curation exemption: manually-curated topic
220+
(10, 3, None, None, False, False), # explicit shouldDisplay=False always wins
221+
], ids=[
222+
'default-threshold-shown',
223+
'default-threshold-hidden',
224+
'below-raised-threshold-shown-at-default',
225+
'below-raised-threshold-hidden',
226+
'meets-raised-threshold-shown',
227+
'description-exemption',
228+
'data-source-exemption',
229+
'explicit-false-overrides',
230+
])
231+
def test_should_display(self, num_sources, min_sources, description, data_source, should_display_flag, expected):
232+
t = Topic()
233+
t.numSources = num_sources
234+
if description is not None:
235+
t.description = description
236+
if data_source is not None:
237+
t.data_source = data_source
238+
if should_display_flag is not None:
239+
t.shouldDisplay = should_display_flag
240+
assert t.should_display(min_sources=min_sources) is expected
241+
212242

213243
class TestTopicLinkHelper(object):
214244

sefaria/model/text.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5230,7 +5230,7 @@ def get_topic_toc_json_recursive(self, topic=None, explored=None, with_descripti
52305230
topic_json = {}
52315231
else:
52325232
children = [] if topic.slug in explored else [l.fromTopic for l in IntraTopicLinkSet({"linkType": "displays-under", "toTopic": topic.slug})]
5233-
topic_json = topic.contents(minify=True, children=children, with_html=True)
5233+
topic_json = topic.contents(minify=True, children=children, with_html=True, min_sources_for_display=constants.MIN_SOURCES_FOR_TOPIC_DISPLAY)
52345234
unexplored_top_level = getattr(topic, "isTopLevelDisplay", False) and getattr(topic, "slug",
52355235
None) not in explored
52365236
explored.add(topic.slug)

sefaria/model/topic.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -364,8 +364,8 @@ def has_types(self, search_slug_set) -> bool:
364364
types = self.get_types(search_slug_set=search_slug_set)
365365
return len(search_slug_set.intersection(types)) > 0
366366

367-
def should_display(self) -> bool:
368-
return getattr(self, 'shouldDisplay', True) and (getattr(self, 'numSources', 0) > 0 or self.has_description() or getattr(self, "data_source", "") == "sefaria")
367+
def should_display(self, min_sources: int = 1) -> bool:
368+
return getattr(self, 'shouldDisplay', True) and (getattr(self, 'numSources', 0) >= min_sources or self.has_description() or getattr(self, "data_source", "") == "sefaria")
369369

370370
def has_description(self) -> bool:
371371
"""
@@ -516,9 +516,10 @@ def contents(self, **kwargs):
516516
d.update(super(Topic, self).contents(**kwargs))
517517
else:
518518
children = kwargs.get("children", [])
519+
min_sources = kwargs.get("min_sources_for_display", 1)
519520
d.update({
520521
'slug': self.slug,
521-
"shouldDisplay": True if len(children) > 0 else self.should_display(),
522+
"shouldDisplay": True if len(children) > 0 else self.should_display(min_sources=min_sources),
522523
"displayOrder": getattr(self, "displayOrder", 10000),
523524
"pools": self.get_pools()})
524525
if getattr(self, "categoryDescription", False):

sefaria/sitemap.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
from sefaria.model import *
1010
from sefaria.system.database import db
11+
from sefaria.constants.model import MIN_SOURCES_FOR_TOPIC_DISPLAY
1112
from .settings import STATICFILES_DIRS, STATIC_URL
1213

1314

@@ -133,7 +134,7 @@ def generate_topics_sitemap(self):
133134
Creates a sitemap for each topic that has at least one source or source sheet.
134135
"""
135136
topics = TopicSet()
136-
topics = [topic for topic in topics if topic.should_display()]
137+
topics = [topic for topic in topics if topic.should_display(min_sources=MIN_SOURCES_FOR_TOPIC_DISPLAY)]
137138
urls = [self._hostname + "/topics/" + topic.slug for topic in topics]
138139
self.write_urls(urls, "topics-sitemap.xml")
139140

0 commit comments

Comments
 (0)