From 98cfa08a608da3e548183f796b00f6fd9479ef39 Mon Sep 17 00:00:00 2001 From: Jedi Burrell Date: Thu, 9 Aug 2018 06:40:35 -0400 Subject: [PATCH 1/8] Fix OK button location for sorting with JS off --- tildes/tildes/templates/topic.jinja2 | 9 ++++----- tildes/tildes/templates/topic_listing.jinja2 | 9 ++++----- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/tildes/tildes/templates/topic.jinja2 b/tildes/tildes/templates/topic.jinja2 index 78e60a8..a3512cb 100644 --- a/tildes/tildes/templates/topic.jinja2 +++ b/tildes/tildes/templates/topic.jinja2 @@ -166,12 +166,11 @@ >{{ option.description }} {% endfor %} + {# add a submit button for people with js disabled so this is still usable #} + - - {# add a submit button for people with js disabled so this is still usable #} - diff --git a/tildes/tildes/templates/topic_listing.jinja2 b/tildes/tildes/templates/topic_listing.jinja2 index ab4bf51..d8f9f56 100644 --- a/tildes/tildes/templates/topic_listing.jinja2 +++ b/tildes/tildes/templates/topic_listing.jinja2 @@ -64,12 +64,11 @@ + {# add a submit button for people with js disabled so this is still usable #} + - - {# add a submit button for people with js disabled so this is still usable #} - {% if not is_default_view %} From d5c2d18ae7eb53a450b4ca99d9ee1f9c3ed41ece Mon Sep 17 00:00:00 2001 From: Deimos Date: Sat, 18 Aug 2018 15:10:33 -0600 Subject: [PATCH 2/8] Add rate limits for posting topics and comments These limits were determined by looking at site activity so far, and generally shouldn't have any impact on normal site usage. This also adds a new request method - apply_rate_limit, which can be used to check the rate limit and immediately raise an error if it's exceeded, instead of needing to check and handle the result separately. --- tildes/tildes/__init__.py | 9 +++++++++ tildes/tildes/lib/ratelimit.py | 2 ++ tildes/tildes/views/api/web/comment.py | 4 +++- tildes/tildes/views/decorators.py | 6 ++---- tildes/tildes/views/topic.py | 4 ++++ 5 files changed, 20 insertions(+), 5 deletions(-) diff --git a/tildes/tildes/__init__.py b/tildes/tildes/__init__.py index 7be2000..aaa7300 100644 --- a/tildes/tildes/__init__.py +++ b/tildes/tildes/__init__.py @@ -4,6 +4,7 @@ from typing import Any, Callable, Dict, Optional 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: 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/views/api/web/comment.py b/tildes/tildes/views/api/web/comment.py index 84ee5f5..e17fc41 100644 --- a/tildes/tildes/views/api/web/comment.py +++ b/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 diff --git a/tildes/tildes/views/decorators.py b/tildes/tildes/views/decorators.py index e8b3042..c0b0a6f 100644 --- a/tildes/tildes/views/decorators.py +++ b/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) diff --git a/tildes/tildes/views/topic.py b/tildes/tildes/views/topic.py index 46e4539..5156272 100644 --- a/tildes/tildes/views/topic.py +++ b/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)) @@ -225,6 +228,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 From 54476a447d79644103bd69275b2ff0ff84ed2766 Mon Sep 17 00:00:00 2001 From: Deimos Date: Mon, 20 Aug 2018 16:54:47 -0600 Subject: [PATCH 3/8] Change url methods to treat routes individually Previously these methods for generating "base" and "normal" urls weren't treating each route individually and just had a single list of query vars that would be kept for all routes. This approach is a lot more flexible and allows separating out only the variables relevant for a particular route. --- tildes/tildes/__init__.py | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/tildes/tildes/__init__.py b/tildes/tildes/__init__.py index aaa7300..473731c 100644 --- a/tildes/tildes/__init__.py +++ b/tildes/tildes/__init__.py @@ -1,6 +1,6 @@ """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 @@ -139,10 +139,17 @@ 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"), + "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 } @@ -165,10 +172,17 @@ 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"), + "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 } From 59799c95db76980a323baa1ad4ae588a538638aa Mon Sep 17 00:00:00 2001 From: Deimos Date: Mon, 20 Aug 2018 18:59:06 -0600 Subject: [PATCH 4/8] Add extremely basic search Quite a few aspects of this are very hackish (especially as related to the templates and things that needed to be done to allow topic_listing.jinja2 to be inherited from for this new one), but it's a lot better than nothing. --- ...a19c_add_search_column_index_for_topics.py | 63 +++++++++++++++++++ tildes/scss/modules/_sidebar.scss | 8 +++ tildes/scss/modules/_site-header.scss | 1 + tildes/sql/init/triggers/topics/topics.sql | 15 +++++ tildes/tildes/__init__.py | 2 + tildes/tildes/models/topic/topic.py | 10 ++- tildes/tildes/models/topic/topic_query.py | 5 ++ tildes/tildes/routes.py | 2 + tildes/tildes/templates/home.jinja2 | 7 +++ tildes/tildes/templates/search.jinja2 | 31 +++++++++ tildes/tildes/templates/topic_listing.jinja2 | 22 +++++-- tildes/tildes/views/topic.py | 57 +++++++++++++++++ 12 files changed, 216 insertions(+), 7 deletions(-) create mode 100644 tildes/alembic/versions/50c251c4a19c_add_search_column_index_for_topics.py create mode 100644 tildes/tildes/templates/search.jinja2 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/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/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/tildes/__init__.py b/tildes/tildes/__init__.py index 473731c..6ec4b2f 100644 --- a/tildes/tildes/__init__.py +++ b/tildes/tildes/__init__.py @@ -142,6 +142,7 @@ def current_listing_base_url( 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"), } @@ -175,6 +176,7 @@ def current_listing_normal_url( 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",), } diff --git a/tildes/tildes/models/topic/topic.py b/tildes/tildes/models/topic/topic.py index 932f840..b822862 100644 --- a/tildes/tildes/models/topic/topic.py +++ b/tildes/tildes/models/topic/topic.py @@ -14,7 +14,7 @@ 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 @@ -111,12 +111,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]: 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 %} +
+
+ + +
+
+

Home

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 %} +
+
+ + +
+
+ +

