Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

notifications: Add initial implementation for reaction notifications. #27896

Closed
wants to merge 269 commits into from

Conversation

akarsh-jain-790
Copy link
Collaborator

Fixes: #27327

Screenshots and screen captures:

Issue-27327.mov
Self-review checklist
  • Self-reviewed the changes for clarity and maintainability
    (variable names, code reuse, readability, etc.).

Communicate decisions, questions, and potential concerns.

  • Explains differences from previous plans (e.g., issue description).
  • Highlights technical choices and bugs encountered.
  • Calls out remaining decisions and concerns.
  • Automated tests verify logic where appropriate.

Individual commits are ready for review (see commit discipline).

  • Each commit is a coherent idea.
  • Commit message(s) explain reasoning and motivation for changes.

Completed manual review and testing of the following:

  • Visual appearance of the changes.
  • Responsiveness and internationalization.
  • Strings and tooltips.
  • End-to-end functionality of buttons, interactions and flows.
  • Corner cases, error conditions, and easily imagined bugs.

andersk and others added 29 commits November 27, 2023 19:40
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Signed-off-by: Anders Kaseorg <anders@zulip.com>
This previous parameter name was inaccurate, since that's not what the
caller is actually asserting for us.
There's no good reason to have the caller of deactivate pass this
parameter in.

This effectively reverts a18b166,
which did this as part of trying to avoid an import cycle, with a more
appropriate solution using the existing message_scroll_state module.

Importantly, it also means that we again wait for scrolls longer than
50ms to finish before opening All messages; I think this might fix a
regression.
Co-Authored-By: Tim Abbott <tabbott@zulip.com>
Testing experimentally, removing the setTimeout seems to fix a visible
flicker when using Esc to navigate to "All messages" from the Inbox
view. That setTimeout has been moved around without real examination
since 5d79bb6 from early 2013; I
don't see any good reason why it would make be necessary only in the
"All messages" code path, and not when narrowing to any other view.
Since at least 6ef0753, it's been the
case that narrow.activate already hides the inbox/recent views if
open, and the same is true for all messages.

Fixing the duplicate call is important in show_home_view, because
show_all_message_view relies on having an accurate value for whether the
recent/inbox views were already open in order to correctly update the
left sidebar.
We already do a very parallel construction in narrow.activate, so this
moves us towards being able to unify those code paths, while also just
being more readable by avoiding a small-but-important wrapper function
in hashchange.js.

I believe this fixes a bug where we were not saving scroll position in
browser history when navigating to "All messages" from another view.
This should now happen at the same time it did prior to this change,
without requiring the show_all_message_view wrapper to have any
business logic.

This fixes a potential scroll position bug in the event that
narrow.deactivate in fact calls itself recursively after a timeout.
Documents this new feature as a tip for the existing instructions.

Fixes zulip#27655.
The word "Filtrar" is ambiguous in this context since it can be
interpreted as "filter out" which is the opposite of what we want
here. "Buscar solo" is a better phrase that we can use unambiguously
and consistently for all instances of "Narrow to".
This commit moves the main context creation part of the
'billing_home` view to a new shared
'BillingSession.get_billing_page_context' method.

This refactoring will help in minimizing duplicate code
while supporting both realm and remote_server customers.
Documents new wildcard mention for topic participants updating
and making tweaks to all relevant pages.

Fixes zulip#27657.

Co-authored-by: Alya Abbott <alya@zulip.com>
This reverts commit 273081d.

This change broken the arrow key navigation in the lightbox.
This commit removes "email_address" field from Subscription objects
and we would instead a new endpoint in next commit to get email
address for stream with proper access check.

This change also fixes the bug where we would include email address
for the unsubscribed private stream as well when user did not have
permission to send message to the stream, and having email allowed
the unsubscribed user to send message to the stream.

Note that the unsubscribed user can still send message to the stream
if the user had noted down the email before being unsubscribed
and the stream token is not changed after unsubscribing the user.
This commit adds new API endpoint to get stream email which is
used by the web-app as well to get the email when a user tries
to open the stream email modal.

