The proposed_val for "realm_can_access_all_users_group" setting
was always returning NaN value which resulted in the value
being included in request even when it was not changed or disabled
due to org not being on "Plus" plan. This resulted in a bug where
user was not allowed to change the "guest" indicator setting
on orgs without "Plus" plan.
For orgs with "Plus" plan there was no problem in changing the
setting but the save button was visible incorrectly if the setting
was set to the initial value.
This commit fixes both the bugs.
The "Who can access all users" setting is not disabled for
organizations not on "Plus" plans in production because of
the "page_params.development_environment" check.
We do not need that check anymore as the feature is now
enabled in production.
The previous logic would treat navigating with the arrow keys, or
alt-tab, as a reason to clear the subdomain unavailable warning, and
would not necessarily check again immediately. Now just trigger off
changes in the input.
Instead, just trigger off actual changes in the value of the input.
On /upgrade page, we show annual schedule & price for a
fixed-price plan, by default.
This also acts as a prep commit for fixed-price plan
with pay-by-invoice collection method as we only offer
annual plan in such case. So, annual data is shown by default
on /upgrade page.
Many of these code paths largely expect it to be a valid user ID, so
we could raise an exception if the user ID is invalid, but probably
gracefully proceeding is reasonable for the callers.
This correctly avoids pageup/pagedown scrolling past already rendered
content, while also always making progress, which the previous logic
introduced in d8ec141de2 might do, at
least in degenerate cases with very small render windows.
I'm not sure this change has any effect in practice with current
render window values, but it does fix a bug if I set the maximum
render window size to 20.
The previous batch of improvements to this code path in
6562ea94e4 introduced a race bug where:
- You navigate to a narrowed view; Zulip renders cached data from
`all_messages_data` that we know is current, but
`fetch_status.has_found_newest` is initialized to `false`
nonetheless.
- The bottom is visible, triggering the check for whether the view
should be marked as read.
- `fetch_status.has_found_newest` is still `false`, and so we
incorrectly refuse to mark as read.
- We finish fetching data from the server, do the background rerender
for that (with no changes), but that doesn't trigger a re-check for
whether the bottom is visible.
There's several ways to address this, but most correct to me is to not
check fetch_status in this particular code path.
The same reasoning applies to the navigate.js call sites.
Previously, when no parameter was passed to the get_timestamp_for_
flatpickr method, it would result in an uncaught exception. This is
breaking the "Add global time" of compose bar.
This can be avoided by doing an early return of current time to hour
in case no string is passed.
This was introduced in #28767 with the intention to skip scrolling
the selected message.
So, the actual bug that the PR fixed would have been just fixed
by opening the compose box early.
This matches the algorithm that we designed for the Python API, except
that we use a ratio of 2 rather than sqrt(2) in the message_fetch code
path, because it's a heavier request.
We increase the number of failures before showing a user-facing error
to roughly preserve the same time period before a user-facing error is
shown.
Previously, these endpoints just did exponential backoff, without
looking at the rate-limiting headers returned by the server, resulting
in requests that the client could have been certain would fail with an
additional rate-limiting error.
Fix this by using the maximum of the existing exponential backoff with
the value returned by the rate-limiting header.
Fixes#28807.
This should help reduce the risk of hitting rate limits when users
have a very large number of messages to fetch via this mechanism.
Inline the `messages` variable that was only used in one place while
we're touching this.
This function incorrectly and misleadingly did an immediate initial
call, despite both of its callers doing immediate calls themselves (in
one case, with a different parameter passed).
This led to unnecessary server load when reloading the app via event
system triggered reloads, since every client would call `/` twice.
I was not able to reproduce obviously badly broken behavior from these
logic bugs, but after the renaming of message_viewport helpers in the
last few commits, it's clear that this logic was trying to check if
we're actually at the start/end of the possibly message feed, not just
the rendered portion, and doing so incorrectly.
The previous logic for both scrolling down and using pagedown would
incorrectly mark an entire conversation as read when reaching the
bottom of a render window, even if there were more messages loaded or
to fetch from the server.
Fix this error in the calculation by asking the correct data
structures if we're actually at the bottom.
To avoid the navigate.js keyboard shortcut code paths circumventing
this new logic, or needing to duplicate it, they now call
process_visible, rather than its helper.
Tim Abbott
Karl Stolley
Sahil Batra
Varun Singh
Prakhar Pratyush
David Rosa
evykassirer
roanster007
Aman Agrawal