Search results

+ +

Back to home page

+ + {% if request.user and request.user.subscriptions %} +
+
    +
  • Subscriptions
    • + +
+ {% endif %} + +{% endblock %} diff --git a/tildes/tildes/templates/topic_listing.jinja2 b/tildes/tildes/templates/topic_listing.jinja2 index d8f9f56..ec91068 100644 --- a/tildes/tildes/templates/topic_listing.jinja2 +++ b/tildes/tildes/templates/topic_listing.jinja2 @@ -46,9 +46,14 @@
- {% if tag %} + {% if tag is defined and tag %} {% endif %} + + {% if search is defined %} + + {% endif %} +
+ +
+
+

~{{ group.path }}

{% if group.short_description %} diff --git a/tildes/tildes/views/topic.py b/tildes/tildes/views/topic.py index 5156272..3f22eba 100644 --- a/tildes/tildes/views/topic.py +++ b/tildes/tildes/views/topic.py @@ -164,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" ) From 595af8f9ae2e74dfa78043e9866d09e7e5c44e26 Mon Sep 17 00:00:00 2001 From: Jeff Kayser Date: Tue, 14 Aug 2018 15:40:05 -0400 Subject: [PATCH 5/8] Add buttons for bulk collapse/expand of comments Adds two buttons: one for collapsing all child comments (non-top-level ones) and one for uncollapsing all comments. --- tildes/scss/modules/_btn.scss | 16 ++++++++++++++++ .../js/behaviors/comment-collapse-all-button.js | 11 +++++++++++ .../js/behaviors/comment-expand-all-button.js | 11 +++++++++++ tildes/tildes/templates/topic.jinja2 | 3 +++ 4 files changed, 41 insertions(+) create mode 100644 tildes/static/js/behaviors/comment-collapse-all-button.js create mode 100644 tildes/static/js/behaviors/comment-expand-all-button.js diff --git a/tildes/scss/modules/_btn.scss b/tildes/scss/modules/_btn.scss index 0dc0641..90d901d 100644 --- a/tildes/scss/modules/_btn.scss +++ b/tildes/scss/modules/_btn.scss @@ -65,6 +65,22 @@ } } +.btn-comment-collapse-all { + margin-left: 0.4rem; + border-left-width: 1px; + @media (min-width: $size-md) { + margin-top: 0.35rem; + } +} + +.btn-comment-expand-all { + margin-left: 0.2rem; + border-left-width: 1px; + @media (min-width: $size-md) { + margin-top: 0.35rem; + } +} + .btn-comment-tag { display: inline-flex; align-items: center; 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/tildes/templates/topic.jinja2 b/tildes/tildes/templates/topic.jinja2 index a3512cb..cdd913f 100644 --- a/tildes/tildes/templates/topic.jinja2 +++ b/tildes/tildes/templates/topic.jinja2 @@ -153,6 +153,9 @@ {% endtrans %} + + +
From 5170d283f7db4dac387932e3c632cedbc05801c4 Mon Sep 17 00:00:00 2001 From: Deimos Date: Tue, 21 Aug 2018 12:48:40 -0600 Subject: [PATCH 6/8] Adjust styles for comment collapse buttons Just a few minor style adjustments to simplify the styles and fix some bad wrapping behavior with the new bulk-collapse/expand buttons. --- tildes/scss/modules/_btn.scss | 18 ------------------ tildes/scss/modules/_topic.scss | 6 ++++++ tildes/tildes/templates/topic.jinja2 | 4 ++-- 3 files changed, 8 insertions(+), 20 deletions(-) diff --git a/tildes/scss/modules/_btn.scss b/tildes/scss/modules/_btn.scss index 90d901d..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; } @@ -65,22 +63,6 @@ } } -.btn-comment-collapse-all { - margin-left: 0.4rem; - border-left-width: 1px; - @media (min-width: $size-md) { - margin-top: 0.35rem; - } -} - -.btn-comment-expand-all { - margin-left: 0.2rem; - border-left-width: 1px; - @media (min-width: $size-md) { - margin-top: 0.35rem; - } -} - .btn-comment-tag { display: inline-flex; align-items: center; diff --git a/tildes/scss/modules/_topic.scss b/tildes/scss/modules/_topic.scss index 82827aa..4e6ac05 100644 --- a/tildes/scss/modules/_topic.scss +++ b/tildes/scss/modules/_topic.scss @@ -248,8 +248,14 @@ .topic-comments { header { display: flex; + flex-wrap: wrap; + align-items: center; + + margin-bottom: 0.4rem; h2 { + margin-bottom: 0; + margin-right: 0.4rem; white-space: nowrap; } diff --git a/tildes/tildes/templates/topic.jinja2 b/tildes/tildes/templates/topic.jinja2 index cdd913f..b737087 100644 --- a/tildes/tildes/templates/topic.jinja2 +++ b/tildes/tildes/templates/topic.jinja2 @@ -153,8 +153,8 @@ {% endtrans %} - - + +
From 0760f6441de661171c506c273e07abef143214c7 Mon Sep 17 00:00:00 2001 From: Deimos Date: Tue, 21 Aug 2018 13:42:31 -0600 Subject: [PATCH 7/8] Fix comments header styles applying to comments Previous way was a bad way of doing the styles and some of the rules were leaking through into the comments themselves. This should be more specific. --- tildes/scss/modules/_topic.scss | 26 ++++++++++++-------------- tildes/tildes/templates/topic.jinja2 | 2 +- 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/tildes/scss/modules/_topic.scss b/tildes/scss/modules/_topic.scss index 4e6ac05..2307fcc 100644 --- a/tildes/scss/modules/_topic.scss +++ b/tildes/scss/modules/_topic.scss @@ -245,23 +245,21 @@ overflow: auto; } -.topic-comments { - header { - display: flex; - flex-wrap: wrap; - align-items: center; +.topic-comments-header { + display: flex; + flex-wrap: wrap; + align-items: center; - margin-bottom: 0.4rem; + margin-bottom: 0.4rem; - h2 { - margin-bottom: 0; - margin-right: 0.4rem; - white-space: nowrap; - } + h2 { + margin-bottom: 0; + margin-right: 0.4rem; + white-space: nowrap; + } - .form-listing-options { - margin-left: auto; - } + .form-listing-options { + margin-left: auto; } } diff --git a/tildes/tildes/templates/topic.jinja2 b/tildes/tildes/templates/topic.jinja2 index b737087..fa08a6f 100644 --- a/tildes/tildes/templates/topic.jinja2 +++ b/tildes/tildes/templates/topic.jinja2 @@ -144,7 +144,7 @@ {% if topic.num_comments > 0 %}
-
+

{% trans num_comments=topic.num_comments %} {{ num_comments }} comment From be6fc19c16896c34656a1efe5e8c29be7c32a60e Mon Sep 17 00:00:00 2001 From: Celeo Date: Sun, 19 Aug 2018 19:05:49 -0700 Subject: [PATCH 8/8] Convert all-caps titles to title case --- tildes/requirements-to-freeze.txt | 1 + tildes/requirements.txt | 1 + tildes/tests/test_topic.py | 16 ++++++++++++++++ tildes/tildes/models/topic/topic.py | 6 ++++++ 4 files changed, 24 insertions(+) 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/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/models/topic/topic.py b/tildes/tildes/models/topic/topic.py index b822862..137054f 100644 --- a/tildes/tildes/models/topic/topic.py +++ b/tildes/tildes/models/topic/topic.py @@ -20,6 +20,7 @@ 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 @@ -172,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