diff --git a/tildes/alembic/versions/50c251c4a19c_add_search_column_index_for_topics.py b/tildes/alembic/versions/50c251c4a19c_add_search_column_index_for_topics.py new file mode 100644 index 0000000..8eb4c41 --- /dev/null +++ b/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") diff --git a/tildes/requirements-to-freeze.txt b/tildes/requirements-to-freeze.txt index 39cc8a1..5e15100 100644 --- a/tildes/requirements-to-freeze.txt +++ b/tildes/requirements-to-freeze.txt @@ -37,6 +37,7 @@ SQLAlchemy SQLAlchemy-Utils stripe testing.redis +titlecase webargs webtest zope.sqlalchemy diff --git a/tildes/requirements.txt b/tildes/requirements.txt index aacc564..74dc993 100644 --- a/tildes/requirements.txt +++ b/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 diff --git a/tildes/scss/modules/_btn.scss b/tildes/scss/modules/_btn.scss index 0dc0641..7acc525 100644 --- a/tildes/scss/modules/_btn.scss +++ b/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; } diff --git a/tildes/scss/modules/_sidebar.scss b/tildes/scss/modules/_sidebar.scss index d7a56f9..7106908 100644 --- a/tildes/scss/modules/_sidebar.scss +++ b/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 { diff --git a/tildes/scss/modules/_site-header.scss b/tildes/scss/modules/_site-header.scss index eff2914..708d5a9 100644 --- a/tildes/scss/modules/_site-header.scss +++ b/tildes/scss/modules/_site-header.scss @@ -12,6 +12,7 @@ } .site-header-context { + white-space: nowrap; overflow: hidden; text-overflow: ellipsis; } diff --git a/tildes/scss/modules/_topic.scss b/tildes/scss/modules/_topic.scss index 82827aa..2307fcc 100644 --- a/tildes/scss/modules/_topic.scss +++ b/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; } } diff --git a/tildes/sql/init/triggers/topics/topics.sql b/tildes/sql/init/triggers/topics/topics.sql index c62891e..6825a85 100644 --- a/tildes/sql/init/triggers/topics/topics.sql +++ b/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); diff --git a/tildes/static/js/behaviors/comment-collapse-all-button.js b/tildes/static/js/behaviors/comment-collapse-all-button.js new file mode 100644 index 0000000..64827a5 --- /dev/null +++ b/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(); + }); +}); diff --git a/tildes/static/js/behaviors/comment-expand-all-button.js b/tildes/static/js/behaviors/comment-expand-all-button.js new file mode 100644 index 0000000..f870139 --- /dev/null +++ b/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(); + }); +}); diff --git a/tildes/tests/test_topic.py b/tildes/tests/test_topic.py index 723d91e..e9c21fd 100644 --- a/tildes/tests/test_topic.py +++ b/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" diff --git a/tildes/tildes/__init__.py b/tildes/tildes/__init__.py index 7be2000..6ec4b2f 100644 --- a/tildes/tildes/__init__.py +++ b/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 } diff --git a/tildes/tildes/lib/ratelimit.py b/tildes/tildes/lib/ratelimit.py index 96661b7..7046736 100644 --- a/tildes/tildes/lib/ratelimit.py +++ b/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 diff --git a/tildes/tildes/models/topic/topic.py b/tildes/tildes/models/topic/topic.py index 932f840..137054f 100644 --- a/tildes/tildes/models/topic/topic.py +++ b/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 diff --git a/tildes/tildes/models/topic/topic_query.py b/tildes/tildes/models/topic/topic_query.py index 4d3ad59..af7edf7 100644 --- a/tildes/tildes/models/topic/topic_query.py +++ b/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))) diff --git a/tildes/tildes/routes.py b/tildes/tildes/routes.py index 82c1a47..f39b70f 100644 --- a/tildes/tildes/routes.py +++ b/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") diff --git a/tildes/tildes/templates/home.jinja2 b/tildes/tildes/templates/home.jinja2 index 4e7aca8..e8c9fd4 100644 --- a/tildes/tildes/templates/home.jinja2 +++ b/tildes/tildes/templates/home.jinja2 @@ -17,6 +17,13 @@ {% endblock %} {% block sidebar %} +
+The home page shows topics from groups that you're subscribed to.
{% if request.user %} diff --git a/tildes/tildes/templates/search.jinja2 b/tildes/tildes/templates/search.jinja2 new file mode 100644 index 0000000..90e41f0 --- /dev/null +++ b/tildes/tildes/templates/search.jinja2 @@ -0,0 +1,31 @@ +{% extends 'topic_listing.jinja2' %} + +{% block title %}Search results for "{{ search }}"{% endblock %} + +{% block header_context_link %}Search: "{{ search }}"{% endblock %} + +{% block sidebar %} + + +
{% trans num_comments=topic.num_comments %} {{ num_comments }} comment @@ -153,6 +153,9 @@ {% endtrans %}
+ + +