Skip to content

Commit e58ac63

Browse files
escattoneescattone
authored
disallow restricted articles from pinned/featured (#6989)
1 parent f8945ce commit e58ac63

File tree

6 files changed

+34
-114
lines changed

6 files changed

+34
-114
lines changed

‎kitsune/landings/views.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ def home(request):
1515
"landings/home.html",
1616
{
1717
"products": Product.active.filter(visible=True),
18-
"featured": get_featured_articles(user=request.user, locale=request.LANGUAGE_CODE),
18+
"featured": get_featured_articles(locale=request.LANGUAGE_CODE),
1919
},
2020
)
2121

‎kitsune/products/views.py‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,9 +68,7 @@ def product_landing(request: HttpRequest, slug: str) -> HttpResponse:
6868
"topics": build_topics_data(request, product, topics),
6969
"search_params": {"product": slug},
7070
"latest_version": latest_version,
71-
"featured": get_featured_articles(
72-
user=request.user, product=product, locale=request.LANGUAGE_CODE
73-
),
71+
"featured": get_featured_articles(product=product, locale=request.LANGUAGE_CODE),
7472
"has_aaq_config": has_aaq_config(product),
7573
},
7674
)

‎kitsune/questions/views.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -604,7 +604,7 @@ def aaq(request, product_slug=None, step=1, is_loginless=False):
604604
topics = topics_for(request.user, product, parent=None)
605605

606606
context["featured"] = get_featured_articles(
607-
user=request.user, product=product, locale=request.LANGUAGE_CODE, fetch_for_aaq=True
607+
product=product, locale=request.LANGUAGE_CODE, fetch_for_aaq=True
608608
)
609609
context["topics"] = build_topics_data(request, product, topics)
610610

‎kitsune/wiki/config.py‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@
6161
),
6262
(
6363
MAJOR_SIGNIFICANCE,
64-
_lazy("Major content changes that will make older translations " "inaccurate"),
64+
_lazy("Major content changes that will make older translations inaccurate"),
6565
),
6666
]
6767

@@ -115,10 +115,11 @@
115115
# Q object for filtering valid pinned articles.
116116
# This is used in both the admin UI (via limit_choices_to) and in runtime
117117
# filtering (via get_pinned_articles()) to ensure consistency.
118-
# Excludes: templates, archived, redirects, and non-IA categories.
118+
# Excludes: templates, archived, restricted, redirects, and non-IA categories.
119119
PINNED_ARTICLE_LIMIT_Q = Q(
120120
parent__isnull=True,
121121
is_template=False,
122122
is_archived=False,
123+
restrict_to_groups__isnull=True,
123124
category__in=settings.IA_DEFAULT_CATEGORIES,
124125
) & ~Q(html__startswith=REDIRECT_HTML)

‎kitsune/wiki/tests/test_utils.py‎

Lines changed: 17 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -1081,10 +1081,10 @@ def test_when_no_config_for_product(self):
10811081
"""
10821082
Test when a product has no pinned articles.
10831083
"""
1084-
result = get_pinned_articles(user=self.user, product=self.product2)
1084+
result = get_pinned_articles(product=self.product2)
10851085
self.assertEqual(result.count(), 0)
10861086

1087-
result = get_pinned_articles(user=self.user, product=self.product2, fetch_for_aaq=True)
1087+
result = get_pinned_articles(product=self.product2, fetch_for_aaq=True)
10881088
self.assertEqual(result.count(), 0)
10891089

10901090
def test_for_default_locale(self):
@@ -1100,37 +1100,16 @@ def test_for_default_locale(self):
11001100
result = get_pinned_articles(product=self.product1, fetch_for_aaq=True)
11011101
self.assertEqual(result.count(), 0)
11021102

