Skip to content

Commit

Permalink
DONOTMERGE: Rename subject -> topic on Message.
Browse files Browse the repository at this point in the history
This is still just a proof of concept, but we try
to at least get all tests passing with fairly minimal
effort.

Not so sure about fix_unreads here.
  • Loading branch information
Steve Howell committed Nov 13, 2018
1 parent 87ddee4 commit 97f0945
Show file tree
Hide file tree
Showing 6 changed files with 73 additions and 48 deletions.
4 changes: 2 additions & 2 deletions puppet/zulip/files/postgresql/process_fts_updates
Expand Up @@ -37,11 +37,11 @@ def update_fts_columns(cursor):
if USING_PGROONGA:
cursor.execute("UPDATE zerver_message SET "
"search_pgroonga = "
"escape_html(subject) || ' ' || rendered_content "
"escape_html(topic) || ' ' || rendered_content "
"WHERE id = %s", (message_id,))
cursor.execute("UPDATE zerver_message SET "
"search_tsvector = to_tsvector('zulip.english_us_search', "
"subject || rendered_content) "
"topic || rendered_content) "
"WHERE id = %s", (message_id,))
ids.append(id)
cursor.execute("DELETE FROM fts_update_log WHERE id = ANY(%s)", (ids,))
Expand Down
2 changes: 1 addition & 1 deletion zerver/lib/fix_unreads.py
Expand Up @@ -195,7 +195,7 @@ def find_old_ids() -> None:
SELECT
zerver_usermessage.id,
zerver_message.recipient_id,
zerver_message.subject
zerver_message.topic
FROM
zerver_usermessage
INNER JOIN zerver_message ON (
Expand Down
40 changes: 20 additions & 20 deletions zerver/lib/topic.py
Expand Up @@ -34,7 +34,7 @@

# This constant is pretty closely coupled to the
# database, but it's the JSON field.
EXPORT_TOPIC_NAME = "subject"
EXPORT_TOPIC_NAME = "topic"

'''
The following functions are for user-facing APIs
Expand Down Expand Up @@ -75,35 +75,35 @@ def REQ_topic() -> Optional[str]:

# This is used in low-level message functions in
# zerver/lib/message.py, and it's not user facing.
DB_TOPIC_NAME = "subject"
MESSAGE__TOPIC = 'message__subject'
DB_TOPIC_NAME = "topic"
MESSAGE__TOPIC = 'message__topic'

def topic_match_sa(topic_name: str) -> Any:
# _sa is short for Sql Alchemy, which we use mostly for
# queries that search messages
topic_cond = func.upper(column("subject")) == func.upper(literal(topic_name))
topic_cond = func.upper(column("topic")) == func.upper(literal(topic_name))
return topic_cond

def topic_column_sa() -> Any:
return column("subject")
return column("topic")

def filter_by_exact_message_topic(query: QuerySet, message: Message) -> QuerySet:
topic_name = message.topic_name()
return query.filter(subject=topic_name)
return query.filter(topic=topic_name)

def filter_by_topic_name_via_message(query: QuerySet, topic_name: str) -> QuerySet:
return query.filter(message__subject__iexact=topic_name)
return query.filter(message__topic__iexact=topic_name)

def messages_for_topic(stream_id: int, topic_name: str) -> QuerySet:
# It might be the case that we really want subject__contains
# It might be the case that we really want topic__contains
# here. This code is used for the archive.
return Message.objects.filter(
recipient__type_id=stream_id,
subject=topic_name,
topic=topic_name,
)

def save_message_for_edit_use_case(message: Message) -> None:
message.save(update_fields=["subject", "content", "rendered_content",
message.save(update_fields=["topic", "content", "rendered_content",
"rendered_content_version", "last_edit_time",
"edit_history"])

Expand All @@ -113,14 +113,14 @@ def user_message_exists_for_topic(user_profile: UserProfile,
return UserMessage.objects.filter(
user_profile=user_profile,
message__recipient=recipient,
message__subject__iexact=topic_name,
message__topic__iexact=topic_name,
).exists()

def update_messages_for_topic_edit(message: Message,
propagate_mode: str,
orig_topic_name: str,
topic_name: str) -> List[Message]:
propagate_query = Q(recipient = message.recipient, subject = orig_topic_name)
propagate_query = Q(recipient = message.recipient, topic = orig_topic_name)
# We only change messages up to 2 days in the past, to avoid hammering our
# DB by changing an unbounded amount of messages
if propagate_mode == 'change_all':
Expand All @@ -135,7 +135,7 @@ def update_messages_for_topic_edit(message: Message,

# Evaluate the query before running the update
messages_list = list(messages)
messages.update(subject=topic_name)
messages.update(topic=topic_name)

for m in messages_list:
# The cached ORM object is not changed by messages.update()
Expand Down Expand Up @@ -171,22 +171,22 @@ def get_topic_history_for_stream(user_profile: UserProfile,
if public_history:
query = '''
SELECT
"zerver_message"."subject" as topic,
"zerver_message"."topic" as topic,
max("zerver_message".id) as max_message_id
FROM "zerver_message"
WHERE (
"zerver_message"."recipient_id" = %s
)
GROUP BY (
"zerver_message"."subject"
"zerver_message"."topic"
)
ORDER BY max("zerver_message".id) DESC
'''
cursor.execute(query, [recipient.id])
else:
query = '''
SELECT
"zerver_message"."subject" as topic,
"zerver_message"."topic" as topic,
max("zerver_message".id) as max_message_id
FROM "zerver_message"
INNER JOIN "zerver_usermessage" ON (
Expand All @@ -197,7 +197,7 @@ def get_topic_history_for_stream(user_profile: UserProfile,
"zerver_message"."recipient_id" = %s
)
GROUP BY (
"zerver_message"."subject"
"zerver_message"."topic"
)
ORDER BY max("zerver_message".id) DESC
'''
Expand All @@ -211,14 +211,14 @@ def get_topic_history_for_web_public_stream(recipient: Recipient) -> List[Dict[s
cursor = connection.cursor()
query = '''
SELECT
"zerver_message"."subject" as topic,
"zerver_message"."topic" as topic,
max("zerver_message".id) as max_message_id
FROM "zerver_message"
WHERE (
"zerver_message"."recipient_id" = %s
)
GROUP BY (
"zerver_message"."subject"
"zerver_message"."topic"
)
ORDER BY max("zerver_message".id) DESC
'''
Expand All @@ -233,6 +233,6 @@ def get_turtle_message(message_ids: List[int]) -> Message:
# here to make subject -> topic sweeping easier.
turtle_message = Message.objects.get( # nolint
id__in=message_ids,
subject='topic demonstration',
topic='topic demonstration',
content__icontains='cute/turtle.png')
return turtle_message
25 changes: 25 additions & 0 deletions zerver/migrations/0191_rename_message_subject.py
@@ -0,0 +1,25 @@
# -*- coding: utf-8 -*-
# Generated by Django 1.11.14 on 2018-11-11 20:27
from __future__ import unicode_literals

from django.db import migrations


class Migration(migrations.Migration):

dependencies = [
('zerver', '0190_cleanup_pushdevicetoken'),
]

operations = [
migrations.RenameField(
model_name='archivedmessage',
old_name='subject',
new_name='topic',
),
migrations.RenameField(
model_name='message',
old_name='subject',
new_name='topic',
),
]
16 changes: 8 additions & 8 deletions zerver/models.py
Expand Up @@ -1304,13 +1304,13 @@ class AbstractMessage(models.Model):

# The message's topic.
#
# Early versions of Zulip called this concept a "subject", as in an email
# "subject line", before changing to "topic" in 2013 (commit dac5a46fa).
# Early versions of Zulip called this concept a "subject", as in an email,
# before changing to "topic" in 2013 (commit dac5a46fa).
# UI and user documentation now consistently say "topic". New APIs and
# new code should generally also say "topic".
#
# See also the `topic_name` method on `Message`.
subject = models.CharField(max_length=MAX_TOPIC_NAME_LENGTH, db_index=True) # type: str
topic = models.CharField(max_length=MAX_TOPIC_NAME_LENGTH, db_index=True) # type: str

content = models.TextField() # type: str
rendered_content = models.TextField(null=True) # type: Optional[str]
Expand All @@ -1335,7 +1335,7 @@ class Meta:
def __str__(self) -> str:
display_recipient = get_display_recipient(self.recipient)
return "<%s: %s / %s / %s>" % (self.__class__.__name__, display_recipient,
self.subject, self.sender)
self.topic, self.sender)


class ArchivedMessage(AbstractMessage):
Expand All @@ -1353,10 +1353,10 @@ def topic_name(self) -> str:
Please start using this helper to facilitate an
eventual switch over to a separate topic table.
"""
return self.subject
return self.topic

def set_topic_name(self, topic_name: str) -> None:
self.subject = topic_name
self.topic = topic_name

def is_stream_message(self) -> bool:
'''
Expand Down Expand Up @@ -1393,7 +1393,7 @@ def to_log_dict(self) -> Dict[str, Any]:
sending_client = self.sending_client.name,
type = self.recipient.type_name(),
recipient = get_display_recipient(self.recipient),
subject = self.topic_name(),
topic = self.topic_name(),
content = self.content,
timestamp = datetime_to_timestamp(self.pub_date))

Expand Down Expand Up @@ -1457,7 +1457,7 @@ def get_context_for_message(message: Message) -> Sequence[Message]:
# TODO: Change return type to QuerySet[Message]
return Message.objects.filter(
recipient_id=message.recipient_id,
subject=message.subject,
topic=message.topic,
id__lt=message.id,
pub_date__gt=message.pub_date - timedelta(minutes=15),
).order_by('-id')[:10]
Expand Down

0 comments on commit 97f0945

Please sign in to comment.