Page MenuHomePhabricator

Conpherence - add "room" search UI and create UI
ClosedPublic

Authored by btrahan on Mar 19 2015, 9:49 PM.
Tags
None
Referenced Files
F14064943: D12113.diff
Tue, Nov 19, 2:02 AM
F14064306: D12113.diff
Mon, Nov 18, 10:51 PM
F14052073: D12113.diff
Fri, Nov 15, 6:14 AM
F14040934: D12113.diff
Mon, Nov 11, 2:23 PM
F14024052: D12113.diff
Thu, Nov 7, 5:16 AM
F14016058: D12113.id29135.diff
Mon, Nov 4, 4:53 AM
F14016057: D12113.id29198.diff
Mon, Nov 4, 4:53 AM
F14007475: D12113.diff
Tue, Oct 29, 6:29 AM
Subscribers
Tokens
"Yellow Medal" token, awarded by epriestley.

Details

Summary

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.

Test Plan

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
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

btrahan retitled this revision from to Conpherence - add "room" search UI and create UI.
btrahan updated this object.
btrahan edited the test plan for this revision. (Show Details)
btrahan added reviewers: epriestley, chad.
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
7–9

Could get rid of this; maybe just name this ConpherenceRoomSearchEngine?

epriestley edited edge metadata.
epriestley added inline comments.
src/applications/conpherence/controller/ConpherenceNewRoomController.php
62

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
7–9

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.

135

Maybe add getMonogram() to ConpherenceThread at some point.

This revision is now accepted and ready to land.Mar 20 2015, 12:00 AM
btrahan edited edge metadata.

updates as requested
rebase

This revision was automatically updated to reflect the committed changes.