1103-
result = get_pinned_articles(user=self.user, product=self.product1)
1104-
self.assertEqual(list(result), [self.en_doc])
1105-
1106-
result = get_pinned_articles(user=self.user, product=self.product1, fetch_for_aaq=True)
1107-
self.assertEqual(result.count(), 0)
1108-
1109-
result = get_pinned_articles(user=self.user_g1, product=self.product1)
1110-
self.assertEqual(set(result), {self.en_doc, self.en_restricted_doc})
1111-
1112-
result = get_pinned_articles(user=self.user_g1, product=self.product2)
1113-
self.assertEqual(list(result), [self.en_restricted_doc])
1114-
1115-
result = get_pinned_articles(user=self.user_g1, product=self.product1, fetch_for_aaq=True)
1116-
self.assertEqual(list(result), [self.en_restricted_doc])
1117-
11181103
result = get_pinned_articles()
11191104
self.assertEqual(result.count(), 0)
11201105

11211106
result = get_pinned_articles(fetch_for_aaq=True)
11221107
self.assertEqual(result.count(), 0)
11231108

1124-
result = get_pinned_articles(user=self.user)
1125-
self.assertEqual(result.count(), 0)
1126-
1127-
result = get_pinned_articles(user=self.user, fetch_for_aaq=True)
1109+
result = get_pinned_articles()
11281110
self.assertEqual(result.count(), 0)
11291111

1130-
result = get_pinned_articles(user=self.user_g1)
1131-
self.assertEqual(list(result), [self.en_restricted_doc])
1132-
1133-
result = get_pinned_articles(user=self.user_g1, fetch_for_aaq=True)
1112+
result = get_pinned_articles(fetch_for_aaq=True)
11341113
self.assertEqual(result.count(), 0)
11351114

11361115
def test_for_non_default_locale(self):
@@ -1143,75 +1122,28 @@ def test_for_non_default_locale(self):
11431122
result = get_pinned_articles(product=self.product1, locale="de", fetch_for_aaq=True)
11441123
self.assertEqual(result.count(), 0)
11451124

1146-
result = get_pinned_articles(user=self.user, product=self.product1, locale="de")
1147-
self.assertEqual(set(result), {self.de_doc, self.de_parent_doc})
1148-
1149-
result = get_pinned_articles(
1150-
user=self.user, product=self.product1, locale="de", fetch_for_aaq=True
1151-
)
1125+
result = get_pinned_articles(product=self.product2, locale="de")
11521126
self.assertEqual(result.count(), 0)
11531127

1154-
result = get_pinned_articles(user=self.user_g1, product=self.product1, locale="de")
1155-
self.assertEqual(set(result), {self.de_doc, self.de_parent_doc, self.de_restricted_doc})
1156-
1157-
result = get_pinned_articles(user=self.user_g1, product=self.product2, locale="de")
1158-
self.assertEqual(list(result), [self.de_restricted_doc])
1159-
1160-
result = get_pinned_articles(
1161-
user=self.user_g1, product=self.product1, locale="de", fetch_for_aaq=True
1162-
)
1163-
self.assertEqual(list(result), [self.de_restricted_doc])
1164-
1165-
result = get_pinned_articles(user=self.user, product=self.product1, locale="fr")
1128+
result = get_pinned_articles(product=self.product1, locale="fr")
11661129
self.assertEqual(list(result), [self.fr_doc])
11671130

1168-
result = get_pinned_articles(user=self.user, product=self.product2, locale="fr")
1131+
result = get_pinned_articles(product=self.product2, locale="fr")
11691132
self.assertEqual(result.count(), 0)
11701133

1171-
result = get_pinned_articles(
1172-
user=self.user, product=self.product1, locale="fr", fetch_for_aaq=True
1173-
)
1174-
self.assertEqual(result.count(), 0)
1175-
1176-
result = get_pinned_articles(user=self.user_g1, product=self.product1, locale="fr")
1177-
self.assertEqual(set(result), {self.fr_doc, self.fr_restricted_parent_doc})
1178-
1179-
result = get_pinned_articles(user=self.user_g1, product=self.product2, locale="fr")
1180-
self.assertEqual(list(result), [self.fr_restricted_parent_doc])
1181-
1182-
result = get_pinned_articles(
1183-
user=self.user_g1, product=self.product1, locale="fr", fetch_for_aaq=True
1184-
)
1134+
result = get_pinned_articles(product=self.product1, locale="fr", fetch_for_aaq=True)
11851135
self.assertEqual(result.count(), 0)
11861136

