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.

Sign up for GitHub

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

Jump to bottom

Add streams:subscribed search operand #13052

Open
timabbott opened this issue Aug 22, 2019 · 16 comments · May be fixed by #17329
Open

Add streams:subscribed search operand #13052

timabbott opened this issue Aug 22, 2019 · 16 comments · May be fixed by #17329
Labels
area: search difficult Issues which we expect to be quite difficult new feature A proposed new feature for the product

Comments

@timabbott
Copy link
Sponsor Member

Following #12999 (which completes #8859), we should also add the planned streams:subscribed search operator. This is a somewhat awkward thing to implement, in large part because we don't have existing search queries with an OR gate between items with a UserMessage row or not, and this will violate our search base-then-add-conditions query archictecture. I think the operator should do the following:

  • First, we should try to refactor get_base_query_for_search to use a new join_with_usermessage function to create the need_message and need_usermessage case; we'll reuse this function later. If this can't be done without regressing, we don't have to, but the this should help us be confident that the below flow works.
  • Compute search_recipient_ids and full_history_recipient_ids, partitioning the recipient IDs for the user, depending on whether history is available to you, and use them to create subscribed_recipient_cond and full_history_recipient_cond.
  • Have this operator have ok_to_include_history=True so we get just the message case
  • Create user_message_query = join_with_usermessage(message_query) (where message_query is whatever is passed in to the by_streams function). The purpose here is to ensure that any operators processed already apply to both code paths.
  • return query = or_(user_message_query.where(subscribed_recipient_cond)), message_query.where(full_history_recipient_cond).

Tagged as difficult just because this will take significant care especially in unit tests, but I think @thedeveloperr should be able to make it work given his past work :).
@timabbott timabbott added area: search difficult Issues which we expect to be quite difficult labels Aug 22, 2019
@zulipbot
Copy link
Member

Hello @zulip/server-search members, this issue was labeled with the "area: search" label, so you may want to check it out!

This was referenced Aug 22, 2019
Closed
Closed
@thedeveloperr
Copy link
Collaborator

@zulipbot claim

@zulipbot
Copy link
Member

zulipbot commented Sep 2, 2019 •
edited

Hello @thedeveloperr, you have been unassigned from this issue because you have not updated this issue or any referenced pull requests for over 14 days.

You can reclaim this issue or claim any other issue by commenting @zulipbot claim on that issue.

Thanks for your contributions, and hope to see you again soon!

@thedeveloperr
Copy link
Collaborator

@zulipbot claim

@zulipbot
Copy link
Member

zulipbot commented Sep 17, 2019 •
edited

Hello @thedeveloperr, you have been unassigned from this issue because you have not updated this issue or any referenced pull requests for over 14 days.

You can reclaim this issue or claim any other issue by commenting @zulipbot claim on that issue.

Thanks for your contributions, and hope to see you again soon!

@thedeveloperr
Copy link
Collaborator

Still working on it.

@thedeveloperr
Copy link
Collaborator

@zulipbot claim

@zulipbot
Copy link
Member

zulipbot commented Oct 10, 2019 •
edited

Hello @thedeveloperr, you have been unassigned from this issue because you have not updated this issue or any referenced pull requests for over 14 days.

You can reclaim this issue or claim any other issue by commenting @zulipbot claim on that issue.

Thanks for your contributions, and hope to see you again soon!

@thedeveloperr
Copy link
Collaborator

@zulipbot claim

Will resume my work on it.

@zulipbot
Copy link
Member

zulipbot commented Feb 15, 2020 •
edited

Hello @thedeveloperr, you have been unassigned from this issue because you have not updated this issue or any referenced pull requests for over 14 days.

You can reclaim this issue or claim any other issue by commenting @zulipbot claim on that issue.

Thanks for your contributions, and hope to see you again soon!

@thedeveloperr
Copy link
Collaborator

Still working on it.

@Gittenburg Gittenburg added the new feature A proposed new feature for the product label Jul 10, 2020
@tushar912
Copy link
Member

@timabbott Can i work on it.

@thedeveloperr
Copy link
Collaborator

@tushar912 hi feel free to pick it up. I was intially responsible for this issue but due to some personal issues I can't devote time to it. Please feel free to ping me on chat.zulip.org for any questions. Happy to help 😃

@tushar912
Copy link
Member

@thedeveloperr Thanks . I will start work on it.

@timabbott timabbott mentioned this issue Feb 5, 2021
Open
@zulipbot
Copy link
Member

Hello @tushar912, you claimed this issue to work on it, but this issue and any referenced pull requests haven't been updated for 10 days. Are you still working on this issue?

If so, please update this issue by leaving a comment on this issue to let me know that you're still working on it. Otherwise, I'll automatically remove you from this issue in 4 days.

If you've decided to work on something else, simply comment @zulipbot abandon so that someone else can claim it and continue from where you left off.

Thank you for your valuable contributions to Zulip!

@tushar912
Copy link
Member

@zulipbot still working

tushar912 added a commit to tushar912/zulip that referenced this issue Feb 17, 2021
@tushar912
search: Refactor get base_query_for_search.
ee4a1ac 
Added join_with_usermessage function and use
it in get_base_query_for_search to generate
query for the user_message case.

Fixes zulip#13052
@tushar912 tushar912 linked a pull request Feb 17, 2021 that will close this issue
Open
tushar912 added a commit to tushar912/zulip that referenced this issue Feb 17, 2021
@tushar912
search: Refactor get base_query_for_search.
61235ee 
Added join_with_usermessage function and use
it in get_base_query_for_search to generate
query for the user_message case.

Fixes zulip#13052
tushar912 added a commit to tushar912/zulip that referenced this issue Feb 17, 2021
@tushar912
search: Add function get_subscribed_streams_for_user_by_history_publi…
799e2d3 
…c_to_subscribers

This function is used to get subscribed streams
according to whether history is public to
subscribers or not .

Fixes zulip#13052
tushar912 added a commit to tushar912/zulip that referenced this issue Feb 20, 2021
@tushar912
search: Add function get_subscribed_streams_for_user_by_history_publi…
466b39a 
…c_to_subscribers

This function is used to get subscribed streams
according to whether history is public to
subscribers or not .

Fixes zulip#13052
tushar912 added a commit to tushar912/zulip that referenced this issue Feb 21, 2021
@tushar912
search: Add function get_subscribed_streams_for_user_by_history_publi…
1166437 
…c_to_subscribers

This function is used to get subscribed streams
according to whether history is public to
subscribers or not .

Fixes zulip#13052
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area: search difficult Issues which we expect to be quite difficult new feature A proposed new feature for the product
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[WIP]search: Add "streams:subscribed" search operand. tushar912/zulip
5 participants
@timabbott @thedeveloperr @zulipbot @Gittenburg @tushar912