Enable "mark new comments" for all users

Previously, this feature was disabled by default. However, despite being
one of the best features on the site, only about 10% of users ever
enabled it, and even very involved/frequent users often didn't realize
it existed.

My original thought about why it should be opt-in only is that I thought
it had a meaningful privacy impact, but it really doesn't. User visits
to topics are already tracked through server logs and similar data, so
the feature doesn't really make any difference.

This commit enables the feature for everyone, removes the separate
Settings page, and moves the "Collapse old comments" sub-setting onto
the main Settings page.

tildes/alembic/versions/6c840340ab86_drop_track_comment_visits_column_on_.py

@@ -0,0 +1,32 @@
+"""Drop track_comment_visits column on users
+
+Revision ID: 6c840340ab86
+Revises: cc12ea6c616d
+Create Date: 2020-01-27 21:42:25.565355
+
+"""
+from alembic import op
+import sqlalchemy as sa
+
+
+# revision identifiers, used by Alembic.
+revision = "6c840340ab86"
+down_revision = "cc12ea6c616d"
+branch_labels = None
+depends_on = None
+
+
+def upgrade():
+    op.drop_column("users", "track_comment_visits")
+
+
+def downgrade():
+    op.add_column(
+        "users",
+        sa.Column(
+            "track_comment_visits",
+            sa.BOOLEAN(),
+            server_default=sa.text("false"),
+            nullable=False,
+        ),
+    )

tildes/scripts/clean_private_data.py

@@ -192,7 +192,6 @@ class DataCleaner:
                     "two_factor_backup_codes": DEFAULT,
                     "inviter_id": DEFAULT,
                     "invite_codes_remaining": DEFAULT,
-                    "track_comment_visits": DEFAULT,
                     "collapse_old_comments": DEFAULT,
                     "auto_mark_notifications_read": DEFAULT,
                     "open_new_tab_external": DEFAULT,

tildes/tildes/lib/message.py

@@ -22,7 +22,6 @@ There's a page on the Docs site that explains the basic mechanics on Tildes: htt
 There are multiple useful links in the sidebar on your user page—get there by clicking your username in the top right, or in the sidebar if you're on mobile. You can access the settings page from there, which includes multiple things you'll probably want to do:
 
 * Check the available options for display themes (including dark themes)