11871137
result = get_pinned_articles(product=self.product1, locale="es")
1188-
self.assertEqual(result.count(), 0)
1189-
1190-
result = get_pinned_articles(user=self.user, product=self.product1, locale="es")
11911138
self.assertEqual(list(result), [self.es_doc])
11921139

1193-
result = get_pinned_articles(user=self.user_g1, product=self.product1, locale="es")
1194-
self.assertEqual(result.count(), 0)
1195-
11961140
result = get_pinned_articles(locale="de")
11971141
self.assertEqual(result.count(), 0)
11981142

1199-
result = get_pinned_articles(user=self.user, locale="de")
1143+
result = get_pinned_articles(locale="fr")
12001144
self.assertEqual(result.count(), 0)
12011145

1202-
result = get_pinned_articles(user=self.user, locale="fr")
1203-
self.assertEqual(result.count(), 0)
1204-
1205-
result = get_pinned_articles(user=self.user, locale="es")
1206-
self.assertEqual(result.count(), 0)
1207-
1208-
result = get_pinned_articles(user=self.user_g1, locale="de")
1209-
self.assertEqual(list(result), [self.de_restricted_doc])
1210-
1211-
result = get_pinned_articles(user=self.user_g1, locale="fr")
1212-
self.assertEqual(list(result), [self.fr_restricted_parent_doc])
1213-
1214-
result = get_pinned_articles(user=self.user_g1, locale="es")
1146+
result = get_pinned_articles(locale="es")
12151147
self.assertEqual(result.count(), 0)
12161148

12171149
def test_articles_transitioning_to_invalid_states(self):
@@ -1233,39 +1165,39 @@ def test_articles_transitioning_to_invalid_states(self):
12331165
self.product1.pinned_article_config = config
12341166
self.product1.save()
12351167

1236-
result = get_pinned_articles(user=self.user, product=self.product1)
1168+
result = get_pinned_articles(product=self.product1)
12371169
self.assertEqual(list(result), [valid_doc])
12381170

12391171
valid_doc.is_archived = True
12401172
valid_doc.save()
1241-
result = get_pinned_articles(user=self.user, product=self.product1)
1173+
result = get_pinned_articles(product=self.product1)
12421174
self.assertEqual(result.count(), 0)
12431175

12441176
valid_doc.is_archived = False
12451177
valid_doc.category = ADMINISTRATION_CATEGORY
12461178
valid_doc.save()
1247-
result = get_pinned_articles(user=self.user, product=self.product1)
1179+
result = get_pinned_articles(product=self.product1)
12481180
self.assertEqual(result.count(), 0)
12491181

12501182
valid_doc.category = NAVIGATION_CATEGORY
12511183
valid_doc.save()
1252-
result = get_pinned_articles(user=self.user, product=self.product1)
1184+
result = get_pinned_articles(product=self.product1)
12531185
self.assertEqual(result.count(), 0)
12541186

12551187
valid_doc.category = TEMPLATES_CATEGORY
12561188
valid_doc.title = "Template:" + valid_doc.title
12571189
valid_doc.save()
1258-
result = get_pinned_articles(user=self.user, product=self.product1)
1190+
result = get_pinned_articles(product=self.product1)
12591191
self.assertEqual(result.count(), 0)
12601192

12611193
valid_doc.category = CANNED_RESPONSES_CATEGORY
12621194
valid_doc.title = valid_doc.title.replace("Template:", "")
12631195
valid_doc.save()
1264-
result = get_pinned_articles(user=self.user, product=self.product1)
1196+
result = get_pinned_articles(product=self.product1)
12651197
self.assertEqual(result.count(), 0)
12661198

