For certain queries where both include_history and
use_first_unread_anchor are set to True, we were excluding
historical rows. Now we only use the use_first_unread_anchor
flag to filter rows that we use to find the anchor, without
having it filter the actual search results.
The bug went unreported for a long time, because it only
affected mobile users who had newly subscribed to streams.
Note that we make a small change to the test called
test_use_first_unread_anchor_with_muted_topics, which has
a very scary comment about being "arcane" and "be
absolutely sure you know what you're doing." I think it's
fine.
Also, the new test code would fail before this fix, so it
should help prevent future regressions.
Fixes#8958
This is a bit more than a pure refactor, because we duplicate a
chunk of code to calculate a query inside of
find_first_unread_anchor(), so we're doing a bit more work
than before.
We need this refactoring to start decoupling find_first_unread_anchor
from get_messages_backend for the case where include_history is
True. This will happen in a subsequent commit.
The only test that changes here is a direct test on
find_first_unread_anchor(). All other tests pass without
modification, and we have decent coverage on get_messages_backend.
We use an array now to build up the list of search operands and
then consolidate the special search handling after the loop (which
means setting the flag, putting two more columns in the query, and
using ' '.join to build the string).
We have a debugging statement for some obscure errors we get
when narrows have search terms. We now show all the narrow
operators. This isn't really to improve debugging; it's more
to make it easier in the next commit to extract a function
that would make search_term have to be passed back in a tuple.
But it shouldn't hurt debugging either.
Refactoring in this file had resulted in the logic for
html_settings_link being duplicated and extra logic being needed to
ensure these variables were set where they were needed.
This fixes subscriptions_html not being rendered properly in the /help
and /api pages, in addition to removing duplicate code.
This is a pure refactoring and just pulls the function out
to the top level of the module. (The prior commit extracted
it inside a larger function to make a nicer diff.)
Remove models.get_user_profiles_by_ids() and
obtain user's bots profiles in actions.get_service_dicts_for_bots() by
users.user_ids_to_users() instead of models.get_user_profiles_by_ids().
Fixes#8939
We don't indend for this server-level setting to exist in the long
term; the purpose of this is just to make it easy to test this code
path for development purposes.
This implements much of the Message side part of #2745.
The new name can_access_stream_history_by_name gets to the point of
what this function actually does. And passing in a user object lets
us define what this does based on the user subscribed.
The comments explain this pretty well, but basically because we
rewrite the realm ID during the import process, we need to edit all
the message bodies that link to an attachment to instead link to the
post-processed URL where that file will be hosted on the new server.
Fixes#8926.
Implement few optimizations for reading admin's bot dicts from database
for a constants number of requests:
- add models.get_user_profiles_by_ids() for reading bots profiles
by single query from database
- add models.get_services_for_bots() for reading services for bots
by single query from database
- add bot_config.get_bot_configs() for reading config data for bots
by single query from database
Fixes#8838
This fixes a bug where the endpoint for editing bot users would allow
an organization administrator to edit the full name of a bot user.
A combination of this an another recently fixed bug made it possible
for this process to set a `bot_owner` for a non-bot user; so we also
include a migration to fix that for any users that might have had our
model invariants corrupted in that way.
This was a user-reported bug and a very subtle and painful one
to track down.
Previously, if payload['push']['changes'][i]['closed'] was True,
we assumed that a branch was removed. Looking at whether `closed`
was set to True or not was our way to tell whether a push removed
a branch or not.
However, this is wrong! `closed` being set to True can also mean
that the pull request associated with the branch was approved but
the branch itself was not deleted. According to the BitBucket docs,
the correct way to see if a branch is deleted is to check if `new`
is null.
This bug was leading to KeyErrors about not being able to find
the `commits` key, which shouldn't happen anymore!
'processing_emojis' check is added in the 'import_uploads'
function, so that the emoji files present in the to be imported
data file can be uploaded.
The procedure of saving emoji files in slack importer is same as
saving attachments and avatars, and the import has the similar
procedure too.
Change 'get_user_data' function to a more general function
to get data from the slack api using legacy tokens.
Also, change the error handling as upon invalid token,
the response is 200, but the response has an error
field in it.
For eg. Go to the following link with invalid token:
https://slack.com/api/emoji.list?token=xoxp-249056023425
Remove allocation ID function from slack import script. All the IDs
count will start from 0. Hence the ID List returned
by the allocation function is of no use, and we remove its implementation.
(example: get_total_messages_and_attachments function is of no use anymore,
hence we remove it)
In importing avatars, we use the implementation where the 'avatar_path'
is seperately calculated using realm and user ID and then the content
of the path provided in the avatar's 'records.json' are copied to this
'avatar_path'.
Similary, here for the uploads, 's3_file_name' is seperately calculated
using the realm ID and uploaded file name and then the content of the
path provided in upload's 'records.json' are copied to this 's3_file_name'.
'recipient_field' is added as a bool variable in the function
'update_id_map' to update the recipient foreign keys.
Recipient Foreign Key is equal to the UserProfile ID, if the
type is 1, and the same is equal to Stream ID, if the type is 2.
Hence a check is added in the 'update_id_map' field for this.
All the objects with realm ID as the foreign keys need to
be remapped with updated with the allocated ID.
Also the ID of the realm object itself is updated with the allocated
ID.
The 'id_field' bool variable is added to the function just to check
if the field is the ID of that object, and not the foreign key relation.
For foreign key field names, a "_id" has to be added after the field name,
however we don't need that for the ID field of the object.
Fixes#8853.
In certain cases, the browser is not able to look up the message.
Include the recipient data for the message in the delete_message event,
so look up of those attributes by the browser isn't required.
This PR solves some of the parity issues in the emoticon translation
logic. I was unable to find a way of matching only one of the
lookaround groups, so we still have some inconsistency (see
testcase). The approach of having another check while converting just
for this seemed like an inefficient way, so I've left that last change
as it is.
This bot was basically a duplicate of NOTIFICATION_BOT for some
specific corner cases, and didn't add much value. It's better to just
eliminate it, which also removes some ugly corner cases around what
happens if the user account doesn't exist.
Since the test database is in part controlled by the Zulip settings
files for testing, these settings files should be included in the list
of files that require populate_db to be rerun.
This issue was found due to changes to internal bots.
This was causing a rather confusing test flake in
test_stream_error_pm_to_bot_owner. What was happening was that if
this test (which used that code path) ran within 5 minutes of the
populate_db run, it would fail.
Usually, to debug a small change, you have to remove some tests from JSON
because of lack of support for comments in JSON. This commit allows to
ignore some tests by setting `"ignore" : true` in the bugdown fixtures.
Also, since this is only for while developing, the complete test suite will
throw an error if we leave an 'ignored' test in a commit.
This is basically a simple fix, where we consistently set
`flags` to an empty array when we pass it around. The history
here is that we had kind of a nasty bug from setting it to
`None`, which only showed up in the somewhat obscure circumstance
of somebody subscribing to all stream events in our API.
Fixes#7921
Also switches the default behaviour of the code to not translate the
emoticons. Earlier, the code was testing-aware, and used to translate
when there was no user profile data available(assuming that as a testing
environment).
This number is way too high, because of a recent regression. Adding a
test here lets us prevent similar regressions in the future and
provides an easy way to be sure if we've fixed the issue.
webhook-errors.log file is cluttered with Stream.DoesNotExist
errors, which hides the errors that we actually need to see. So,
since check_message already sends the bot_owner a PM if the webhook
bot tries to send a message to a non-existent stream, we can ignore
such exceptions.
In 18e43895ff we replaced
stream subscribe buttons with stream links. The new feature
has been well tested and well received for over a year now,
so it's safe to remove the older feature at this point.
Older sites will have super old messages that still have the
rendered markup; this commit does not attempt to address those
situations. Most likely, clicking on an old button in the old
message will either do nothing or look like a message reply.
This cuts out about 11 calls to `git describe`. In a nice fast LXC
container following our instructions for development on a Linux host,
this might save "only" about 1.5s; in a dev environment on a Windows
host, the savings have been clocked at 49s, presumably due to an
extremely slow filesystem in the VM.
The tests weren't doing much with this codepath as they were, and
there isn't a lot of value to be gained by testing it anyway; it's
totally non-critical and rarely changes.
[Commit message rewritten by greg.]
Add function in user-groups.py for getting member ids
for a group.
Update view to enforce checks for modifying user-groups.
Only admins and user group members can modify user-groups.
This feature isn't really ready yet -- the relevance isn't good, so
the emails aren't a great experience. More work needed; pending that,
just don't send them.
There's already a per-realm setting, which doesn't have a control in
the org settings UI but does suppress it in the per-user settings UI.
Piggyback on that to suppress that UI control when the feature is
disabled at the server level too.
Also cut a comment that hasn't really made sense since the logic was
changed months ago -- the comment originally explained why we sent
digests on Tuesday, Wednesday, and Thursday, and doesn't correspond to
why we dialled back to weekly on Tuesdays.
The digest emails have little in common with the email mirror, beyond
that they both involve email. Give their tests their own file, with a
corresponding name, so it's easy to find this code's tests.
Steve Howell
Tim Abbott
Nikhil Kumar Mishra
novokrest
Rhea Parekh
Greg Price
Eeshan Garg
Umair Khan
Shubham Padia
Anurag Sharma
Shubham Dhama
Puneeth Chaganti
Rohitt Vashishtha
neiljp (Neil Pilgrim)
Balaji2198
sinwar
Ricky
Aayush Agrawal
Tarun Kumar