Ref T7584. This hits all the major bullets there. Next step on T7584 is figuring out how it integrates into the full UI and column UI. That said, this is a bit buggy feeling right now since Conpherence as is assumes you are a participant all over the place and rooms make no such assumption. I'll probably this bit up next.
Details
- Reviewers
epriestley chad - Maniphest Tasks
- T7584: Add an ApplicationSearch UI for rooms to Conpherence
- Commits
- Restricted Diffusion Commit
rP014bb720505f: Conpherence - add "room" search UI and create UI
viewed /conpherence/room/ and saw stuff. viewed the "participant" query as two different users and saw different correct result sets. made a room via the button and it worked. tried to view a room I wasn't a participant in and it failed horribly, which is something to fix in a future diff
created a thread via "send message" on a user profile and it worked
Diff Detail
- Repository
- rP Phabricator
- Branch
- T7584
- Lint
Lint Passed Severity Location Code Message Advice src/applications/conpherence/query/ConpherenceThreadSearchEngine.php:73 XHP16 TODO Comment - Unit
Tests Passed - Build Status
Buildable 4926 Build 4944: [Placeholder Plan] Wait for 30 Seconds
Event Timeline
src/applications/conpherence/query/ConpherenceThreadQuery.php | ||
---|---|---|
119–124 | Do I need to group by conpherence_thread.id if I do this join? (I think I might get duplicate results if e.g. you and I were in the same room and we were both desired participants...) | |
src/applications/conpherence/query/ConpherenceThreadSearchEngine.php | ||
6–8 | Could get rid of this; maybe just name this ConpherenceRoomSearchEngine? |
src/applications/conpherence/controller/ConpherenceNewRoomController.php | ||
---|---|---|
61 | Mildly prefer $this->newDialog(), which will do setUser() for you to save a little typing. | |
src/applications/conpherence/editor/ConpherenceEditor.php | ||
498 | Does this have the side effect of requiring a thread title for non-rooms? I don't think Profile > Send Message, at a minimum, even has a way to set a thread title. Does that still work? | |
src/applications/conpherence/query/ConpherenceThreadQuery.php | ||
119–124 | Yep, for exactly that reason. | |
src/applications/conpherence/query/ConpherenceThreadSearchEngine.php | ||
6–8 | I'd expect this to eventually be a dropdown something like this, and for this to remain "ThreadSearchEngine": Show: [Only Rooms] [Only Threads] [Threads and Rooms] But whatever's easiest here seems fine. In the long run, I think we should likely have a single UI/SearchEngine for both, though, since I'd imagine that a lot of queries don't care about the room type (e.g., "I know I talked with Bob and Joe about this, let me go find all the conversations where we were all participants, regardless of whether they were rooms or threads"). Or, with search, "find all rooms/threads that mentioned 'poodle'". Maybe that will never actually happen in practice depending on where things go, but I'd imagine being able to get rooms + threads back in a single view is useful is reasonably likely to be useful somewhere in this gamut of use cases. | |
134 | Maybe add getMonogram() to ConpherenceThread at some point. |