Merge branch 'master' into feature-group-404

tildes/alembic/versions/50c251c4a19c_add_search_column_index_for_topics.py

@@ -0,0 +1,63 @@
+"""Add search column/index for topics
+
+Revision ID: 50c251c4a19c
+Revises: d33fb803a153
+Create Date: 2018-08-20 19:18:04.129255
+
+"""
+from alembic import op
+import sqlalchemy as sa
+from sqlalchemy.dialects import postgresql
+
+# revision identifiers, used by Alembic.
+revision = "50c251c4a19c"
+down_revision = "d33fb803a153"
+branch_labels = None
+depends_on = None
+
+
+def upgrade():
+    op.add_column(
+        "topics", sa.Column("search_tsv", postgresql.TSVECTOR(), nullable=True)
+    )
+    op.create_index(
+        "ix_topics_search_tsv_gin",
+        "topics",
+        ["search_tsv"],
+        unique=False,
+        postgresql_using="gin",
+    )
+
+    op.execute(
+        """
+        UPDATE topics
+        SET search_tsv = to_tsvector('pg_catalog.english', title)
+                || to_tsvector('pg_catalog.english', COALESCE(markdown, ''));
+    """
+    )
+
+    op.execute(
+        """
+        CREATE TRIGGER topic_update_search_tsv_insert
+            BEFORE INSERT ON topics
+            FOR EACH ROW
+            EXECUTE PROCEDURE tsvector_update_trigger(search_tsv, 'pg_catalog.english', title, markdown);
+
+        CREATE TRIGGER topic_update_search_tsv_update
+            BEFORE UPDATE ON topics
+            FOR EACH ROW
+            WHEN (
+                (OLD.title IS DISTINCT FROM NEW.title)
+                OR (OLD.markdown IS DISTINCT FROM NEW.markdown)
+            )
+            EXECUTE PROCEDURE tsvector_update_trigger(search_tsv, 'pg_catalog.english', title, markdown);
+    """
+    )
+
+
+def downgrade():
+    op.drop_index("ix_topics_search_tsv_gin", table_name="topics")
+    op.drop_column("topics", "search_tsv")
+
+    op.execute("DROP TRIGGER topic_update_search_tsv_insert ON topics")
+    op.execute("DROP TRIGGER topic_update_search_tsv_update ON topics")

tildes/requirements-to-freeze.txt

@@ -37,6 +37,7 @@ SQLAlchemy
 SQLAlchemy-Utils
 stripe
 testing.redis
+titlecase
 webargs
 webtest
 zope.sqlalchemy

tildes/requirements.txt

@@ -83,6 +83,7 @@ SQLAlchemy-Utils==0.33.3
 stripe==2.4.0
 testing.common.database==2.0.3
 testing.redis==1.1.1
+titlecase==0.12.0
 toml==0.9.4
 traitlets==4.3.2
 transaction==2.2.1

tildes/scss/modules/_btn.scss

@@ -52,10 +52,8 @@
 
   font-weight: normal;
 
-  border-left-width: 0;
   margin-right: 0.4rem;
   @media (min-width: $size-md) {
-    border-left-width: 1px;
     margin-right: 0.2rem;
     min-width: 0.8rem;
   }

tildes/scss/modules/_sidebar.scss

@@ -11,6 +11,14 @@
   .sidebar-controls .btn {
     width: auto;
   }
+
+  .form-search {
+    margin-bottom: 1rem;
+
+    .btn {
+      font-weight: normal;
+    }
+  }
 }
 
 .sidebar-controls {

tildes/scss/modules/_site-header.scss

@@ -12,6 +12,7 @@
 }
 
 .site-header-context {
+  white-space: nowrap;
   overflow: hidden;
   text-overflow: ellipsis;
 }

tildes/scss/modules/_topic.scss

@@ -245,17 +245,21 @@
   overflow: auto;
 }
 
