diff --git a/kitsune/questions/jinja2/questions/includes/question_editing_frame.html b/kitsune/questions/jinja2/questions/includes/question_editing_frame.html index d5c49681dfb..ce70012fbe9 100644 --- a/kitsune/questions/jinja2/questions/includes/question_editing_frame.html +++ b/kitsune/questions/jinja2/questions/includes/question_editing_frame.html @@ -142,8 +142,6 @@

{% block headline %}{% endblock %}

{{ field }} {{ field.errors }} - {% elif category_field_attrs and (field.name == 'category') %} - {{ field.as_widget(attrs=category_field_attrs)|safe }} {% else %} {{ field|safe }} {% endif %} diff --git a/kitsune/questions/jinja2/questions/new_question.html b/kitsune/questions/jinja2/questions/new_question.html index f4ee223f652..bd286b3e64d 100644 --- a/kitsune/questions/jinja2/questions/new_question.html +++ b/kitsune/questions/jinja2/questions/new_question.html @@ -7,7 +7,6 @@ {% set meta = [('robots', 'noindex')] %} {% set classes = 'new-question' %} -{% set category_field_attrs = {"hx-get": "", "hx-swap": "none"}%} {% block breadcrumbs %}{% endblock %} @@ -40,4 +39,4 @@

{% block submit_button_value %} {{ _('Submit') }} {% endblock %} -{% block submit_button_attrs %}data-event-name="question_submit" data-event-parameters='{{ submit_event_parameters }}'{% endblock %} +{% block submit_button_attrs %}data-event-name="question_submit"{% endblock %} diff --git a/kitsune/questions/models.py b/kitsune/questions/models.py index 094a23998ff..a735536d078 100755 --- a/kitsune/questions/models.py +++ b/kitsune/questions/models.py @@ -1,4 +1,3 @@ -import json import logging import re from datetime import datetime, timedelta @@ -233,9 +232,7 @@ def clear_mutable_metadata(self): This excludes immutable fields: user agent, product, and category. """ - self.metadata_set.exclude( - name__in=["useragent", "product", "category", "kb_visits_prior"] - ).delete() + self.metadata_set.exclude(name__in=["useragent", "product", "category"]).delete() self._metadata = None def remove_metadata(self, name): @@ -405,25 +402,6 @@ def is_moderated(self): .exists() ) - @cached_property - def created_after_failed_kb_deflection(self) -> bool: - """ - Returns a boolean indicating whether or not this question was created after its - creator had visited one or more KB articles with the same product and topic. - """ - return self.metadata_set.filter(name="kb_visits_prior").exists() - - @cached_property - def kb_visits_prior_to_creation(self) -> list[str]: - """ - Returns the list of KB article URL's visited prior to the creation of this question. - """ - try: - metadata = self.metadata_set.filter(name="kb_visits_prior").get() - except QuestionMetaData.DoesNotExist: - return [] - return json.loads(metadata.value) - @property def my_tags(self): """A caching wrapper around self.tags.all().""" diff --git a/kitsune/questions/tests/test_models.py b/kitsune/questions/tests/test_models.py index 6cd46907ed1..f72dade407d 100644 --- a/kitsune/questions/tests/test_models.py +++ b/kitsune/questions/tests/test_models.py @@ -1,4 +1,3 @@ -import json from datetime import datetime, timedelta from unittest import mock @@ -215,7 +214,6 @@ def test_clear_mutable_metadata(self): category="fix-problems", useragent="Fyerfocks", crash_id="7", - kb_visits_prior='["/en-US/kb/stuff", "/en-US/kb/nonsense"]', ) q.metadata @@ -223,13 +221,7 @@ def test_clear_mutable_metadata(self): md = q.metadata assert "crash_id" not in md, "clear_mutable_metadata() didn't clear the cached metadata." self.assertEqual( - dict( - product="desktop", - category="fix-problems", - useragent="Fyerfocks", - kb_visits_prior='["/en-US/kb/stuff", "/en-US/kb/nonsense"]', - ), - md, + dict(product="desktop", category="fix-problems", useragent="Fyerfocks"), md ) def test_auto_tagging(self): @@ -276,16 +268,6 @@ def test_has_beta(self): assert _has_beta("11.0", {"11.0b7": "2011-06-01"}) assert not _has_beta("10.0", {"11.0b7": "2011-06-01"}) - def test_kb_visits_prior(self): - visits = ["/en-US/kb/stuff", "/en-US/kb/nonsense"] - self.question.add_metadata(kb_visits_prior=json.dumps(visits)) - self.assertTrue(self.question.created_after_failed_kb_deflection) - self.assertEqual(set(self.question.kb_visits_prior_to_creation), set(visits)) - - def test_no_kb_visits_prior(self): - self.assertFalse(self.question.created_after_failed_kb_deflection) - self.assertEqual(self.question.kb_visits_prior_to_creation, []) - class QuestionTests(TestCase): """Tests for Question model""" diff --git a/kitsune/questions/utils.py b/kitsune/questions/utils.py index 0ae3349ca1e..2f9b26965ca 100644 --- a/kitsune/questions/utils.py +++ b/kitsune/questions/utils.py @@ -1,14 +1,10 @@ -import json import logging import re -from typing import Optional from django.conf import settings -from django.contrib.sessions.backends.base import SessionBase -from kitsune.products.models import Product, Topic from kitsune.questions.models import Answer, Question -from kitsune.wiki.utils import get_featured_articles as kb_get_featured_articles, has_visited_kb +from kitsune.wiki.utils import get_featured_articles as kb_get_featured_articles REGEX_NON_WINDOWS_HOME_DIR = re.compile( @@ -119,22 +115,3 @@ def remove_pii(data: dict) -> None: remove_pii(value) elif isinstance(value, str): data[key] = remove_home_dir_pii(value) - - -def get_ga_submit_event_parameters_as_json( - session: Optional[SessionBase] = None, - product: Optional[Product] = None, - topic: Optional[Topic] = None, -) -> str: - """ - Returns a JSON string of the event parameters for the GA4 "question_submit" - event, given the session, product, and/or topic. - """ - data = dict(is_failed_deflection="false") - - if session and product: - data["is_failed_deflection"] = str(has_visited_kb(session, product, topic=topic)).lower() - if topic: - data["topics"] = f"/{topic.slug}/" - - return json.dumps(data) diff --git a/kitsune/questions/views.py b/kitsune/questions/views.py index d4a46dd531c..f729ed58fde 100644 --- a/kitsune/questions/views.py +++ b/kitsune/questions/views.py @@ -49,11 +49,7 @@ WatchQuestionForm, ) from kitsune.questions.models import AAQConfig, Answer, AnswerVote, Question, QuestionVote -from kitsune.questions.utils import ( - get_featured_articles, - get_ga_submit_event_parameters_as_json, - get_mobile_product_from_ua, -) +from kitsune.questions.utils import get_featured_articles, get_mobile_product_from_ua from kitsune.sumo.decorators import ratelimit from kitsune.sumo.i18n import split_into_language_and_path from kitsune.sumo.templatetags.jinja_helpers import urlparams @@ -74,7 +70,7 @@ from kitsune.upload.models import ImageAttachment from kitsune.users.models import Setting from kitsune.wiki.facets import topics_for -from kitsune.wiki.utils import build_topics_data, get_kb_visited +from kitsune.wiki.utils import build_topics_data log = logging.getLogger("k.questions") @@ -607,9 +603,6 @@ def aaq(request, product_slug=None, step=1, is_loginless=False): user=request.user, ) context["form"] = zendesk_form - context["submit_event_parameters"] = get_ga_submit_event_parameters_as_json( - request.session, product - ) if zendesk_form.is_valid() and not is_ratelimited(request, "loginless", "3/d"): try: @@ -655,9 +648,6 @@ def aaq(request, product_slug=None, step=1, is_loginless=False): product=product, ) - if visits := get_kb_visited(request.session, product, question.topic): - question.add_metadata(kb_visits_prior=json.dumps(visits)) - if form.cleaned_data.get("is_spam"): _add_to_moderation_queue(request, question) @@ -690,17 +680,6 @@ def aaq(request, product_slug=None, step=1, is_loginless=False): content_type=user_ct, ).order_by("-id")[:IMG_LIMIT] - if form.is_bound and (topic := form.cleaned_data.get("category")): - # We've got invalid POST data, but the topic has been provided. - # Let's set the proper GA4 event parameters on the submit button. - context["submit_event_parameters"] = get_ga_submit_event_parameters_as_json( - request.session, product, topic=topic - ) - else: - # We don't know the topic yet, since that's set via the form, so let's - # start by providing default GA4 event parameters for the submit button. - context["submit_event_parameters"] = get_ga_submit_event_parameters_as_json() - return render(request, template, context) @@ -712,26 +691,6 @@ def aaq_step2(request, product_slug): def aaq_step3(request, product_slug): """Step 3: Show full question form.""" - # This view can be called by htmx to set the GA4 event parameters on the submit - # button of the new-question form whenever the category (topic) menu is changed. - if ("hx-request" in request.headers) and ( - request.headers.get("hx-trigger-name") == "category" - ): - topic_id = request.GET.get("category") - product = get_object_or_404(Product.active, slug=product_slug) - topic = get_object_or_404(Topic.active.filter(products=product, in_aaq=True), pk=topic_id) - response = HttpResponse(status=204) - response["HX-Trigger"] = json.dumps( - { - "setQuestionSubmitEventParameters": { - "eventParameters": get_ga_submit_event_parameters_as_json( - request.session, product, topic=topic - ) - } - } - ) - return response - # Since removing the @login_required decorator for MA form # need to catch unauthenticated, non-MA users here """ referer = request.META.get("HTTP_REFERER", "") diff --git a/kitsune/sumo/static/sumo/js/questions.js b/kitsune/sumo/static/sumo/js/questions.js index 40ead146a41..7b716ba8ffb 100644 --- a/kitsune/sumo/static/sumo/js/questions.js +++ b/kitsune/sumo/static/sumo/js/questions.js @@ -13,18 +13,6 @@ import AAQSystemInfo from "sumo/js/aaq"; * Scripts for the questions app. */ -document.addEventListener("DOMContentLoaded", async () => { - const body = document.querySelector("body.new-question"); - if (body) { - const submitButton = body.querySelector('#question-form button[type="submit"]'); - if (submitButton) { - body.addEventListener('setQuestionSubmitEventParameters', (event) => { - submitButton.dataset.eventParameters = event.detail.eventParameters; - }); - } - } -}); - // TODO: Figure out how to break out the functionality here into // testable parts. diff --git a/kitsune/wiki/tests/test_utils.py b/kitsune/wiki/tests/test_utils.py index 3778aa603c7..c43a06cd44a 100644 --- a/kitsune/wiki/tests/test_utils.py +++ b/kitsune/wiki/tests/test_utils.py @@ -1,9 +1,6 @@ -import json -import time from datetime import date, timedelta from unittest import mock -from django.contrib.sessions.backends.base import SessionBase from django.test.utils import override_settings from requests.exceptions import HTTPError @@ -24,11 +21,7 @@ active_contributors, generate_short_url, get_featured_articles, - get_kb_visited, - has_visited_kb, num_active_contributors, - remove_expired_from_kb_visited, - update_kb_visited, ) @@ -251,452 +244,3 @@ def test_get_featured_articles(self): ) self.assertEqual(len(featured), 1) self.assertEqual(featured[0].id, self.de1.id) - - -class RemoveExpiredFromKBVisitedTests(TestCase): - def setUp(self): - self.session = SessionBase() - - def test_no_session(self): - try: - remove_expired_from_kb_visited(None) - except Exception as err: - self.fail( - ( - "remove_expired_from_kb_visited() raised an " - f"exception with a session of None: {err}" - ) - ) - - def test_session_without_kb_visited(self): - remove_expired_from_kb_visited(self.session) - self.assertFalse(self.session.modified) - - def test_session_with_empty_kb_visited(self): - self.session["kb-visited"] = {} - self.session.modified = False - remove_expired_from_kb_visited(self.session) - self.assertFalse(self.session.modified) - - def test_no_expired_slugs(self): - now = time.time() - self.session["kb-visited"] = { - "/firefox/browse/": { - "/en-US/kb/slug1": now, - "/de/kb/slug2": now + 1, - }, - "/mobile/privacy-and-security/": { - "/it/kb/slug3": now + 2, - "/fr/kb/slug4": now + 3, - }, - } - self.session.modified = False - remove_expired_from_kb_visited(self.session, ttl=10) - self.assertEqual( - self.session["kb-visited"], - { - "/firefox/browse/": { - "/en-US/kb/slug1": now, - "/de/kb/slug2": now + 1, - }, - "/mobile/privacy-and-security/": { - "/it/kb/slug3": now + 2, - "/fr/kb/slug4": now + 3, - }, - }, - ) - self.assertFalse(self.session.modified) - - def test_remove_expired_slugs(self): - now = time.time() - self.session["kb-visited"] = { - "/firefox/browse/": { - "/en-US/kb/expired1": now - 20, - "/en-US/kb/valid": now, - }, - "/mobile/privacy-and-security/": { - "/en-US/kb/expired2": now - 15, - "/en-US/kb/expired3": now - 11, - }, - } - self.session.modified = False - remove_expired_from_kb_visited(self.session, ttl=10) - self.assertTrue(self.session.modified) - self.assertEqual( - self.session["kb-visited"], - { - "/firefox/browse/": { - "/en-US/kb/valid": now, - }, - }, - ) - - def test_all_slugs_expired(self): - now = time.time() - self.session["kb-visited"] = { - "/firefox/browse/": { - "/en-US/kb/expired1": now - 20, - "/en-US/kb/expired2": now - 40, - }, - "/mobile/privacy-and-security/": { - "/en-US/kb/expired3": now - 15, - "/en-US/kb/expired4": now - 11, - }, - } - self.session.modified = False - remove_expired_from_kb_visited(self.session, ttl=10) - self.assertTrue(self.session.modified) - self.assertEqual(self.session["kb-visited"], {}) - - -class UpdateKBVisitedTests(TestCase): - def setUp(self): - self.session = SessionBase() - self.product1 = product1 = ProductFactory(slug="firefox") - self.product2 = product2 = ProductFactory(slug="mobile") - self.topic1 = topic1 = TopicFactory(slug="browse", products=[product1]) - self.topic2 = topic2 = TopicFactory(slug="settings", products=[product1, product2]) - self.doc1 = DocumentFactory(products=[product1], topics=[topic1]) - self.doc2 = DocumentFactory(products=[product1, product2], topics=[topic1, topic2]) - self.doc3 = DocumentFactory(locale="de", parent=self.doc1) - - def test_no_session(self): - try: - update_kb_visited(None, self.doc1) - except Exception as err: - self.fail(f"update_kb_visited() raised an exception with a session of None: {err}") - - def test_session_without_kb_visited(self): - update_kb_visited(self.session, self.doc2) - self.assertIn("kb-visited", self.session) - key = "/firefox/mobile/browse/settings/" - self.assertEqual(list(self.session["kb-visited"].keys()), [key]) - self.assertEqual( - list(self.session["kb-visited"][key].keys()), [self.doc2.get_absolute_url()] - ) - self.assertTrue(self.session.modified) - - def test_with_localized_document(self): - update_kb_visited(self.session, self.doc3) - self.assertIn("kb-visited", self.session) - key = "/firefox/browse/" - self.assertEqual(list(self.session["kb-visited"].keys()), [key]) - self.assertEqual( - list(self.session["kb-visited"][key].keys()), [self.doc3.get_absolute_url()] - ) - self.assertTrue(self.session.modified) - - def test_session_with_existing_kb_visited(self): - now = time.time() - self.session["kb-visited"] = { - "/firefox/browse/": { - "/en-US/kb/expired1": now - 20, - "/en-US/kb/valid": now, - }, - "/mobile/privacy-and-security/": { - "/en-US/kb/expired2": now - 15, - "/en-US/kb/expired3": now - 11, - }, - } - self.session.modified = False - update_kb_visited(self.session, self.doc1, ttl=10) - key = "/firefox/browse/" - self.assertEqual(list(self.session["kb-visited"].keys()), [key]) - self.assertEqual( - set(self.session["kb-visited"][key].keys()), - set(["/en-US/kb/valid", self.doc1.get_absolute_url()]), - ) - self.assertTrue(self.session.modified) - - -class GetKBVisitedTests(TestCase): - def setUp(self): - self.session = SessionBase() - self.product1 = product1 = ProductFactory(slug="firefox") - self.product2 = product2 = ProductFactory(slug="mobile") - self.topic1 = topic1 = TopicFactory(slug="browse", products=[product1]) - self.topic2 = topic2 = TopicFactory(slug="settings", products=[product1, product2]) - self.doc1 = DocumentFactory(products=[product1], topics=[topic1]) - self.doc2 = DocumentFactory(products=[product1, product2], topics=[topic2]) - self.doc3 = DocumentFactory(locale="de", parent=self.doc1) - - def test_no_session(self): - try: - visits = get_kb_visited(None, self.product1, topic=self.topic1) - except Exception as err: - self.fail(f"get_kb_visited() raised an exception with a session of None: {err}") - else: - self.assertEqual(visits, []) - - def test_session_without_kb_visited(self): - visits = get_kb_visited(self.session, self.product1) - self.assertEqual(visits, []) - self.assertFalse(self.session.modified) - - def test_session_with_empty_kb_visited(self): - self.session["kb-visited"] = {} - self.session.modified = False - visits = get_kb_visited(self.session, self.product2) - self.assertEqual(visits, []) - self.assertFalse(self.session.modified) - - def test_with_product_1(self): - now = time.time() - self.session["kb-visited"] = { - "/firefox/browse/": { - self.doc1.get_absolute_url(): now, - self.doc3.get_absolute_url(): now, - }, - "/firefox/mobile/settings/": { - self.doc2.get_absolute_url(): now - 15, - }, - } - self.session.modified = False - visits = get_kb_visited(self.session, self.product1, ttl=10) - self.assertEqual( - set(visits), set([self.doc1.get_absolute_url(), self.doc3.get_absolute_url()]) - ) - self.assertEqual( - self.session["kb-visited"], - { - "/firefox/browse/": { - self.doc1.get_absolute_url(): now, - self.doc3.get_absolute_url(): now, - }, - }, - ) - self.assertTrue(self.session.modified) - - def test_with_product_2(self): - now = time.time() - self.session["kb-visited"] = { - "/firefox/browse/": { - self.doc1.get_absolute_url(): now, - self.doc3.get_absolute_url(): now + 1, - }, - "/firefox/mobile/settings/": { - self.doc2.get_absolute_url(): now + 2, - }, - } - self.session.modified = False - visits = get_kb_visited(self.session, self.product2, ttl=10) - self.assertEqual(visits, [self.doc2.get_absolute_url()]) - self.assertEqual( - self.session["kb-visited"], - { - "/firefox/browse/": { - self.doc1.get_absolute_url(): now, - self.doc3.get_absolute_url(): now + 1, - }, - "/firefox/mobile/settings/": { - self.doc2.get_absolute_url(): now + 2, - }, - }, - ) - self.assertFalse(self.session.modified) - - def test_with_product_and_topic_1(self): - now = time.time() - self.session["kb-visited"] = { - "/firefox/browse/": { - self.doc1.get_absolute_url(): now, - self.doc3.get_absolute_url(): now - 15, - }, - "/firefox/mobile/settings/": { - self.doc2.get_absolute_url(): now + 2, - }, - } - self.session.modified = False - visits = get_kb_visited(self.session, self.product1, topic=self.topic1, ttl=10) - self.assertEqual(visits, [self.doc1.get_absolute_url()]) - self.assertEqual( - self.session["kb-visited"], - { - "/firefox/browse/": { - self.doc1.get_absolute_url(): now, - }, - "/firefox/mobile/settings/": { - self.doc2.get_absolute_url(): now + 2, - }, - }, - ) - self.assertTrue(self.session.modified) - - def test_with_product_and_topic_2(self): - now = time.time() - self.session["kb-visited"] = { - "/firefox/browse/": { - self.doc1.get_absolute_url(): now, - self.doc3.get_absolute_url(): now + 1, - }, - "/firefox/mobile/settings/": { - self.doc2.get_absolute_url(): now + 2, - }, - } - self.session.modified = False - visits = get_kb_visited(self.session, self.product1, topic=self.topic2, ttl=10) - self.assertEqual(visits, [self.doc2.get_absolute_url()]) - self.assertEqual( - self.session["kb-visited"], - { - "/firefox/browse/": { - self.doc1.get_absolute_url(): now, - self.doc3.get_absolute_url(): now + 1, - }, - "/firefox/mobile/settings/": { - self.doc2.get_absolute_url(): now + 2, - }, - }, - ) - self.assertFalse(self.session.modified) - - def test_conversion_to_json(self): - now = time.time() - self.session["kb-visited"] = { - "/firefox/browse/": { - "/en-US/kb/stuff": now, - }, - "/firefox/mobile/settings/": { - "/en-US/kb/nonsense": now + 1, - }, - } - self.session.modified = False - visits = get_kb_visited(self.session, self.product1, ttl=10) - self.assertEqual(json.dumps(sorted(visits)), '["/en-US/kb/nonsense", "/en-US/kb/stuff"]') - - -class HasVisitedKBTests(TestCase): - def setUp(self): - self.session = SessionBase() - self.product1 = product1 = ProductFactory(slug="firefox") - self.product2 = product2 = ProductFactory(slug="mobile") - self.topic1 = topic1 = TopicFactory(slug="browse", products=[product1]) - self.topic2 = topic2 = TopicFactory(slug="settings", products=[product1, product2]) - self.doc1 = DocumentFactory(products=[product1], topics=[topic1]) - self.doc2 = DocumentFactory(products=[product1, product2], topics=[topic2]) - self.doc3 = DocumentFactory(locale="de", parent=self.doc1) - - def test_no_session(self): - try: - result = has_visited_kb(None, self.product1, topic=self.topic1) - except Exception as err: - self.fail(f"get_kb_visited() raised an exception with a session of None: {err}") - else: - self.assertIs(result, False) - - def test_session_without_kb_visited(self): - result = has_visited_kb(self.session, self.product1) - self.assertIs(result, False) - self.assertFalse(self.session.modified) - - def test_session_with_empty_kb_visited(self): - self.session["kb-visited"] = {} - self.session.modified = False - result = has_visited_kb(self.session, self.product2) - self.assertIs(result, False) - self.assertFalse(self.session.modified) - - def test_with_product_1(self): - now = time.time() - self.session["kb-visited"] = { - "/firefox/browse/": { - self.doc1.get_absolute_url(): now, - self.doc3.get_absolute_url(): now, - }, - "/firefox/mobile/settings/": { - self.doc2.get_absolute_url(): now - 15, - }, - } - self.session.modified = False - result = has_visited_kb(self.session, self.product1, ttl=10) - self.assertIs(result, True) - self.assertEqual( - self.session["kb-visited"], - { - "/firefox/browse/": { - self.doc1.get_absolute_url(): now, - self.doc3.get_absolute_url(): now, - }, - }, - ) - self.assertTrue(self.session.modified) - - def test_with_product_2(self): - now = time.time() - self.session["kb-visited"] = { - "/firefox/browse/": { - self.doc1.get_absolute_url(): now, - self.doc3.get_absolute_url(): now + 1, - }, - "/firefox/mobile/settings/": { - self.doc2.get_absolute_url(): now - 20, - }, - } - self.session.modified = False - result = has_visited_kb(self.session, self.product2, ttl=10) - self.assertIs(result, False) - self.assertEqual( - self.session["kb-visited"], - { - "/firefox/browse/": { - self.doc1.get_absolute_url(): now, - self.doc3.get_absolute_url(): now + 1, - }, - }, - ) - self.assertTrue(self.session.modified) - - def test_with_product_and_topic_1(self): - now = time.time() - self.session["kb-visited"] = { - "/firefox/browse/": { - self.doc1.get_absolute_url(): now, - self.doc3.get_absolute_url(): now - 15, - }, - "/firefox/mobile/settings/": { - self.doc2.get_absolute_url(): now + 2, - }, - } - self.session.modified = False - result = has_visited_kb(self.session, self.product1, topic=self.topic1, ttl=10) - self.assertIs(result, True) - self.assertEqual( - self.session["kb-visited"], - { - "/firefox/browse/": { - self.doc1.get_absolute_url(): now, - }, - "/firefox/mobile/settings/": { - self.doc2.get_absolute_url(): now + 2, - }, - }, - ) - self.assertTrue(self.session.modified) - - def test_with_product_and_topic_2(self): - now = time.time() - self.session["kb-visited"] = { - "/firefox/browse/": { - self.doc1.get_absolute_url(): now, - self.doc3.get_absolute_url(): now + 1, - }, - "/firefox/mobile/settings/": { - self.doc2.get_absolute_url(): now + 2, - }, - } - self.session.modified = False - result = has_visited_kb(self.session, self.product1, topic=self.topic2, ttl=10) - self.assertIs(result, True) - self.assertEqual( - self.session["kb-visited"], - { - "/firefox/browse/": { - self.doc1.get_absolute_url(): now, - self.doc3.get_absolute_url(): now + 1, - }, - "/firefox/mobile/settings/": { - self.doc2.get_absolute_url(): now + 2, - }, - }, - ) - self.assertFalse(self.session.modified) diff --git a/kitsune/wiki/utils.py b/kitsune/wiki/utils.py index 7323ef483be..66055f50c7b 100644 --- a/kitsune/wiki/utils.py +++ b/kitsune/wiki/utils.py @@ -1,12 +1,9 @@ import random -import time from itertools import chain, islice -from typing import Optional import requests from django.conf import settings from django.contrib.auth.models import User -from django.contrib.sessions.backends.base import SessionBase from django.db.models import OuterRef, Q, Subquery from django.db.models.functions import Now from django.http import Http404, HttpRequest @@ -21,9 +18,6 @@ from kitsune.wiki.models import Document, Revision -KB_VISITED_DEFAULT_TTL = 60 * 60 * 24 # 24 hours - - def active_contributors(from_date, to_date=None, locale=None, product=None): """Return active KB contributors for the specified parameters. @@ -296,102 +290,3 @@ def build_topics_data(request: HttpRequest, product: Product, topics: list[Topic topics_data.append(topic_data) return topics_data - - -def remove_expired_from_kb_visited( - session: SessionBase, ttl: int = KB_VISITED_DEFAULT_TTL -) -> None: - """ - Remove expired visits (KB article URL's and their timestamps) from the - "kb-visited" dictionary within the given session based on the given TTL. - """ - if not (session and ("kb-visited" in session)): - return - - now = time.time() - kb_visited = session["kb-visited"] - - empty_keys = [] - for key, visits in kb_visited.items(): - # Remove any visits that have expired within this products-topics key. - if expired_visits := [url for url, ts in visits.items() if (now - ts) > ttl]: - for url in expired_visits: - del visits[url] - if not visits: - # If there are no more visits under this key, - # add it to the list of keys to remove. - empty_keys.append(key) - session.modified = True - - # Remove keys that no longer contain any slugs. - for key in empty_keys: - del kb_visited[key] - - -def update_kb_visited( - session: SessionBase, doc: Document, ttl: int = KB_VISITED_DEFAULT_TTL -) -> None: - """ - Updates the "kb-visited" dictionary within the given session, and also - removes any visits that have expired based on the given TTL. - """ - if not session: - return - - remove_expired_from_kb_visited(session, ttl=ttl) - - kb_visited = session.setdefault("kb-visited", {}) - - product_slugs = doc.get_products().order_by("slug").values_list("slug", flat=True) - topic_slugs = doc.get_topics().order_by("slug").values_list("slug", flat=True) - key = f"/{'/'.join(product_slugs)}/{'/'.join(topic_slugs)}/" - - url = doc.get_absolute_url() - - kb_visited.setdefault(key, {})[url] = time.time() - session.modified = True - - -def has_visited_kb( - session: SessionBase, - product: Product, - topic: Optional[Topic] = None, - ttl: int = KB_VISITED_DEFAULT_TTL, -) -> bool: - """ - Return a boolean indicating whether or not the user has visited at least one - KB article associated with the given product and, optionally, topic, within - the given TTL. - """ - if not (session and ("kb-visited" in session)): - return False - - remove_expired_from_kb_visited(session, ttl=ttl) - - return any( - (f"/{product.slug}/" in key) and ((topic is None) or (f"/{topic.slug}/" in key)) - for key in session["kb-visited"].keys() - ) - - -def get_kb_visited( - session: SessionBase, - product: Product, - topic: Optional[Topic] = None, - ttl: int = KB_VISITED_DEFAULT_TTL, -) -> list[str]: - """ - Return the KB articles (as a list of document URL's) visited by the user that are - associated with the given product and, optionally, topic, within the given TTL. - """ - if not (session and ("kb-visited" in session)): - return [] - - remove_expired_from_kb_visited(session, ttl=ttl) - - urls = [] - for key, visits in session["kb-visited"].items(): - if (f"/{product.slug}/" in key) and ((topic is None) or (f"/{topic.slug}/" in key)): - urls.extend(visits.keys()) - - return urls diff --git a/kitsune/wiki/views.py b/kitsune/wiki/views.py index d0b85cfa18e..5c60093b584 100644 --- a/kitsune/wiki/views.py +++ b/kitsune/wiki/views.py @@ -80,11 +80,7 @@ send_contributor_notification, send_reviewed_notification, ) -from kitsune.wiki.utils import ( - get_visible_document_or_404, - get_visible_revision_or_404, - update_kb_visited, -) +from kitsune.wiki.utils import get_visible_document_or_404, get_visible_revision_or_404 log = logging.getLogger("k.wiki") @@ -313,8 +309,6 @@ def maybe_vary_on_accept_language(response): and doc.slug != "get-community-support" ) - update_kb_visited(request.session, doc) - data = { "document": doc, "is_first_revision": is_first_revision,