12671199
valid_doc.category = HOW_TO_CATEGORY
12681200
valid_doc.save()
12691201
RedirectRevisionFactory(document=valid_doc)
1270-
result = get_pinned_articles(user=self.user, product=self.product1)
1202+
result = get_pinned_articles(product=self.product1)
12711203
self.assertEqual(result.count(), 0)

‎kitsune/wiki/utils.py‎

Lines changed: 11 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -113,11 +113,11 @@ def _active_contributors_id(from_date, to_date, locale, product):
113113

114114

115115
def get_pinned_articles(
116-
user=None, product=None, locale=settings.WIKI_DEFAULT_LANGUAGE, fetch_for_aaq=False
116+
product=None, locale=settings.WIKI_DEFAULT_LANGUAGE, fetch_for_aaq=False
117117
) -> QuerySet[Document]:
118118
"""
119119
Given the product, locale, and whether or not we're getting pinned articles for the
120-
AAQ, returns a queryset of the pinned articles that are visible to the given user.
120+
AAQ, returns a queryset of the pinned articles.
121121
"""
122122
qs = PinnedArticleConfig.objects
123123

@@ -139,43 +139,33 @@ def get_pinned_articles(
139139
condition = condition | Q(parent__in=pinned_articles)
140140

141141
return (
142-
Document.objects.visible(user, locale=locale)
143-
.filter(condition)
144-
.select_related("current_revision")
142+
Document.objects.filter(locale=locale).filter(condition).select_related("current_revision")
145143
)
146144

147145

148146
def get_featured_articles(
149-
user=None,
150-
product=None,
151-
topics=None,
152-
locale=settings.WIKI_DEFAULT_LANGUAGE,
153-
fetch_for_aaq=False,
154-
limit=4,
147+
product=None, topics=None, locale=settings.WIKI_DEFAULT_LANGUAGE, fetch_for_aaq=False, limit=4
155148
):
156149
"""Returns up to 4 random articles from the most visited.
157150
158151
Args:
159-
user: Optional user for visibility
160152
product: Optional product to filter by
161153
topics: Optional iterable of topics to filter by
162154
locale: Locale to get articles for, defaults to WIKI_DEFAULT_LANGUAGE
163155
fetch_for_aaq: Optional boolean (currently ignored)
164156
limit: Optional integer to limit the number of articles
165157
"""
166158
pinned_articles = list(
167-
get_pinned_articles(
168-
user=user,
169-
product=product,
170-
locale=locale,
171-
fetch_for_aaq=fetch_for_aaq,
172-
)
159+
get_pinned_articles(product=product, locale=locale, fetch_for_aaq=fetch_for_aaq)
173160
)
174161

175162
if len(pinned_articles) >= limit:
176163
return random.sample(pinned_articles, limit)
177164

178-
filter_kwargs = {"category__in": settings.IA_DEFAULT_CATEGORIES}
165+
filter_kwargs = {
166+
"restrict_to_groups__isnull": True,
167+
"category__in": settings.IA_DEFAULT_CATEGORIES,
168+
}
179169

180170
if product:
181171
filter_kwargs.update(products=product)
@@ -195,8 +185,7 @@ def q_for_parent(**kwargs):
195185
child_q = Q(**{f"parent__{k}": v for k, v in kwargs.items()})
196186
return (Q(parent__isnull=True) & parent_q) | (Q(parent__isnull=False) & child_q)
197187

198-
qs = Document.objects.visible(
199-
user,
188+
qs = Document.objects.filter(
200189
locale=locale,
201190
is_template=False,
202191
is_archived=False,
@@ -295,7 +284,7 @@ def build_topics_data(request: HttpRequest, product: Product, topics: list[Topic
295284
topics_data: list[dict] = []
296285

297286
featured_articles = get_featured_articles(
298-
user=request.user, product=product, topics=topics, locale=request.LANGUAGE_CODE
287+
product=product, topics=topics, locale=request.LANGUAGE_CODE
299288
)
300289

301290
# Get both main and fallback documents from the faceted search

0 commit comments

Comments
 (0)