-.topic-comments {
-  header {
-    display: flex;
+.topic-comments-header {
+  display: flex;
+  flex-wrap: wrap;
+  align-items: center;
 
-    h2 {
-      white-space: nowrap;
-    }
+  margin-bottom: 0.4rem;
 
-    .form-listing-options {
-      margin-left: auto;
-    }
+  h2 {
+    margin-bottom: 0;
+    margin-right: 0.4rem;
+    white-space: nowrap;
+  }
+
+  .form-listing-options {
+    margin-left: auto;
   }
 }
 

tildes/sql/init/triggers/topics/topics.sql

@@ -12,3 +12,18 @@ CREATE TRIGGER delete_topic_set_deleted_time_update
     FOR EACH ROW
     WHEN (OLD.is_deleted = false AND NEW.is_deleted = true)
     EXECUTE PROCEDURE set_topic_deleted_time();
+
+
+CREATE TRIGGER topic_update_search_tsv_insert
+    BEFORE INSERT ON topics
+    FOR EACH ROW
+    EXECUTE PROCEDURE tsvector_update_trigger(search_tsv, 'pg_catalog.english', title, markdown);
+
+CREATE TRIGGER topic_update_search_tsv_update
+    BEFORE UPDATE ON topics
+    FOR EACH ROW
+    WHEN (
+        (OLD.title IS DISTINCT FROM NEW.title)
+        OR (OLD.markdown IS DISTINCT FROM NEW.markdown)
+    )
+    EXECUTE PROCEDURE tsvector_update_trigger(search_tsv, 'pg_catalog.english', title, markdown);

tildes/static/js/behaviors/comment-collapse-all-button.js

@@ -0,0 +1,11 @@
+$.onmount('[data-js-comment-collapse-all-button]', function() {
+    $(this).click(function(event) {
+        $('.comment[data-comment-depth="1"]:not(.is-comment-collapsed)').each(
+            function(idx, child) {
+                $(child).find(
+                    '[data-js-comment-collapse-button]:first').trigger('click');
+            });
+
+        $(this).blur();
+    });
+});

tildes/static/js/behaviors/comment-expand-all-button.js

@@ -0,0 +1,11 @@
+$.onmount('[data-js-comment-expand-all-button]', function() {
+    $(this).click(function(event) {
+        $('.comment.is-comment-collapsed').each(
+            function(idx, child) {
+                $(child).find(
+                    '[data-js-comment-collapse-button]:first').trigger('click');
+            });
+
+        $(this).blur();
+    });
+});

tildes/tests/test_topic.py

@@ -130,3 +130,19 @@ def test_multiple_edits_update_time(text_topic):
 def test_topic_initial_last_activity_time(text_topic):
     """Ensure last_activity_time is initially the same as created_time."""
     assert text_topic.last_activity_time == text_topic.created_time
+
+
+def test_convert_all_caps(session_user, session_group):
+    """Ensure that all-caps titles are converted to title case."""
+    topic = Topic.create_link_topic(
+        session_group, session_user, "THE TITLE", "http://example.com"
+    )
+    assert topic.title == "The Title"
+
+
+def test_no_convert_partial_caps_title(session_user, session_group):
+    """Ensure that partially-caps titles are not converted to title case."""
+    topic = Topic.create_link_topic(
+        session_group, session_user, "This is a TITLE", "http://example.com"
+    )
+    assert topic.title == "This is a TITLE"

tildes/tildes/__init__.py

@@ -1,9 +1,10 @@
 """Configure and initialize the Pyramid app."""
 
-from typing import Any, Callable, Dict, Optional
+from typing import Any, Callable, Dict, Optional, Tuple
 
 from paste.deploy.config import PrefixMiddleware
 from pyramid.config import Configurator
+from pyramid.httpexceptions import HTTPTooManyRequests
 from pyramid.registry import Registry
 from pyramid.request import Request
 from redis import StrictRedis
@@ -50,6 +51,7 @@ def main(global_config: Dict[str, str], **settings: str) -> PrefixMiddleware:
     # pylint: enable=unnecessary-lambda
 
     config.add_request_method(check_rate_limit, "check_rate_limit")
+    config.add_request_method(apply_rate_limit, "apply_rate_limit")
 
     config.add_request_method(current_listing_base_url, "current_listing_base_url")
     config.add_request_method(current_listing_normal_url, "current_listing_normal_url")
@@ -120,6 +122,13 @@ def check_rate_limit(request: Request, action_name: str) -> RateLimitResult:
     return RateLimitResult.merged_result(results)
 
 
+def apply_rate_limit(request: Request, action_name: str) -> None:
+    """Check the rate limit for an action, and raise HTTP 429 if it's exceeded."""
+    result = request.check_rate_limit(action_name)
+    if not result.is_allowed:
+        raise result.add_headers_to_response(HTTPTooManyRequests())
+
+
 def current_listing_base_url(
     request: Request, query: Optional[Dict[str, Any]] = None
 ) -> str:
@@ -130,10 +139,18 @@ def current_listing_base_url(
 
     The `query` argument allows adding query variables to the generated url.
     """
-    if request.matched_route.name not in ("home", "group", "user"):
+    base_vars_by_route: Dict[str, Tuple[str, ...]] = {
+        "group": ("order", "period", "per_page", "tag", "unfiltered"),
+        "home": ("order", "period", "per_page", "tag", "unfiltered"),
+        "search": ("order", "period", "per_page", "q"),
+        "user": ("per_page", "type"),
+    }
+
+    try:
+        base_view_vars = base_vars_by_route[request.matched_route.name]
+    except KeyError:
         raise AttributeError("Current route is not supported.")
 
-    base_view_vars = ("order", "period", "per_page", "tag", "type", "unfiltered")
     query_vars = {
         key: val for key, val in request.GET.copy().items() if key in base_view_vars
     }
@@ -156,10 +173,18 @@ def current_listing_normal_url(
 
     The `query` argument allows adding query variables to the generated url.
     """
-    if request.matched_route.name not in ("home", "group", "user"):
+    normal_vars_by_route: Dict[str, Tuple[str, ...]] = {
+        "group": ("order", "period", "per_page"),
+        "home": ("order", "period", "per_page"),
+        "search": ("order", "period", "per_page", "q"),
+        "user": ("per_page",),
+    }
+
+    try:
+        normal_view_vars = normal_vars_by_route[request.matched_route.name]
+    except KeyError:
         raise AttributeError("Current route is not supported.")
 
-    normal_view_vars = ("order", "period", "per_page")
     query_vars = {
         key: val for key, val in request.GET.copy().items() if key in normal_view_vars
     }

tildes/tildes/lib/ratelimit.py

@@ -281,6 +281,8 @@ _RATE_LIMITED_ACTIONS = (
     RateLimitedAction("login", timedelta(hours=1), 20),
     RateLimitedAction("login_two_factor", timedelta(hours=1), 20),
     RateLimitedAction("register", timedelta(hours=1), 50),
+    RateLimitedAction("topic_post", timedelta(hours=1), 6, max_burst=4),
+    RateLimitedAction("comment_post", timedelta(hours=1), 30, max_burst=20),
 )
 
 # (public) dict to be able to look up the actions by name

tildes/tildes/models/topic/topic.py

@@ -14,12 +14,13 @@ from sqlalchemy import (
     Text,
     TIMESTAMP,
 )
-from sqlalchemy.dialects.postgresql import ENUM, JSONB
+from sqlalchemy.dialects.postgresql import ENUM, JSONB, TSVECTOR
 from sqlalchemy.ext.hybrid import hybrid_property
 from sqlalchemy.ext.mutable import MutableDict
 from sqlalchemy.orm import deferred, relationship
 from sqlalchemy.sql.expression import text
 from sqlalchemy_utils import Ltree
+from titlecase import titlecase
 
 from tildes.enums import TopicType
 from tildes.lib.database import ArrayOfLtree
@@ -111,12 +112,16 @@ class Topic(DatabaseModel):
     )
     is_official: bool = Column(Boolean, nullable=False, server_default="false")
     is_locked: bool = Column(Boolean, nullable=False, server_default="false")
+    search_tsv: Any = deferred(Column(TSVECTOR))
 
     user: User = relationship("User", lazy=False, innerjoin=True)
     group: Group = relationship("Group", innerjoin=True)
 
-    # Create a GiST index on the tags column
-    __table_args__ = (Index("ix_topics_tags_gist", _tags, postgresql_using="gist"),)
+    # Create specialized indexes
+    __table_args__ = (
+        Index("ix_topics_tags_gist", _tags, postgresql_using="gist"),
+        Index("ix_topics_search_tsv_gin", "search_tsv", postgresql_using="gin"),
+    )
 
     @hybrid_property
     def markdown(self) -> Optional[str]:
@@ -168,6 +173,11 @@ class Topic(DatabaseModel):
         new_topic = cls()
         new_topic.group_id = group.group_id
         new_topic.user_id = author.user_id
+
+        # if the title is all caps, convert to title case
+        if title.isupper():
+            title = titlecase(title)
+
         new_topic.title = title
 
         return new_topic

tildes/tildes/models/topic/topic_query.py

@@ -3,6 +3,7 @@
 from typing import Any, Sequence
 
 from pyramid.request import Request
+from sqlalchemy import func
 from sqlalchemy.sql.expression import and_, null
 from sqlalchemy_utils import Ltree
 
@@ -137,3 +138,7 @@ class TopicQuery(PaginatedQuery):
 
         # pylint: disable=protected-access
         return self.filter(Topic._tags.descendant_of(tag))  # type: ignore
+
+    def search(self, query: str) -> "TopicQuery":
+        """Restrict the topics to ones that match a search query (generative)."""
+        return self.filter(Topic.search_tsv.op("@@")(func.plainto_tsquery(query)))

tildes/tildes/routes.py

@@ -17,6 +17,8 @@ def includeme(config: Configurator) -> None:
     """Set up application routes."""
     config.add_route("home", "/")
 
+    config.add_route("search", "/search")
+
     config.add_route("groups", "/groups")
 
     config.add_route("login", "/login")

tildes/tildes/templates/home.jinja2

@@ -17,6 +17,13 @@
 {% endblock %}
 
 {% block sidebar %}
+  <form class="form-search" method="GET" action="/search">
+    <div class="input-group">
+      <input type="text" class="form-input input-sm" name="q" id="q">
+      <button class="btn btn-sm input-group-btn">Search</button>
+    </div>
+  </form>
+
   <h2>Home</h2>
   <p>The home page shows topics from groups that you're subscribed to.</p>
   {% if request.user %}

tildes/tildes/templates/search.jinja2

@@ -0,0 +1,31 @@
+{% extends 'topic_listing.jinja2' %}
+
+{% block title %}Search results for "{{ search }}"{% endblock %}
+
+{% block header_context_link %}<span class="site-header-context">Search: "{{ search }}"</span>{% endblock %}
+
+{% block sidebar %}
+  <form class="form-search" method="GET" action="/search">
+    <div class="input-group">
+      <input type="text" class="form-input input-sm" name="q" id="q" value="{{ search }}">
+      <button class="btn btn-sm input-group-btn">Search</button>
+    </div>
+  </form>
+
+  <h2>Search results</h2>
+
+  <p><a href="/">Back to home page</a></p>
+
+  {% if request.user and request.user.subscriptions %}
+    <div class="divider"></div>
+    <ul class="nav">
+      <li>Subscriptions</li>
+      <ul class="nav">
+      {% for subscription in request.user.subscriptions|sort(attribute='group') %}
+        <li class="nav-item"><a href="/~{{ subscription.group.path }}">~{{ subscription.group.path }}</a></li>
+      {% endfor %}
+      </ul>
+    </ul>
+  {% endif %}
+
+{% endblock %}

tildes/tildes/templates/topic.jinja2

@@ -144,7 +144,7 @@
 
 {% if topic.num_comments > 0 %}
   <section class="topic-comments">
-    <header>
+    <header class="topic-comments-header">
       <h2>
       {% trans num_comments=topic.num_comments %}
       {{ num_comments }} comment
@@ -153,6 +153,9 @@
       {% endtrans %}
       </h2>
 
+      <button class="btn btn-comment-collapse" title="Collapse reply comments" data-js-comment-collapse-all-button>&minus;</button>
+      <button class="btn btn-comment-collapse" title="Expand all comments" data-js-comment-expand-all-button>+</button>
+
       <form class="form-listing-options" method="get">
         <div class="form-group">
           <label for="comment_order">Comments sorted by</label>
@@ -166,12 +169,11 @@
               >{{ option.description }}</option>
             {% endfor %}
           </select>
+          {# add a submit button for people with js disabled so this is still usable #}
+          <noscript>
+            <button type="submit" class="btn btn-primary btn-sm">OK</button>
+          </noscript>
         </div>
-
-        {# add a submit button for people with js disabled so this is still usable #}
-        <noscript>
-          <button type="submit" class="btn btn-primary btn-sm">OK</button>
-        </noscript>
       </form>
     </header>
 

tildes/tildes/templates/topic_listing.jinja2

@@ -46,9 +46,14 @@
 
   <form class="form-listing-options" method="get">
     <input type="hidden" name="order" value="{{ order.name.lower() }}">
-    {% if tag %}
+    {% if tag is defined and tag %}
       <input type="hidden" name="tag" value="{{ tag }}">
     {% endif %}
+
+    {% if search is defined %}
+      <input type="hidden" name="q" value="{{ search }}">
+    {% endif %}
+
     <div class="form-group">
       <label for="period">from</label>
       <select id="period" name="period" class="form-select" data-js-time-period-select>
@@ -64,15 +69,14 @@
         <option value="all"{{ ' selected' if not period else '' }}>all time</option>
         <option value="other">other period</option>
       </select>
+      {# add a submit button for people with js disabled so this is still usable #}
+      <noscript>
+        <button type="submit" class="btn btn-primary btn-sm">OK</button>
+      </noscript>
     </div>
-
-    {# add a submit button for people with js disabled so this is still usable #}
-    <noscript>
-      <button type="submit" class="btn btn-primary btn-sm">OK</button>
-    </noscript>
   </form>
 
-  {% if not is_default_view %}
+  {% if is_default_view is defined and not is_default_view %}
     <form
       {% if is_single_group %}
         data-ic-patch-to="{{ request.route_url(
@@ -96,6 +100,7 @@
   {% endif %}
 </div>
 
+{% if search is not defined %}
 <div class="topic-listing-filter">
 {% if tag %}
   Showing only topics with the tag "{{ tag|replace('_', ' ') }}".
@@ -108,6 +113,7 @@
   <a href="{{ request.current_listing_normal_url({'unfiltered': 'true'}) }}">View unfiltered list</a>
 {% endif %}
 </div>
+{% endif %}
 
 {% if not topics %}
   <div class="empty">
@@ -125,7 +131,7 @@
 
 {% if topics %}
   <ol class="topic-listing"
-  {% if rank_start is not none %}
+  {% if rank_start is defined and rank_start is not none %}
     start="{{ rank_start }}"
   {% endif %}
   >
@@ -134,7 +140,7 @@
     <li>
 
     {# only display the rank on topics if the rank_start variable is set #}
-    {% if rank_start is not none %}
+    {% if rank_start is defined and rank_start is not none %}
       {{ render_topic_for_listing(
           topic,
           show_group=topic.group != request.context,
@@ -169,6 +175,13 @@
 {% endblock %}
 
 {% block sidebar %}
+  <form class="form-search" method="GET" action="/search">
+    <div class="input-group">
+      <input type="text" class="form-input input-sm" name="q" id="q">
+      <button class="btn btn-sm input-group-btn">Search</button>
+    </div>
+  </form>
+
   <h3>~{{ group.path }}</h3>
 
   {% if group.short_description %}

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

@@ -16,7 +16,7 @@ from tildes.models.comment import Comment, CommentNotification, CommentTag, Comm
 from tildes.models.topic import TopicVisit
 from tildes.schemas.comment import CommentSchema, CommentTagSchema
 from tildes.views import IC_NOOP
-from tildes.views.decorators import ic_view_config
+from tildes.views.decorators import ic_view_config, rate_limit_view
 
 
 def _increment_topic_comments_seen(request: Request, comment: Comment) -> None:
@@ -57,6 +57,7 @@ def _increment_topic_comments_seen(request: Request, comment: Comment) -> None:
     permission="comment",
 )
 @use_kwargs(CommentSchema(only=("markdown",)))
+@rate_limit_view("comment_post")
 def post_toplevel_comment(request: Request, markdown: str) -> dict:
     """Post a new top-level comment on a topic with Intercooler."""
     topic = request.context
@@ -90,6 +91,7 @@ def post_toplevel_comment(request: Request, markdown: str) -> dict:
     permission="reply",
 )
 @use_kwargs(CommentSchema(only=("markdown",)))
+@rate_limit_view("comment_post")
 def post_comment_reply(request: Request, markdown: str) -> dict:
     """Post a reply to a comment with Intercooler."""
     parent_comment = request.context

tildes/tildes/views/decorators.py

@@ -2,7 +2,7 @@
 
 from typing import Any, Callable
 
-from pyramid.httpexceptions import HTTPFound, HTTPTooManyRequests
+from pyramid.httpexceptions import HTTPFound
 from pyramid.request import Request
 from pyramid.view import view_config
 
@@ -35,10 +35,8 @@ def rate_limit_view(action_name: str) -> Callable:
     def decorator(func: Callable) -> Callable:
         def wrapper(*args: Any, **kwargs: Any) -> Any:
             request = args[0]
-            result = request.check_rate_limit(action_name)
 
-            if not result.is_allowed:
-                raise result.add_headers_to_response(HTTPTooManyRequests())
+            request.apply_rate_limit(action_name)
 
             return func(*args, **kwargs)
 

tildes/tildes/views/topic.py

@@ -30,6 +30,7 @@ from tildes.schemas.comment import CommentSchema
 from tildes.schemas.fields import Enum, ShortTimePeriod
 from tildes.schemas.topic import TopicSchema
 from tildes.schemas.topic_listing import TopicListingSchema
+from tildes.views.decorators import rate_limit_view
 
 
 DefaultSettings = namedtuple("DefaultSettings", ["order", "period"])
@@ -64,6 +65,8 @@ def post_group_topics(
     except ValidationError:
         raise ValidationError({"tags": ["Invalid tags"]})
 
+    request.apply_rate_limit("topic_post")
+
     request.db_session.add(new_topic)
 
     request.db_session.add(LogTopic(LogEventType.TOPIC_POST, request, new_topic))
@@ -161,6 +164,63 @@ def get_group_topics(
     }
 
 
+@view_config(route_name="search", renderer="search.jinja2")
+@use_kwargs(TopicListingSchema(only=("after", "before", "order", "per_page", "period")))
+@use_kwargs({"search": String(load_from="q")})
+def get_search(
+    request: Request,
+    order: Any,
+    period: Any,
+    after: str,
+    before: str,
+    per_page: int,
+    search: str,
+) -> dict:
+    """Get a list of search results."""
+    # pylint: disable=too-many-arguments
+    if order is missing:
+        order = TopicSortOption.NEW
+
+    if period is missing:
+        period = None
+
+    query = (
+        request.query(Topic)
+        .join_all_relationships()
+        .search(search)
+        .apply_sort_option(order)
+    )
+
+    # restrict the time period, if not set to "all time"
+    if period:
+        query = query.inside_time_period(period)
+
+    # apply before/after pagination restrictions if relevant
+    if before:
+        query = query.before_id36(before)
+
+    if after:
+        query = query.after_id36(after)
+
+    topics = query.get_page(per_page)
+
+    period_options = [SimpleHoursPeriod(hours) for hours in (1, 12, 24, 72)]
+
+    # add the current period to the bottom of the dropdown if it's not one of the
+    # "standard" ones
+    if period and period not in period_options:
+        period_options.append(period)
+
+    return {
+        "search": search,
+        "topics": topics,
+        "order": order,
+        "order_options": TopicSortOption,
+        "period": period,
+        "period_options": period_options,
+    }
+
+
 @view_config(
     route_name="new_topic", renderer="new_topic.jinja2", permission="post_topic"
 )
@@ -225,6 +285,7 @@ def get_topic(request: Request, comment_order: CommentSortOption) -> dict:
 
 @view_config(route_name="topic", request_method="POST", permission="comment")
 @use_kwargs(CommentSchema(only=("markdown",)))
+@rate_limit_view("comment_post")
 def post_comment_on_topic(request: Request, markdown: str) -> HTTPFound:
     """Post a new top-level comment on a topic."""
     topic = request.context