There is a bug when clicking on the stream row in stream settings page
subscribes/unsubscribes the user from stream. The ideal behaviour
should be that user should be subscribed/unsubscribed only
when user clicks on tick icon. The buggy behaviour is visible only after
clicking the tick icon once.
This is because clicking on tick is adding sub_unsub_button class to the
stream row, while it should be added only to the ".check" element to
re-enable the button to handle requests.
This commit fixes the bug by adding the "sub_unsub_button" class to
".check" element only and not to the stream row and same change
is done for removing the "sub_unsub_button" class also.
Subscribe/Unsubscribe button in the right section of stream settings
page is not working because the target element in click handler was
changed to 'check.sub_unsub_button' in c234b4f2 and the button in
the right section with class 'sub_unsub_button' also uses the same
click handler.
This commit reverts c234b4f2 and the bug of subscribing/unsubscribing
the user by clicking on stream row in the stream settings page, which
c234b4f2 intended to fix will be fixed in next commit.
As we add more features where rendered_markdown.update_elements does
something useful, it'll become important to run this code everywhere
we render markdown in the DOM.
One can see in this case that we had actually copied one hunk of
rendered_markdown.update_elements years ago, before we extracted it as
an independent function; we get to delete that copy.
Fixes#15500.
success-http-headers-bionic.txt and success-http-headers-focal.txt
differ only in the nginx version so this substitution will allow
us to have single file for both of them. Also this change helps
to avoid CI failure if Nginx version is updated in the OS.
We do not show the warning while sending messages to announce
stream if there is a wildcard mention in the message (i.e.
when wildcard_mention != null)
There are two cases where we should ideally show the warning
but we don't-
- When there is no wildcard mention in the message and
wildcard_mention is set to undefined (initial value of
wildcard_mention).
This is because "wildcard_mention != null" returns true for
this case and thus the warning is not shown, assuming the
message to have wildcard mention.
- When previous message had a wildcard mention and now a message
is being sent with no wildcard mention.
This is because the condition "wildcard_mention != null" is
checked with the previous value of wildcard_mention and not
with the value according to current message content, and thus
the warning is not shown, assuming the message to have wildcard
mention.
This commit changes the code to set wildcard_mention from the
latest message content before performing other validations and
thus solves the problems described above.
This particular commit has been a long time coming. For reference,
!avatar(email) was an undocumented syntax that simply rendered an
inline 50px avatar for a user in a message, essentially allowing
you to create a user pill like:
`!avatar(alice@example.com) Alice: hey!`
---
Reimplementation
If we decide to reimplement this or a similar feature in the future,
we could use something like `<avatar:userid>` syntax which is more
in line with creating links in markdown. Even then, it would not be
a good idea to add this instead of supporting inline images directly.
Since any usecases of such a syntax are in automation, we do not need
to make it userfriendly and something like the following is a better
implementation that doesn't need a custom syntax:
` Alice: hey!`
---
History
We initially added this syntax back in 2012 and it was 'deprecated'
from the get go. Here's what the original commit had to say about
the new syntax:
> We'll use this internally for the commit bot. We might eventually
> disable it for external users.
We eventually did start using this for our github integrations in 2013
but since then, those integrations have been neglected in favor of
our GitHub webhooks which do not use this syntax.
When we copied `!gravatar` to add the `!avatar` syntax, we also noted
that we want to deprecate the `!gravatar` syntax entirely - in 2013!
Since then, we haven't advertised either of these syntaxes anywhere
in our docs, and the only two places where this syntax remains is
our game bots that could easily do without these, and the git commit
integration that we have deprecated anyway.
We do not have any evidence of someone asking about this syntax on
chat.zulip.org when developing an integration and rightfully so- only
the people who work on Zulip (and specifically, markdown) are likely
to stumble upon it and try it out.
This is also the only peice of code due to which we had to look up
emails -> userid mapping in our backend markdown. By removing this,
we entirely remove the backend markdown's dependency on user emails
to render messages.
---
Relevant commits:
- Oct 2012, Initial commit c31462c278
- Nov 2013, Update commit bot 968c393826
- Nov 2013, Add avatar syntax 761c0a0266
- Sep 2017, Avoid email use c3032a7fe8
- Apr 2019, Remove from webhook 674fcfcce1
This commits adds the steps for configuring message retention policy
for an organization and for individual streams in message retention
policy docs.
Fixes#15495.
The installer does not adjust the node name if the rabbitmq already
exists, and the default node name bakes in the
`zulip-install-bionic-base` hostname. As such, the resulting LXC
image does not properly start rabbitmq.
Remove rabbitmq, allowing the installer to install and configure it
with a nodename of `zulip@localhost`. This also lets the installed
image be successfully copied and booted under a new hostname without
breaking rabbitmq.
Previously, the function update_table_stream_color did not update the
color of the date_row when it appeared within a topic. This was
incorrect because it meant that a small piece of the border to the
left of the message box would be left to the previous color when the
stream color was updated. This commit resolves the issue by adding an
update.
Fixes#15197.
Running `pg-upgradecluster` runs the `CREATE TEXT SEARCH DICTIONARY`
and `CREATE TEXT SEARCH CONFIGURATION` from
`zerver/migrations/0001_initial.py` on the new PostgreSQL cluster;
this requires that the stopwords file and dictionary exist _prior_
to `pg_upgradecluster` being run.
This causes a minor dependency conflict -- we do not wish to duplicate
the functionality from `zulip::postgres_appdb_base` which configures
those files, but installing all of `zulip::postgres_appdb_tuned` will
attempt to restart PostgreSQL -- which has not configured the cluster
for the new version yet.
In order to split out configuration of the prerequisites for the
application database, and the steps required to run it, we need to be
able to apply only part of the puppet configuration. Use the
newly-added `--config` argument to provide a more limited `zulip.conf`
which only applies `zulip::postgres_appdb_base` to the new version of
Postgres, creating the required tsearch data files.
This also preserves the property that a failure at any point prior to
the `pg_upgradecluster` is easily recoverable, by re-running
`zulip-puppet-apply`.
This is a fine solution short-term until github implements the
yaml anchors support. The limitation of this method is that we
cannot re-use most of the steps again for production install test
builds.
Thanks, Anders for this solution.
Verifying everything is migrated correctly is a pain. This script
ensures everything is done correctly (previous commit message
contains explainations for the steps being ignored if; in case
of github-actions steps they are ignored because they are actions
specific):
"""
This script prints out the ignore steps first. Then
prints out each step of both circle and actions side-by-side.
One step is out of order for bionic but verfying correction is
still easier. Format:
Actions: Install dependencies
Circle CI: install dependencies
....
"""
import yaml
with open('.circleci/config.yml') as f:
circleci_config = yaml.safe_load(f)
with open('.github/workflows/zulip-ci.yml') as f:
actions_config = yaml.safe_load(f)
circle_bionic_steps = []
circle_focal_steps = []
actions_bionic_steps = []
actions_focal_steps = []
"""
We ignore casper artifact upload, save_cache, and
store_tests_reports steps.
"""
def get_circleci_steps(job, arr):
for step in circleci_config['jobs'][job]['steps']:
if isinstance(step, str):
arr.append(step)
continue
step_name = step.get('run', {}).get('name', False)
if not step_name:
if step.get('restore_cache'):
key = step['restore_cache']['keys'][0].split('.')[0]
step_name = f'<restore-cache> {key}'
elif step.get('store_artifacts', False):
destination = step['store_artifacts']['destination']
step_name = f'<store-artificats> {destination}'
if destination == 'casper':
\# This is no longer needed
print('Ignoring step:')
print(step)
print()
continue
else:
"""
We don't care about save_cache; github-actions
does this automatically, and store_tests_reports
is circelci timing specific.
"""
print('Ignoring step:')
print(step)
print()
continue
if step_name != 'On fail':
arr.append(step_name)
get_circleci_steps('bionic-backend-frontend', circle_bionic_steps)
get_circleci_steps('focal-backend', circle_focal_steps)
""" We ignore there steps specific to github-actions"""
for step in actions_config['jobs']['focal_bionic']['steps']:
BOTH_OS = 'BOTH_OS'
if_check = step.get('if', BOTH_OS)
step_name = step.get('name')
if step_name is None:
step_name = step['uses']
if (
step_name == 'Upgrade git for bionic' or
step_name == 'Add required permissions' or
step_name == 'Move test reports to var'
):
print('Ignoring step:')
print(step)
print()
"""These are github-actions specific; see comments"""
continue
if if_check == BOTH_OS:
actions_bionic_steps.append(step_name)
actions_focal_steps.append(step_name)
elif 'is_bionic' in if_check:
actions_bionic_steps.append(step_name)
else:
actions_focal_steps.append(step_name)
bionic = zip(circle_bionic_steps, actions_bionic_steps)
focal = zip(circle_focal_steps, actions_focal_steps)
print('Bionic steps:')
for (circle_step, actions_step) in bionic:
print(f'CircleCI: {circle_step}')
print(f'Actions: {actions_step}')
print()
print('Focal steps:')
for (circle_step, actions_step) in focal:
print(f'CircleCI: {circle_step}')
print(f'Actions: {actions_step}')
print()
Doing service memcached start instead of restart fixed an issue on
focal build in GitHub actions, where it exits with code 1 when it
is done twice.It is done first in Install Dependencies step and then
again in last step where we call tools/ci/setup-backend again which
runs provision.
Furthermore, I don't belive there is a technical reason we use
restart over start; rather I think it was just a random choice with
the intend to just start the services in CI. I traced the code back
to commit 1f2f497cab if it helps.
Looking at the source code of memcached, the step that's failing is:
start-stop-daemon --stop --quiet --oknodo --pidfile $PIDFILE
which is equivilent to: service memcached stop, we can rule out the
service memcache start since it works. Ideally, we do figure out and
solve the issue of why memcached fails when executing service
memcached stop but I am not equipped with debugging it. And this
workaround seems reasonable rather than a "hacky" solution.
For the relevant code in memcached see:
https://github.com/memcached/memcached/blob/master/scripts/memcached-init.
Finally, the change to the rest of services is for consistency.
Some noteable diffrence from circleci:
- We upgrade git to newer version (reason explained in comments)
- We set HOME to /home/github (also explained in comments)
- Adjust permissions (... comments)
- Minor changes to step names and cache keys.
- We don't need to port the save_cache steps they are done
automatically in actions. And, we did not port the
store_test_results step which is circleci specific.
- We didn't port the notify_failure step yet (see the TODO).
Log RealmAuditLog in do_set_realm_property and do_remove_realm_domain.
Tests for the changes are written in test_events because it will save
duplicate code for test_change_realm_property.
Added new Event Type in AbstractRealmAuditLog STREAM_CREATED.
Since we finally create streams in create_stream_if_needed function
in zerver/lib/streams.py so logged realm_audit there.
Passed acting_user when create_stream_if_needed or ensure_stream
function is called.
Added tests in test_audit_log.
This commit moves out the SoftDeactivationMessageTest out of
test_messages.py (which at the moment have mixed category of tests) into
a more logical file, test_soft_deactivation.py.
This commit moves TestMessageForIdsDisplayRecipientFetching class which
have tests regarding display_recipient filled in by MessageDict to
test_message_dict.py.
This commit moves InternalPrepTest test class to test_message_send.py
because it tests internal_send_* and internal_prep_* functions which are
used for internal message sending in zulip.
This commit moves few tests related to testing proper sending of private
messages from PrivateMessagesTest class in test_messages.py to a new class
in test_message_send.py.
The commit moves, test_create_mirror_user_despite_race which is not related
to message sending from MessagePOSTTest class in test_message_send.py to
test_mirror_users.py.
This commit extracts out MessagePOSTTest class from test_messages.py
intially.
In future commits other related message sending tests will be moved from
test_messages.py to test_message_send.py.
Starting with extracting out MirroredMessageUsersTests as it is related to
mirror users than anything message-specific.
In a future commit, may extract out some tests from MessagePOSTTest as well
but still deciding on those.
Mohit Gupta
sahil839
Tim Abbott
arpit551
Rohitt Vashishtha
Aman Agrawal
Alex Vandiver
Anders Kaseorg
Sanskar Bajpai
Priyank Patel