The stream email is returned only to the users who have access
to it. Specifically for private streams only subscribed users
have access to its email. And for public streams, all non-guest
users and only subscribed guests have access to its email.
All users can access email of web-public streams.
It was discovered by the Zulip development team that active users who
had previously been subscribed to a stream incorrectly continued being
able to use the Zulip API to access metadata for that stream. As a
result, users who had been removed from a stream, but still had an
account in the organization, could still view metadata for that
stream (including the stream name, description, settings, and an email
address used to send emails into the stream via the incoming email
integration). This potentially allowed users to see changes to a
stream’s metadata after they had lost access to the stream.

This bug was present in all Zulip releases prior to today's Zulip
Server 7.5.
This commit adds code to send stream deletion events when
unsubscribing non-admin users from private streams and
when unsubscribing guests from public streams since
non-admins cannot access unsubscribed private streams
and guests cannot access unsubscribed public streams.
This will make it possible to send notifications to multiple
distinct app IDs over the same connection.
Riken-Shah and others added 29 commits November 27, 2023 19:40
This commit introduces non-intro hotspots.
They are a bit different than intro hotspots in the
following ways:

* All the non-intro hotspots are sent at once instead of
sending them one by one like intro hotspots.

* They only activate when a specific event occurs,
unlike intro hotspot where they activate after the
previous hotspot is read.
This commit also solves a bug where it displayed
multiple copies of the hotspots when
`ALWAYS_SEND_ALL_HOTSPOTS` is set to true.
Generally, hotspots popover are depended on
the `?` icon to click to activate.

As we have introduced non-intro hotspots, the (non
intro) hotspot popover will open immediately after a
specific event occurs, they won't wait for `?` to click to
activate.

This function helps non-intro hotspots popover to open
easily.
This commit refactors the current hotspot subsytem to use a more
robust dataclass `Hotspot` defined in `lib/hotspots.py`. This fixes
mypy errors as well as make code more readable.
Replaced for "(initial_upgrade)", " initial_upgrade"
`'initial_upgrade'` and `"initial_upgrade"`.
This will help simplify things for remote realms.
This will help us verify changes easily.
The will make redirects to sponsorship page work for all
BillingSession child classes.
This logic has been #nocoverage since its implementation, but since this
is an authentication codepath, it seems important for it to have a test.
Renames CustomerPlan.SWITCH_NOW_FROM_STANDARD_TO_PLUS to be more
generic, CustomerPlan.SWITCH_PLAN_TIER_NOW.

Because the plan tier change is immediate, moves the code to end
the current plan with the old tier and create the new plan with
the new tier from `make_end_of_cycle_updates_if_needed` to instead
be in a separate helper function, `switch_plan_tier`.
Simplifies the initial check for switching a customer plan tier
for an existing and active plan, so that it is more generic.
Moves and generalizes `switch_realm_from_standard_to_plus_plan`
in stripe.py to be a more general function for changing a
CustomerPlan to a new and valid tier, `do_change_plan_to_new_tier`.

Adds a helper function with the previous function name to be used
for the support view and management command for changing a realm
from the Standard plan tier to the Plus plan tier.
This allows re-use in other test_*.py files, which may also want to test
bouncer-reliant logic.
`zulip-limited-section` is not longer present and
we don't want `free-trial-alert-message` to hide on any form
submission on the page.
This message is barely visible to the user so removed. Now, the
card change behaviour is same as on upgrade page.
Earlier, for the push notifications having latex math
like "$$1 \oplus 0 = 1$$, the notification had the math
included multiple times.

This commit fixes the incorrect behavior by replacing
the KaTeX with the raw LaTeX source.

Fixes part of zulip#25289.
Earlier, for the desktop notifications having latex math
like "$$1 \oplus 0 = 1$$, the notification had the math
included multiple times.

This commit fixes the incorrect behavior by replacing
the KaTeX with the raw LaTeX source.

Fixes zulip#25289.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Notify message sender about emoji reactions