Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

stream settings: Use the .show-sender version of email address. #13136

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 0 additions & 7 deletions frontend_tests/node_tests/templates.js
Original file line number Diff line number Diff line change
Expand Up @@ -641,13 +641,6 @@ run_test('draft_table_body', () => {
assert.equal(row_2.find(".message_content").text().trim(), "Private draft");
});


run_test('email_address_hint', () => {
var html = render('email_address_hint');
var li = $(html).find("li").first();
assert.equal(li.text(), 'translated: The email will be forwarded to this stream');
});

run_test('emoji_popover', () => {
var args = {
class: "emoji-info-popover",
Expand Down
23 changes: 0 additions & 23 deletions static/js/subs.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
var render_email_address_hint = require('../templates/email_address_hint.hbs');
var render_subscription = require('../templates/subscription.hbs');
var render_subscription_settings = require('../templates/subscription_settings.hbs');
var render_subscription_table_body = require('../templates/subscription_table_body.hbs');
Expand Down Expand Up @@ -211,26 +210,6 @@ exports.rerender_subscriptions_settings = function (sub) {
stream_ui_updates.update_subscribers_list(sub);
};

function add_email_hint_handler() {
// Add a popover explaining stream e-mail addresses on hover.

$("body").on("mouseover", '.stream-email-hint', function (e) {
var email_address_hint_content = render_email_address_hint({ page_params: page_params });
$(e.target).popover({
placement: "right",
title: "Email integration",
content: email_address_hint_content,
trigger: "manual",
animation: false});
$(e.target).popover('show');
e.stopPropagation();
});
$("body").on("mouseout", '.stream-email-hint', function (e) {
$(e.target).popover('hide');
e.stopPropagation();
});
}

exports.add_sub_to_table = function (sub) {
if (exports.is_sub_already_present(sub)) {
// If a stream is already listed/added in subscription modal,
Expand Down Expand Up @@ -805,8 +784,6 @@ exports.sub_or_unsub = function (sub) {


exports.initialize = function () {
add_email_hint_handler();

$("#subscriptions_table").on("click", ".create_stream_button", function (e) {
e.preventDefault();
exports.open_create_stream();
Expand Down
49 changes: 25 additions & 24 deletions static/styles/settings.scss
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,31 @@ label {
margin: 0;
}

label,
h3 {
a {
color: inherit;
}

.fa-question-circle-o {
top: 1px;
opacity: 0.4;
position: relative;

&:hover {
opacity: 1;
}
}
}

label .fa-question-circle-o {
left: 2px;
}

h3 .fa-question-circle-o {
left: 5px;
}

.new-style {
.block {
display: block;
Expand Down Expand Up @@ -213,30 +238,6 @@ td .button {
padding: 0;
}

label > a,
h3 > a {
color: inherit;
}

label .fa-question-circle-o {
top: 1px;
left: 2px;
}

h3 .fa-question-circle-o {
top: 1px;
left: 5px;
}

.fa-question-circle-o {
opacity: 0.4;
position: relative;

&:hover {
opacity: 1;
}
}

.settings-section-title {
font-size: 1.4em;
font-weight: 500;
Expand Down
6 changes: 0 additions & 6 deletions static/styles/subscriptions.scss
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,6 @@
margin: 0;
}

.email_address_hint {
max-width: 430px;
padding-left: 10px;
padding-right: 10px;
}

.subscription-email-hint-image {
float: right;
width: 80px;
Expand Down
10 changes: 0 additions & 10 deletions static/templates/email_address_hint.hbs

This file was deleted.

4 changes: 2 additions & 2 deletions static/templates/subscription_members.hbs
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
{{#render_subscribers}}
<div class="subscriber_list_settings_container" {{#unless can_access_subscribers}}style="display: none"{{/unless}}>
<div class="subscriber_list_settings">
<div class="sub_settings_title float-left">
<label class="sub_settings_title float-left">
{{t "Stream membership" }}
<div class="stream_subscription_info small"></div>
</div>
</label>
<div class="subscriber_list_add float-right">
<form class="form-inline">
<input type="text" class="search" placeholder="{{t 'Search subscribers' }}" />
Expand Down
7 changes: 6 additions & 1 deletion static/templates/subscription_settings.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,12 @@
</ul>
</div>
<div class="stream-email-box" {{#unless sub.email_address}}style="display: none;"{{/unless}}>
<div class="sub_settings_title">{{t "Email address" }} <i class="fa fa-question-circle stream-email-hint" aria-hidden="true"></i></div>
<label class="sub_settings_title">
{{t "Email address" }}
<a href="/help/message-a-stream-by-email" target="_blank">
<i class="fa fa-question-circle-o" aria-hidden="true"></i>
</a>
</label>
<div class="stream-email">
<span class="email-address">{{sub.email_address}}</span>
</div>
Expand Down
8 changes: 5 additions & 3 deletions zerver/lib/actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -2696,7 +2696,7 @@ def notify_subscriptions_added(user_profile: UserProfile,
is_web_public=stream.is_web_public,
is_announcement_only=stream.is_announcement_only,
color=subscription.color,
email_address=encode_email_address(stream),
email_address=encode_email_address(stream, show_sender=True),
desktop_notifications=subscription.desktop_notifications,
audible_notifications=subscription.audible_notifications,
push_notifications=subscription.push_notifications,
Expand Down Expand Up @@ -3557,7 +3557,7 @@ def do_rename_stream(stream: Stream,
# date field in all cases.
cache_delete_many(
to_dict_cache_key_id(message.id) for message in messages)
new_email = encode_email_address(stream)
new_email = encode_email_address(stream, show_sender=True)

# We will tell our users to essentially
# update stream.name = new_name where name = old_name
Expand Down Expand Up @@ -4679,6 +4679,8 @@ def gather_subscriptions_helper(user_profile: UserProfile,
if not sub["active"] and user_profile.is_guest:
subscribers = None

email_address = encode_email_address_helper(stream["name"], stream["email_token"],
show_sender=True)
stream_dict = {'name': stream["name"],
'in_home_view': not sub["is_muted"],
'is_muted': sub["is_muted"],
Expand All @@ -4699,7 +4701,7 @@ def gather_subscriptions_helper(user_profile: UserProfile,
'stream_weekly_traffic': get_average_weekly_stream_traffic(stream["id"],
stream["date_created"],
recent_traffic),
'email_address': encode_email_address_helper(stream["name"], stream["email_token"]),
'email_address': email_address,
'history_public_to_subscribers': stream['history_public_to_subscribers']}

if subscribers is not None:
Expand Down
9 changes: 6 additions & 3 deletions zerver/lib/email_mirror_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,10 @@ def get_email_gateway_message_string_from_address(address: str) -> str:

return msg_string

def encode_email_address(stream: Stream) -> str:
return encode_email_address_helper(stream.name, stream.email_token)
def encode_email_address(stream: Stream, show_sender: bool=False) -> str:
return encode_email_address_helper(stream.name, stream.email_token, show_sender)

def encode_email_address_helper(name: str, email_token: str) -> str:
def encode_email_address_helper(name: str, email_token: str, show_sender: bool=False) -> str:
# Some deployments may not use the email gateway
if settings.EMAIL_GATEWAY_PATTERN == '':
return ''
Expand All @@ -52,6 +52,9 @@ def encode_email_address_helper(name: str, email_token: str) -> str:
else:
encoded_token = email_token

if show_sender:
encoded_token += ".show-sender"

return settings.EMAIL_GATEWAY_PATTERN % (encoded_token,)

def decode_email_address(email: str) -> Tuple[str, Dict[str, bool]]:
Expand Down
34 changes: 28 additions & 6 deletions zerver/tests/test_email_mirror.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,16 +69,30 @@ def test_encode_decode(self) -> None:
stream_name = 'dev. help'
stream = ensure_stream(realm, stream_name)
email_address = encode_email_address(stream)
self.assertTrue(email_address.startswith('dev-help'))
self.assertTrue(email_address.endswith('@testserver'))
self.assertEqual(email_address, "dev-help.{}@testserver".format(stream.email_token))

# The default form of the email address (with an option - "include-footer"):
token, options = decode_email_address(
"dev-help.{}.include-footer@testserver".format(stream.email_token)
)
self._assert_options(options, include_footer=True)
self.assertEqual(token, stream.email_token)

# Using + instead of . as the separator is also supported for backwards compatibility,
# since that was the original form of addresses that we used:
token, options = decode_email_address(
"dev-help+{}+include-footer@testserver".format(stream.email_token)
)
self._assert_options(options, include_footer=True)
self.assertEqual(token, stream.email_token)

token, options = decode_email_address(email_address)
self._assert_options(options)
self.assertEqual(token, stream.email_token)

parts = email_address.split('@')
# Use a mix of + and . as separators, to test that it works:
parts[0] += "+include-footer.show-sender+include-quotes"
email_address_all_options = '@'.join(parts)
# We also handle mixing + and . but it shouldn't be recommended to users.
email_address_all_options = "dev-help.{}+include-footer.show-sender+include-quotes@testserver"
email_address_all_options = email_address_all_options.format(stream.email_token)
token, options = decode_email_address(email_address_all_options)
self._assert_options(options, show_sender=True, include_footer=True, include_quotes=True)
self.assertEqual(token, stream.email_token)
Expand Down Expand Up @@ -136,6 +150,14 @@ def test_decode_ignores_stream_name(self) -> None:
token = decode_email_address(stream_to_address)[0]
self.assertEqual(token, stream.email_token)

def test_encode_with_show_sender(self) -> None:
stream = get_stream("Denmark", get_realm("zulip"))
stream_to_address = encode_email_address(stream, show_sender=True)

token, options = decode_email_address(stream_to_address)
self._assert_options(options, show_sender=True)
self.assertEqual(token, stream.email_token)

class TestGetMissedMessageToken(ZulipTestCase):
def test_get_missed_message_token(self) -> None:
with self.settings(EMAIL_GATEWAY_PATTERN="%s@example.com"):
Expand Down