Using user IDs instead of emails is more reliable since users can
have arbitrarily complex emails that are hard to encode in a URL.
This has led to NoReverseMatch exceptions in the past.
This commit adds related_name parameter to UserGroup.direct_members
such that we can use direct_groups instead of the default
usergroupmembership_set for getting all the groups of which the
user is direct member.
This commit also sets related_name of UserGroupMembership.user_group
and UserGroupMembership.user_profile to "+" which means that we will
not be having backward relations for these. This change is correct
since we would need to use the recursive queries to get all the
groups of a user and all the members of a group after we add the
subgroups concept in next commit. This leads to us using direct_members
field of UserGroup instead of usergroupmembership_set in mention code,
but this will soon be replaced with the recursive query function to
include subgroup's members as well.
Extracted this commit from #19866.
Authored-by : Anders Kaseorg <anders@zulip.com>
This commit makes the query in get_user_group_direct_members
efficient by directly fetching user-profile ids instead of
first fetching user profile object and then id.
It can be pretty annoying to lose your place when replying to an old
message, even though every other chat application does this. And it
doesn't really buy us much; the user can always scroll down if they
want to, we have a helpful notification about where their message is
(which could be improved), and then we don't need to add some sort of
new complicated logic to avoid marking messages as read unexpectedly,
which the existing logic for this block badly needed.
(It had existing logic of that form dating from the pre-unread counts
pointer era).
Fixes#11462.
The from_scroll=true setting has been present since essentially the
beginning of time (6ae117ea5f), and has
moved around a number of times since. It's possible that it was
correct with the UI model as originally implemented, but the behavior
it creates now is that sending a message in the home view does not
move the cursor, and sending in a narrow does, without any intent
behind that behavior.
Further, the logic for controlling whether to display a "Scroll down"
notification clearly expects that this code path will actually trigger
a scroll to the current message, which would be the case without the
from_scroll setting.
The previous commit apparently didn't have its migration attached properly.
Since this is just a reference to a ManyToManyField, this migration
doesn't actually modify the database, but it is needed for CI to pass.
This commit renames members field of UserGroup to direct_members
for better readability because in the new permissions model, a
user group can be a sub-group of another group and thus technically
members of sub-group will also be members of that group.
This is a prep commit for new permissions model.
Extracted this commit from #19866.
Co-authored-by: Anders Kaseorg <anders@zulip.com>
This is a prep commit for new permissions model in
which a user group would be able to have a subgroup.
This commit renames get_memberships_of_users to
get_direct_memberships_of_users to specify that
the function is used only to fetch the direct
memberships and not memberships of subgroups of
the direct group.
Extracted this commit from #19866.
Co-authored-by: Anders Kaseorg <anders@zulip.com>
This is a prep commit for new permissions model in which a user
group would be able to have a subgroup.
This commit renames get_user_groups to get_direct_user_groups
to specify that the function is used only to fetch the direct
groups that user is part of and not subgroups of the direct
group.
Extracted this commit from #19866.
Co-authored-by: Anders Kaseorg <anders@zulip.com>
This is a prep commit for new permissions model in which a user group would
be able to have a subgroup.
This commit renames get_user_group_members to get_user_group_direct_members
to specify that the function is used only to fetch direct members of group
and excludes the subgroup's members.
Extracted this commit from #19866.
Co-authored-by: Anders Kaseorg <anders@zulip.com>
This commit adds django-cte as dependency
which will be used for querying recursive
group membership.
Extracted this commit from #19866.
Co-authored-by: Anders Kaseorg <anders@zulip.com>
This commit refactors the display settings code to use
a single handler similar to what we do in notification
settings. We still keep default language and emojiset
setting as they are handled differently and they do
not call 'change_display_setting' directly on changing
input.
We refactor the 'change_display_setting' to directly
accept status element as parameter and not class as
a string. We also add a common class to the subsection
div such that we can get status element for each of
them easily.
We do not need to check for_realm_settings variable for
left_side_userlist handler since realm-level defaults
now uses save-discard widget and we return early in
that case and thus for_realm_settings will always be
false at this point.
There is a bug when a newly created stream appears in the notification
settings table if user changes any setting in 'Streams' row of the
table even though user has not changed the notification setting of
the stream and thus the stream should follow the global-level values.
The stream is correctly not added to the table if user changes the
setting after reloading once after creating the stream.
The bug is due to the notification settings of the new-stream being
set to the user's global settings at time of stream creation which
are not overridden since the stream-creation event contains only
stream fields and not subscription fields. And the newly created
stream is not present in the table after reload because during
initialization of the client-level data structures the notification
setting values are overridden by the values stored in server.
This commit fixes create_sub_from_server_data code to initially have
the notification settings set as null in the sub object which are
overridden by the correct value during initialization. This keeps
the notification setting values as null just after stream creation.
Fixes#19933.
This will be useful to let users enable/disable
sharing read receipts once we add that feature.
Note: Added "I've" to IGNORED_PHRASES in
tools/lib/capitalization.py to avoid capitalization
errors for the label text of this setting.
Note: These are not functional in enabling/disabling sending of
typing notifications with this commit.
Refactored the privacy settings update to keep the code less
duplicated along with making the addition of new settings easier.
From 430c5cb, in `fetch_initial_state_data`,
we only include legacy settings in the top level of
`state` and the newer ones are stored in `state['user_settings']`.
That should've had a corresponding change in apply_event().
Also, fixed a test related to this logic.
From 430c5cb, we do not send events for settings added after
introduction of `user_settings` event.
This test wasn't considering that and started failing on
adding new user_settings.
This isn't necessary as `settings_checkbox.hbs` template used
for presence enabled setting in `account_settings.hbs` takes
care of checking/unchecking this checkbox.
The previous code has a lot of code unrelated to mock_template.
So, this commit replaces it with a better example which also
demonstrates how to test conditionals in templates.
I believe we intended to show 'several users are typing...' only
if number of typists are more than 3 but not 2.
The variable name MAX_USERS_TO_DISPLAY_NAME and commit message of
dba21d201c which added this also
suggests the same.
Translation handlebars helpers were not getting changed when we
mock static/js/i18n.ts and then unmock it in a different test.
We mock i18n.ts for every test except node_tests/i18n.js.
So, running any code that calls the registered handlebar
helpers related to translations before running i18n test
will run the helper functions registered with mocked i18n module.
The subsequent tests too would use those functions registered
with mocked i18n module.
This causes the i18n test to fail as we expect the registered
helpers to use actual i18n.ts instead of a mock if we have
code that would call the registered translation helpers in
tests that run before i18n.
We remove patch_url from settings_notifications.user_settings_panel
since realm-level defaults section uses save-discard widget and its
code is separate from user notification settings and we can directly
use "/json/settings" as url in user settings code.
We do not need patch_url field in realm_default_settings_panel
because we use save-discard widget in realm-level defaults section
which is handled separately from user display settings.
We remove patch_url from settings_display.user_settings_panel
since realm-level defaults section uses save-discard widget
and its code is separate from user display settings and we
can directly use "/json/settings" as url in user settings code.
We don't need container element and patch_url in change_display_setting
since this function is only used for user-display settings and not for
realm-level settings which now uses save-discard widget. So we do not
pass settings_panel as parameter to change_display_setting.
We added language_modal_elem to settings_display.user_settings_panel
object considering there would be realm-level default_language
setting and we would need to access the modal element using a
variable to avoid code duplication. But now we have decided that
we do not want the setting and we would instead use browser's
language to set the language for new user, so now we do not need
this variable and we can instead use id of the modal directly.
This commit removes _test_user_settings_for_adding_streams
and its callers for testing public and private streams
because it uses excessive mocking and we also test the same
thing in _test_user_settings_for_creating_streams without
mocking, so this test doesn't add anything.
Anders Kaseorg
Mateusz Mandera
Eeshan Garg
Sahil Batra
Dinesh
Aman Agrawal
Tim Abbott
anurastogiji
Adam Benesh
Johan Ehinger