The moderators-only option was actually added in the previous
commit for create_stream_policy as we use the same function
'has_permission' for both the policies. But we add the error
handling code and tests for moderators-only option in this
commit.
This commit modifies the has_permission function to include
realm moderator role. Thus this adds a new option of moderators
only for create_stream_policy.
Though this automatically adds this option for invite_to_stream_policy
also, but we will keep other code for showing error and for tests
in a separate commit.
This commit adds an assert statement in the last block of
has_permission which checks whether the policy_value is
POLICY_FULL_MEMBERS_ONLY. This assert statement is added
for readability.
The current API documentation uses call_endpoint to upload a file;
since we've added a custom helper in python-zulip-api, we should
document that cleaner approach.
The session object provides a common place to set headers on all
requests, no matter which implementation.
Because the `headers` attribute of Session is not a true static
attribute, but rather exposed via overriding `__getstate__`, `mock`'s
autospec cannot know about it, and thus throws an error; in tests that
mock the Session, we thus must explicitly set the `session.headers`.
The existing organization, of returning an opaque blob from
`build_bot_request`, which was later consumed by
`send_data_to_server`, is not particularly sensible; the steps become
oddly split between the OutgoingWebhookWorker, `do_rest_call`, and the
`OutgoingWebhookServiceInterface`.
Make the `OutgoingWebhookServiceInterface` in charge of building,
making, and returning the request in one method; another method
handles extracting content from a successful response. `do_rest_call`
is responsible for calling both halves of this, and doing common error
handling.
The comments in stream-policy tests in test_message_send.py specifies
the restriction of creating streams based on stream_post_policy. But
this restriction was removed in 9aaa61963 and we now allow everyone to
create all type of streams. So this commit fixes the stream creation
parts in comments.
The STREAM_POST_POLICY_RESTRICT_NEW_MEMBERS option of stream_post_policy
was explained as "Only new members can post" in the api docs. It should
instead be "Only full members can post" and this commit fixes it.
The /events and /register endpoints were excluded from schema validations,
because they were earlier not completely documented. However, they can
now be added for proper checking. Removed them from excluded endpoints list
and fixed the documentation for /register and /events after the checking.
Fixes#17796.
Backend logic for handling user mention was cluttered
because it was handled at two stages first in
get_possible_mentions_info while fetching mention data
based on the messsage and then later in UserMentionPattern
which handles processing of text for mention.
Ideally UserMentionPattern should depend on
get_possible_mentions_info only for data but there was a
shared logic between these two that made it hard to debug
any possible bugs.
Updates in this commit make both of these functions
coherent in terms of logic and also add appropiate
comments to improve readability of these functions.
There was also a hidden bug that if a user A is
mentioned in with @**name|id** then @**invalid|id**
again mentioned A because of the way we handled mentions
earlier. It is solved as a result of this refactor and
appropiate test has been added for this.
This has been tested manually as well as by adding new
test to address missing case.
I noticed this because the test_events.py tests had the extremely
weird pattern of calling the actual change function, and then testing
the `notify` function's state changes (which should always be noops),
rather than actually testing the state change function.
Fixing the test made it clear that the actual logic in events.py
simply did not handle deleting custom_profile_field_value elements
from user objects when a custom_profile_field object was deleted.
So we fix that bit of logic as well.
It appears this bug was unique -- at least we don't have any other
notify_* functions being used directly in test_events.py, and the
handful of state_change_expected=False entries are all events for data
not present in page_params.
* `op` (operation) field, added in f6fb88549f, was never intended for
`custom_profile_fields` event. This commit removes the `op` as it doesn't
have any use in the code.
* As a part of cleanup, this also eliminates the schema check warnings
for `custom_profile_fields` event, mentioned in #17568.
In 1a12e112d9, this page was converted
to use portico styling, but we intentionally left this page not using
the portico_content class since we didn't want the header/footer.
We still don't want the header/footer clutter, so instead, we achieve
that same goal using the isolated_page flag.
When running some tests multiple times in the same call,
were failing because of the data duplication.
This commit resolves that issue by resetting the test
environment (i.e: Re-cloning test database and clearing
cache) after each run.
Fixes#17607.
Since the list of streams returned by a query which is not sorted
can vary, the tests which use it become flaky.
NormalActionsTest.test_default_stream_groups_events became
flaky due to this and hopefully sorting the streams should
fix it.
I have the updated the documentation page for the hello world
integration to include numbers to bring it up to standard and make it
more readable.
Fixes part of #17633.
This commit migrates some of the backend tests in test_import_export
to use assertLogs(), instead of mock.patch() as planned in #15331.
Logs for tests in this file are suppressed and are not asserted as
that made changes to import/export codebase more fragile. As we
already have checks for the actual functionalities, it made less
sense to assert those logs.
Currently, there was no markdown page for deactivate-own-user API
endpoint. Created deactivate-own-user.md for the API page and
created a new owner client in test-api to reactivate the client
deactivated during testing.
Also changed endpoint name from deactivate-my-account to
deactivate-own-user, for better consistency with other endpoints.
Fixes#16163.
This is no longer used in any important place,
get_user_profile_by_email is meant to be used only in manage.py shell
now and thus there's no point in this function being cached.
Emails are not unique, so we can only sensibly cache using keys formed
with both email and realm.
This requires adding a new cache key function for caching by delivery
email - user_profile_delivery_email_cache_key.
Extend our markdown system to support mentioning of users
by id also. Following these changes, it would be possible
to mention users with @**|user_id** and silently mention
using @_**|user_id**.
Main intention for extending the mention syntax is to make
it convenient for bots to mention a users using their ids. It
is to be noted that previous syntax are also supported.
Documentation tweaked by tabbott for better readability.
The changes were tested manually in development server, and also
by adding some new backend and frontend tests.
Fixes: #17487.
We add support to shorten links and test their shortening in
well-organized, clean manner that makes it trivial to extend the
GitHub approach for GitLab and perhaps other services.
We only shorten basic types of GitHub links (issue, PR, commit) that
fit a set of simple common patterns; the default behaviour of Autolink
is kept for everything else.
Logic added in frontend and backend Markdown Processor is identical.
This makes easy to extend the logic for other services like GitLab.
Fixes#11895.
The schema for unread_msgs was missing additionalProperties: false
that was causing tests to pass even with undocumented parameters
which were validated as an additional property. Set
additionalProperties to false and added documentation for missing
count variable.
Fixes#17728.
This commit changes the list_to_streams function to raise error
according to create_stream_policy value when a user cannot create
streams instead of same error for all cases.
This commit modifies test_user_settings_for_subscribing_other_users
to check all the possible cases including the cases when a user
can successfully subscribe other users along with the already
tested failure cases. This commit also adds checks for guest users
which was not present before.
This commit replaces the code which directly changes user.role,
realm.create_stream_policy and realm.waiting_period_threshold
with do_change_user_role and do_set_realm_property functions
in test_can_create_streams. This makes the code similar to the
other tests.
We refactor test_can_create_streams and test_can_subscribe_other_users
in test_subs.py. We want to follow a specific order in such tests
which is just set the policy value one by one and then checking
that the role in policy returns true and role just below that returns
false. This approach is explained in detail below.
Following hierarchy of roles is considered for these tests -
1. Realm admin
2. Full members
3. Members
4. Guests.
Then if the policy is set to admins only, we check that the having
role as admin returns true and the role just below that, i.e. full
member returns false. Similarly, if the policy is set to members
only, we check that a member should return true and role below it
which is guest should return false. We basically follow these as
we can assume that if a user with particular role cannot do the
required task, then user with role below in the hierarchy would
be not allowed to do the task too.
This commit refactors the above mentioned two tests to have above
explained workflow.
This commit removes the unnecessary do_change_user_role function
in test_can_subcribe_other_users. This was added in 1aebf3cab
which replaced the multiple functions like do_change_is_admin
and do_change_is_guest with do_change_user_role.
Previously two functions do_change_is_admin and do_change_is_guest
were used because there were two flags is_realm_admin and is_guest
which were used to determine the role of a user. But then we added
a single field role to UserProfile and removed the multiple flags
and thus also replaced the different functions with a single
do_change_user_role. With addition of a new field role, two
different do_change_* functions were not needed as we only have
a role field instead of different flags, but this was missed in
1aebf3cab and this commit fixes it.
This add the schema checker, openapi schema, and also a test for
realm/deactivated event.
With several block comments by tabbott explaining the logic behind our
behavior here.
Part of #17568.
We discovered recently that some ops for events were just not
implemented in events.py (specifically, realm/deactivated).
Since our goal is for events.py to be complete, we add this bit of
hardening to ensure that it stays that way.
Modifies `StreamPattern` and `StreamTopicPattern` to inherit
from InlineProcessor instead of Pattern. This change is done
because Pattern stopped checking for matching patterns as soon
as it found a match which was not a valid stream. Due to this
all the subsequent mention failed, even if they were valid.
This bug was only present in backend renderring due to
markdown.inlinepatterns.Pattern.
Due to above changes verbose_compile is no longer used for
precompiling STREAM_LINK_REGEX, STREAM_TOPIC_LINK_REGEX as
adds ^(.*?) and (.*?)$ which cause extra overhead of matching
pattern which is not required. With new InlineProcessor these
extra patterns at beggining and end are not required.
So, StreamPattern and StreamTopicPattern now define their own
__init__ method for precompiling the regex.
Fixes#17535.
These changes were tested locally in dev server and by adding
some new markdown tests to test these.
Modifies `UserGroupMentionPattern` to inherit from InlineProcessor
instead of Pattern. This change is done because Pattern
stopped checking for matching patterns as soon as it found
a match which was not a valid user group. Due to this all
the subsequent user group mention failed, even if they were
valid. This bug was only present in backend renderring due to
markdown.inlinepatterns.Pattern.
This was reported as issue #17535.
These changes were tested locally in dev server and by adding
some new markdown tests to test these.
Modifies `UserMentionPattern` to inherit from InlineProcessor
instead of Pattern. This change is done because Pattern
stopped checking for matching patterns as soon as it found
a match which was not a valid user. Due to this all the
subsequent user mention failed. This bug was only present in
backend renderring due to markdown.inlinepatterns.Pattern.
This was reported as issue #17535.
These changes were tested locally in dev server and by adding
some new markdown tests to test these.
This removes the `add` from op list of stream event, as we do not
actually generate the stream/add event in the API, and when a stream
is created we identify it using the `create` operation.
(This was likely just a mistake introduced as a result of the fact
that `create` does not fit the normal naming scheme; probably
long-term we should actually migrate this to "add", but more important
for now is to document what's accurate).
Part of #17568.
This is preparatory work for investigating reports of missing unread
messages.
It's a little surprising that not test failed after adding the code
without API documentation.
Co-Author-By: Tushar Upadhyay (tushar912).
This is a prep commit which modifies the
`send_message_moved_breadcrumbs` function to take
message strings as input.
This is done to reuse the function in other places
like the /digress command.
Structurally, exception, failure_message, and status_code are mutually
exclusive in how this function is called, and it's best for the
function's flow to represent that.
The message from the bot which triggered the 407 error message notifies
the bot owner about the exceptions as well in the error message. This
commit handles it more gracefully and shows a generic message.
The messages from the bot which were triggered by the outgoing_webhooks
didn't have the bot name in them. This commit adds the bot name to it
and makes the corresponding changes in the tests.
This adds an option for restricting a ldap user
to only be allowed to login into certain realms.
This is done by configuring an attribute mapping of "org_membership"
to an ldap attribute that will contain the list of subdomains the ldap
user is allowed to access. This is analogous to how it's done in SAML.
Co-authored-by: Mateusz Mandera <mateusz.mandera@zulip.com>
On replying to an email notifcation from a stream where the user
does not come under the stream_post_policy will subsequently result
in a failure. In such a case, the user does not receive feedback
regarding the failure.
Notify the user via notification bot if their email
message failed to send.
Fixes#16642.
If the client has an old version of the code which is not present on
the server, don't throw a 500; instead, default to the same `unable to
look up in source map` message is used when the line numbers don't
line up.
I have updated the documentation for the Zabbix integration to give the
correct instructions for the latest version of Zabbix (5.2). The old
instructions are now obsolete.
I have also updated the message that is PMd to a user if the webhook
doesn't receive a complete payload to also align with the new
instructions.
Using get_user_profile_by_email is invalid, as it omits the realm, and
also fetches via .delivery_email - our convention is that .email is
supposed to be used for user-facing purposes like this.
self.example_user("hamlet") uses get_user_by_delivery_email, so it
doesn't actually cache anything. This should use a cached function, like
the test below: test_do_change_realm_subdomain_clears_user_realm_cache.
This is part of our general process of replacing emails, which are not
static with time, with user_ids when referring to users in the API.
We still keep the `email` reference option, since it can be useful for
linking third-party applications to Zulip on an intranet that might
have a user's corporate email handy and not want to do the extra round
trip to lookup the user.
The name of the parameter, user_id_or_email, was chosen to to make it
clear that the default/preferred option is user_id.
Fixes#14304.
TextField is used to allow users to set long stream + topic narrow
names in the urls.
We currently restrict users to only set "all_messages" and
"recent_topics" as narrows.
This commit achieves 3 things:
* Removes recent topics as the default view which loads when
hash is empty.
* Loads default_view when hash is empty.
* Loads default_view on pressing escape key when it is unhandled by
other present UI elements.
NOTE: After this commit loading zulip with an empty hash will
automatically set hash to default_view. Ideally, we'd just display
the default view without a hash, but that involves extra complexity.
One exception is when user is trying to load an overlay directly,
i.e. zulip is loaded with an overlay hash. In this case,
we render recent topics is background irrespective of default_view.
We consider this last detail to be a bug not important enough to block
adding this setting.
The query string parameter authentication method is now deprecated for
newly created Slack applications since the 24th of February[1]. This
causes Slack imports to fail, claiming that the token has none of the
required scopes.
Two methods can be used to solve this problem: either include the
authentication token in the header of an HTTP GET request, or include
it in the body of an HTTP POST request. The former is preferred, as
the code was already written to use HTTP GET requests.
Change the way the parameters are passed to the "requests.get" method
calls, to pass the token via the `Authorization` header.
[1] https://api.slack.com/changelog/2020-11-no-more-tokens-in-querystrings-for-newly-created-appsFixes: #17408.
sahil839
vagrant
Alex Vandiver
shanukun
Suyash Vardhan Mathur
m-e-l-u-h-a-n
Anders Kaseorg
Tim Abbott
tushar912
Riken Shah
Aman Agrawal
Adam Birds
Mateusz Mandera
akshatdalton
Vishnu KS
Sourabh
Steve Howell
Abhijeet Prasad Bodas
Siddharth Asthana
Strifel
Gaurav Pandey
Cyril Pletinckx