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

Refactor /search/chat_channels endpoint #13277

Closed
atsmith813 opened this issue Apr 6, 2021 · 11 comments
Closed

Refactor /search/chat_channels endpoint #13277

atsmith813 opened this issue Apr 6, 2021 · 11 comments
Labels
tech: fullstack changes will heavily involve both frontend and backend technologies tech: javascript changes will heavily involve both JavaScript code tech: ruby changes will heavily involve both Ruby code type: refactoring code refactors

Comments

@atsmith813
Copy link
Contributor

In #13235 we refactored how /search/chat_channels works by essentially removing "search" functionality and turning it into more of a "filter". As noted in the PR:

You'll notice I left everything in the Search namespace (the route, the controller, and the service naming). I originally started removing chat_channels from search all together since this behaves more like a filter than a search.

However, I discovered there are some JS components that are using this endpoint as an "index". I started refactoring everything, but it quickly got out of hand and out of scope for what this PR is intended to address. I also realized it may actually be nice to leave everything in the search namespace so that when we re-do the search for Connect in the future, it's a smoother experience.

With that said, since we have the ChatChannelsController and the ChatChannelMembershipsController each with index actions, I think the right move would be to move those JS components to them so we don't have multiple controllers performing the same actions.

This issue is to essentially update all the JS components on the frontend to use a newly refactored endpoint that lives in a more appropriate spot (i.e. ChatChannelMembershipsController or ChatChannelsController) than the SearchController. Once this issue is complete, the chat_channels action should no longer exist in the SearchController and instead, a new endpoint should be able to take care of any "filtering" and "indexing" being done by the JS components.

@atsmith813 atsmith813 added type: refactoring code refactors area: connect tech: ruby changes will heavily involve both Ruby code tech: javascript changes will heavily involve both JavaScript code tech: fullstack changes will heavily involve both frontend and backend technologies labels Apr 6, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Apr 6, 2021

Thanks for the issue, we will take it into consideration! Our team of engineers is busy working on many types of features, please give us time to get back to you.

Feature requests that require more discussion may be closed. Read more about our feature request process on forem.dev.

To our amazing contributors: issues labeled type: bug are always up for grabs, but for feature requests, please wait until we add a ready for dev before starting to work on it.

To claim an issue to work on, please leave a comment. If you've claimed the issue and need help, please ping @forem/oss. The OSS Community Manager or the engineers on OSS rotation will follow up.

For full info on how to contribute, please check out our contributors guide.

@rhymes
Copy link
Contributor

rhymes commented Apr 6, 2021

+1 on removing the code out of search and into a dedicated query object / service

@cmgorton cmgorton added the bug smash Approved bugs for the DEV community bug smash label May 4, 2021
@Rafi993
Copy link
Contributor

Rafi993 commented May 4, 2021

Hi, if this issue is not worked on I would like to help.

@cmgorton
Copy link
Contributor

cmgorton commented May 5, 2021

Hi, if this issue is not worked on I would like to help.

Thanks Rafi! Just assigned you to it.

@Rafi993
Copy link
Contributor

Rafi993 commented May 29, 2021

Sorry I did not look at this issue for a while I will look at this over next week but in the mean time if this is an urgent issue and I am blocking it do feel free to change the owner.

@cmgorton
Copy link
Contributor

cmgorton commented Jun 1, 2021

I don't believe it is urgent so if you still want to work on it that is totally fine. If not we can remove your assignment. Just let me know what works for you Rafi 😄

@Rafi993
Copy link
Contributor

Rafi993 commented Jun 1, 2021

Thank you @cmgorton. I'll actually un-assign myself now and comeback later to pick up this issue if this still open :)

@Rafi993 Rafi993 removed their assignment Jun 1, 2021
@twinsfan421
Copy link
Contributor

I can work on this

@cmgorton
Copy link
Contributor

I can work on this

Thank you!

@cmgorton
Copy link
Contributor

Hey @twinsfan421
I was reminded by a teammate that we are putting work on Connect on hold at the moment while we plan out the future of Connect and any work we want done on it. I don't want you to work on this issue only to have us remove the code later on, so I am going to unassign you for now. Please let me know if there are any other issues you would like to work on and thanks again for your willingness to contribute here🙏

@cmgorton cmgorton added inquiry: needs review and removed bug smash Approved bugs for the DEV community bug smash external contributors welcome contribution is welcome! labels Jul 21, 2021
@cmgorton
Copy link
Contributor

cmgorton commented Sep 9, 2021

I am going to close this issue since we are moving to deprecate connect.
Deprecating Connect Chat

@cmgorton cmgorton closed this as completed Sep 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tech: fullstack changes will heavily involve both frontend and backend technologies tech: javascript changes will heavily involve both JavaScript code tech: ruby changes will heavily involve both Ruby code type: refactoring code refactors
Projects
None yet
Development

No branches or pull requests

5 participants