-* [Enable marking new comments in threads](https://tildes.net/settings/comment_visits)
 * [Set up account recovery in case you lose access to your account](https://tildes.net/settings/account_recovery)
 
 # Please post topics and comments

tildes/tildes/models/topic/topic_query.py

@@ -7,7 +7,7 @@ from typing import Any, Sequence
 
 from pyramid.request import Request
 from sqlalchemy import func
-from sqlalchemy.sql.expression import and_, label, null
+from sqlalchemy.sql.expression import and_, label
 
 from tildes.enums import TopicSortOption
 from tildes.lib.datetime import SimpleHoursPeriod, utc_now
@@ -94,8 +94,6 @@ class TopicQuery(PaginatedQuery):
 
     def _attach_visit_data(self) -> "TopicQuery":
         """Join the data related to the user's last visit to the topic(s)."""
-        # pylint: disable=assignment-from-no-return
-        if self.request.user.track_comment_visits:
         query = self.outerjoin(
             TopicVisit,
             and_(
@@ -104,11 +102,6 @@ class TopicQuery(PaginatedQuery):
             ),
         )
         query = query.add_columns(TopicVisit.visit_time, TopicVisit.num_comments)
-        else:
-            # if the user has the feature disabled, just add literal NULLs
-            query = self.add_columns(
-                null().label("visit_time"), null().label("num_comments")
-            )
 
         return query
 

tildes/tildes/models/user/user.py

@@ -89,7 +89,6 @@ class User(DatabaseModel):
     num_unread_notifications: int = Column(Integer, nullable=False, server_default="0")
     inviter_id: int = Column(Integer, ForeignKey("users.user_id"))
     invite_codes_remaining: int = Column(Integer, nullable=False, server_default="0")
-    track_comment_visits: bool = Column(Boolean, nullable=False, server_default="false")
     collapse_old_comments: bool = Column(Boolean, nullable=False, server_default="true")
     auto_mark_notifications_read: bool = Column(
         Boolean, nullable=False, server_default="false"

tildes/tildes/routes.py

@@ -93,9 +93,6 @@ def includeme(config: Configurator) -> None:
             "/two_factor/qr_code",
             factory=LoggedInFactory,
         )
-        config.add_route(
-            "settings_comment_visits", "/comment_visits", factory=LoggedInFactory
-        )
         config.add_route("settings_filters", "/filters", factory=LoggedInFactory)
         config.add_route("settings_bio", "/bio", factory=LoggedInFactory)
         config.add_route(

tildes/tildes/schemas/user.py

@@ -8,7 +8,7 @@ from typing import Any
 
 from marshmallow import post_dump, pre_load, Schema, validates, validates_schema
 from marshmallow.exceptions import ValidationError
-from marshmallow.fields import Boolean, DateTime, Email, String
+from marshmallow.fields import DateTime, Email, String
 from marshmallow.validate import Length, Regexp
 
 from tildes.lib.password import is_breached_password
@@ -57,7 +57,6 @@ class UserSchema(Schema):
     email_address = Email(allow_none=True, load_only=True)
     email_address_note = String(validate=Length(max=EMAIL_ADDRESS_NOTE_MAX_LENGTH))
     created_time = DateTime(dump_only=True)
-    track_comment_visits = Boolean()
     bio_markdown = Markdown(max_length=BIO_MAX_LENGTH, allow_none=True)
 
     @post_dump

tildes/tildes/templates/settings.jinja2

@@ -74,8 +74,24 @@
 
   <ul class="settings-list">
     <li>
-      <a href="/settings/comment_visits">Configure marking new comments (currently {{ 'enabled' if request.user.track_comment_visits else 'disabled' }})</a>
-      <div class="text-small text-secondary">Marks new comments in topics since your last visit, and which topics have any</div>
+      <form
+        name="collapse-old-comments"
+        autocomplete="off"
+        data-ic-patch-to="{{ request.route_url('ic_user', username=request.user.username) }}"
+      >
+        <div class="form-group">
+          <label class="form-checkbox">
+            <input
+              type="checkbox"
+              id="collapse_old_comments"
+              name="collapse_old_comments"
+              data-js-autosubmit-on-change
+              {% if request.user.collapse_old_comments %}checked{% endif %}
+            >
+            <i class="form-icon"></i> Collapse old comments when I return to a topic
+          </label>
+        </div>
+      </form>
     </li>
 
     <li>

tildes/tildes/templates/settings_comment_visits.jinja2

@@ -1,63 +0,0 @@
-{# Copyright (c) 2018 Tildes contributors <code@tildes.net> #}
-{# SPDX-License-Identifier: AGPL-3.0-or-later #}
-
-{% extends 'base_settings.jinja2' %}
-
-{% block title %}Toggle marking new comments{% endblock %}
-
-{% block main_heading %}Toggle marking new comments{% endblock %}
-
-{% block settings %}
-<figure>
-  <figcaption>How new comments are displayed</figcaption>
-  <img src="/images/mark-new-comments.png" alt="Examples of how new comments are displayed">
-</figure>
-
-<p>Tildes can mark which comments were posted since your previous visit to a topic's comments (and which topics have any new ones), but doing so requires keeping track of when that previous visit happened. This has a privacy implication, so the feature is opt-in.</p>
-
-<p>While this feature is enabled, we will record and store data about your most recent visit to each topic's comments. We store only the single most recent visit&mdash;any previous visit data for that topic is replaced if you visit the same one again later.</p>
-
-<p>This data is retained for 30 days. After not visiting a particular topic for 30 days, the data about your last visit to it will be deleted.</p>
-
-<p>Disabling the feature will stop marking comments but will not delete any existing data, only prevent new data from being stored. The previously-stored data will be deleted after 30 days, as usual.</p>
-
-<div class="divider"></div>
-
-<form
-  name="comment-visits"
-  autocomplete="off"
-  data-ic-patch-to="{{ request.route_url('ic_user', username=request.user.username) }}"
-  data-js-autosubmit-on-change
->
-  <div class="form-group">
-    <label class="form-checkbox">
-      <input
-        type="checkbox"
-        id="track_comment_visits"
-        name="track_comment_visits"
-        {% if request.user.track_comment_visits %}checked{% endif %}
-      >
-      <i class="form-icon"></i> Track my last visit to each topic's comments and mark new comments
-    </label>
-  </div>
-</form>
-
-<form
-  name="collapse-old-comments"
-  autocomplete="off"
-  data-ic-patch-to="{{ request.route_url('ic_user', username=request.user.username) }}"
->
-  <div class="form-group">
-    <label class="form-checkbox">
-      <input
-        type="checkbox"
-        id="collapse_old_comments"
-        name="collapse_old_comments"
-        data-js-autosubmit-on-change
-        {% if request.user.collapse_old_comments %}checked{% endif %}
-      >
-      <i class="form-icon"></i> Collapse old comments when I return to a topic (no effect unless the overall setting is enabled)
-    </label>
-  </div>
-</form>
-{% endblock %}

tildes/tildes/templates/topic.jinja2

@@ -188,10 +188,9 @@
       <div class="btn-group">
         <button class="btn btn-sm btn-light" data-js-comment-collapse-all-button>Collapse replies</button>
 
-        {# Display the "Collapse read" button if the user is tracking comment visits
-           but not automatically collapsing old comments, and there are new comments #}
+        {# Display the "Collapse read" button if the user is not automatically
+           collapsing old comments, and there are new comments #}
         {% if request.user
-             and request.user.track_comment_visits
              and not request.user.collapse_old_comments
              and topic.comments_since_last_visit %}
           <button class="btn btn-sm btn-light" data-js-comment-collapse-read-button>Collapse read</button>

tildes/tildes/views/api/web/comment.py

@@ -50,15 +50,13 @@ def _mark_comment_read_from_interaction(request: Request, comment: Comment) -> N
 def _increment_topic_comments_seen(request: Request, comment: Comment) -> None:
     """Increment the number of comments in a topic the user has viewed.
 
-    If the user has the "track comment visits" feature enabled, we want to increment the
-    number of comments they've seen in the thread that the comment came from, so that
-    they don't *both* get a notification as well as have the thread highlight with "(1
-    new)". This should only happen if their last visit was before the comment was
-    posted, however.  Below, this is implemented as a INSERT ... ON CONFLICT DO UPDATE
-    so that it will insert a new topic visit with 1 comment if they didn't previously
-    have one at all.
+    We want to increment the number of comments they've seen in the thread that the
+    comment came from, so that they don't *both* get a notification as well as have the
+    thread highlight with "(1 new)". This should only happen if their last visit was
+    before the comment was posted, however.  Below, this is implemented as a
+    INSERT ... ON CONFLICT DO UPDATE so that it will insert a new topic visit with
+    1 comment if they didn't previously have one at all.
     """
-    if request.user.track_comment_visits:
     statement = (
         insert(TopicVisit.__table__)
         .values(

tildes/tildes/views/api/web/user.py

@@ -213,22 +213,6 @@ def patch_change_open_links_new_tab(request: Request) -> Response:
     return IC_NOOP
 
 
-@ic_view_config(
-    route_name="user",
-    request_method="PATCH",
-    request_param="ic-trigger-name=comment-visits",
-    permission="change_comment_visits_setting",
-)
-def patch_change_track_comment_visits(request: Request) -> Response:
-    """Change the user's "track comment visits" setting."""
-    user = request.context
-
-    track_comment_visits = bool(request.params.get("track_comment_visits"))
-    user.track_comment_visits = track_comment_visits
-
-    return IC_NOOP
-
-
 @ic_view_config(
     route_name="user",
     request_method="PATCH",

tildes/tildes/views/settings.py

@@ -86,15 +86,6 @@ def get_settings_two_factor(request: Request) -> dict:
     }
 
 
-@view_config(
-    route_name="settings_comment_visits", renderer="settings_comment_visits.jinja2"
-)
-def get_settings_comment_visits(request: Request) -> dict:
-    """Generate the comment visits settings page."""
-    # pylint: disable=unused-argument
-    return {}
-
-
 @view_config(route_name="settings_filters", renderer="settings_filters.jinja2")
 def get_settings_filters(request: Request) -> dict:
     """Generate the filters settings page."""

tildes/tildes/views/topic.py

@@ -440,8 +440,7 @@ def get_topic(request: Request, comment_order: CommentTreeSortOption) -> dict:
 
     tree.collapse_from_labels()
 
-    # if the user has the "mark new comments" feature enabled
-    if request.user and request.user.track_comment_visits:
+    if request.user:
         # update their last visit time for this topic
         statement = TopicVisit.generate_insert_statement(request.user, topic)
         request.db_session.execute(statement)