-
-
Notifications
You must be signed in to change notification settings - Fork 8.1k
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
UI option to subscribe existing accounts during bulk invites. #16041
base: main
Are you sure you want to change the base?
UI option to subscribe existing accounts during bulk invites. #16041
Conversation
Hello @zulip/server-onboarding members, this pull request was labeled with the "area: onboarding" label, so you may want to check it out! |
@timabbott This is ready for a review. We currently don't have tests for |
I merged the first two commits; haven't had a chance to read the full branch yet. I don't think we need to add automated tests for this necessarily. |
a6fc7db
to
cc80048
Compare
Rebased 👍 |
cc80048
to
06a161d
Compare
@timabbott FYI, in case you forgot to give this a review :) |
… users. Previously during an error with invite_users_backend codepath, we returned 3 error items - invited email, error message, is_deactivated. However, for organizations which haven't enabled EMAIL_ADDRESS_VISIBILITY_EVERYONE the actual user email stored in the frontend is anonymised and different from the invited email. To fix this, we return back the anonymous emails for existing accounts along with the other parameters. This results in 4 return values -> - email (invited email) - error_msg - is_deactivated - maybe_anonymous_email Incase EMAIL_ADDRESS_VISIBILITY_EVERYONE is enabled for the realm, maybe_anonymous_email would be same as the invited email. This is also the case for cross_realm_bots. For invalid emails, we just return back no strings. Tests amended.
When the email_address_visibility realm setting is switched to Realm.EMAIL_ADDRESS_VISIBILITY_EVERYONE, get_existing_user_errors returns the maybe_anonymous_email parameter, which in this case should match with the delivery_email.
This flag helps the frontend to know when to display the option of subscribing users on invitation. Currently we show the option in two cases: - When no one was invited (Probably all users exist already) - When some were invited while some weren't. We do not show the subcription option when: - We fail to send any invites because emails were invalid. - Account is too new to send invites.
06a161d
to
abe2b89
Compare
When inviting existing accounts in the realm, the user is presented with an option to subscribe existing users to the marked streams. The error option displays which accounts are being subscribed to which streams. The actual stream subscription can result in three cases: 1. All the users were newly subscribed to the selected streams. 2. All the users were already part of selected streams. 3. Some users got newly subscribed, while some others were already subscribed. Appropriate error messages are show. Fixes zulip#15784.
abe2b89
to
4916ebf
Compare
Heads up @sumanthvrao, we just merged some commits that conflict with the changes your made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the |
4ec3636
to
88b200c
Compare
Commit structure
Fixes #15784
Testing Plan:
I tested it manually for the most part. Of course, the backend tests pass as well.
GIFs or Screenshots:
Feedback/review is much appreciated.