From 33124e2be9e370cb9c7b7e37d779375c4bc5fd3e Mon Sep 17 00:00:00 2001 From: Deimos Date: Tue, 12 Nov 2019 18:41:51 -0700 Subject: [PATCH] Improve warnings when replying to old posts Previously, the warning would only ever say "over a week old", even when the topic/comment was much older than that. This adds a new function to create a vague timedelta description for longer periods, and also enables the Javascript to use it as well through adding the description as a data attr on the reply button when a warning is needed, instead of duplicating the logic in JS. --- .../js/behaviors/comment-reply-button.js | 18 +++++------ tildes/tildes/jinja.py | 7 ++++- tildes/tildes/lib/datetime.py | 30 +++++++++++++++++++ .../tildes/templates/macros/comments.jinja2 | 11 ++++++- tildes/tildes/templates/topic.jinja2 | 4 +-- tildes/tildes/views/topic.py | 4 --- 6 files changed, 55 insertions(+), 19 deletions(-) diff --git a/tildes/static/js/behaviors/comment-reply-button.js b/tildes/static/js/behaviors/comment-reply-button.js index f09f969..32214dc 100644 --- a/tildes/static/js/behaviors/comment-reply-button.js +++ b/tildes/static/js/behaviors/comment-reply-button.js @@ -79,21 +79,17 @@ $.onmount("[data-js-comment-reply-button]", function() { } } - var parentCommentTimestamp = new Date( - $parentComment - .find(".comment-posted-time") - .first() - .attr("datetime") - ); - - // add a warning if the comment being replied to is over a week old - if (Date.now() - parentCommentTimestamp > 1000 * 3600 * 24 * 7) { + // add a warning about the comment's age, if necessary (determined by backend) + var warningAge = $(this).attr("data-js-old-warning-age"); + if (warningAge) { var warningDiv = document.createElement("div"); warningDiv.classList.add("warning-old-reply"); warningDiv.innerHTML = '

The comment you\'re replying to ' + - "is over a week old. Replying to old comments is fine as long as " + - "you're contributing to the discussion.

"; + "is " + + warningAge + + " old. Replying to old comments is fine as long as you're " + + "contributing to the discussion.

"; clone.querySelector("form").prepend(warningDiv); } diff --git a/tildes/tildes/jinja.py b/tildes/tildes/jinja.py index b881e3a..2065dcf 100644 --- a/tildes/tildes/jinja.py +++ b/tildes/tildes/jinja.py @@ -8,7 +8,11 @@ from urllib.parse import quote_plus from pyramid.config import Configurator -from tildes.lib.datetime import adaptive_date, descriptive_timedelta +from tildes.lib.datetime import ( + adaptive_date, + descriptive_timedelta, + vague_timedelta_description, +) from tildes.models.comment import Comment from tildes.models.group import Group from tildes.models.topic import Topic @@ -46,6 +50,7 @@ def includeme(config: Configurator) -> None: "adaptive_date": adaptive_date, "ago": descriptive_timedelta, "quote_plus": do_quote_plus, + "vague_timedelta_description": vague_timedelta_description, } # add custom jinja tests diff --git a/tildes/tildes/lib/datetime.py b/tildes/tildes/lib/datetime.py index ec4863f..2d0f332 100644 --- a/tildes/tildes/lib/datetime.py +++ b/tildes/tildes/lib/datetime.py @@ -137,6 +137,36 @@ def descriptive_timedelta( return result +def vague_timedelta_description(delta: timedelta) -> str: + """Vaguely describe how long a timedelta refers to, like "over 2 weeks".""" + if delta.days < 1: + raise ValueError("The timedelta must be at least a day.") + + day_thresholds = { + "year": 365, + "month": 30, + "week": 7, + "day": 1, + } + + for threshold, days in day_thresholds.items(): + if delta.days >= days: + largest_threshold = threshold + break + + count = str(delta.days // day_thresholds[largest_threshold]) + + # set the strings we'll output based on count - change count of 1 to "a", otherwise + # pluralize the threshold name (e.g. change "week" to "weeks") + if count == "1": + count = "a" + period = largest_threshold + else: + period = largest_threshold + "s" + + return f"over {count} {period}" + + def adaptive_date( target: datetime, abbreviate: bool = False, precision: Optional[int] = None ) -> str: diff --git a/tildes/tildes/templates/macros/comments.jinja2 b/tildes/tildes/templates/macros/comments.jinja2 index 8944ca4..aa1b89a 100644 --- a/tildes/tildes/templates/macros/comments.jinja2 +++ b/tildes/tildes/templates/macros/comments.jinja2 @@ -212,7 +212,16 @@ {% endif %} {% if request.has_permission('reply', comment) %} -
    • +
  • + +
    • {% endif %} {% endif %} diff --git a/tildes/tildes/templates/topic.jinja2 b/tildes/tildes/templates/topic.jinja2 index fd07a7d..36c2f90 100644 --- a/tildes/tildes/templates/topic.jinja2 +++ b/tildes/tildes/templates/topic.jinja2 @@ -227,9 +227,9 @@ data-js-prevent-double-submit data-js-confirm-leave-page-unsaved > - {% if old_topic_warning %} + {% if topic.age.days >= 7 %}
    -

    This topic is over a week old. Replying to old topics is fine as long as you're contributing to the discussion.

    +

    This topic is {{ topic.age|vague_timedelta_description }} old. Replying to old topics is fine as long as you're contributing to the discussion.

    {% endif %} diff --git a/tildes/tildes/views/topic.py b/tildes/tildes/views/topic.py index d62ee34..3d9c028 100644 --- a/tildes/tildes/views/topic.py +++ b/tildes/tildes/views/topic.py @@ -7,7 +7,6 @@ from collections import namedtuple from decimal import Decimal from typing import Any, Dict, Optional, Union -from datetime import timedelta from marshmallow import missing, ValidationError from marshmallow.fields import Boolean, String from pyramid.httpexceptions import HTTPFound @@ -422,8 +421,6 @@ def get_topic(request: Request, comment_order: CommentTreeSortOption) -> dict: tree.uncollapse_new_comments(topic.last_visit_time) tree.finalize_collapsing_maximized() - old_topic_warning = topic.age > timedelta(days=7) - return { "topic": topic, "log": log, @@ -431,7 +428,6 @@ def get_topic(request: Request, comment_order: CommentTreeSortOption) -> dict: "comment_order": comment_order, "comment_order_options": CommentTreeSortOption, "comment_label_options": CommentLabelOption, - "old_topic_warning": old_topic_warning, }