BounceTribe / bouncetribe

The bounce tribe repository!
https://www.test.bouncetribe.com/
3 stars 2 forks source link

Filtering Relay Queries #11

Closed JakeIwen closed 7 years ago

JakeIwen commented 7 years ago

Any recommendation on how to get the direct message history between two people? In my implementation of direct messaging (not tied to a Session), I leave the SessionId blank/null and had planned to search for null session id's between two users.

Relay doesn't do nested variables (see pic).

I'm considering generating a sessionId for each two user direct message convo and using that to search. Feels kind of hack-y though.

Any insight here? see branch jake-carl-relay-test image

RPowerHimself commented 7 years ago

This might help:

http://www.vertabelo.com/blog/technical-articles/database-model-for-a-messaging-system

carlpeaslee commented 7 years ago

So the problem here is that in a lot of places we aren't really structuring out Relay containers correctly.

I've found that a better build pattern is to make sure that most component/containers are only responsible for fetching a single thing by id. So like, things should instead be:

Page Component (container fetches one user by ID and all of their messages) => Message List Component (reorders all of the messages by date and passes their ids to each message component) => Message Component fetches each individual message by ID

JakeIwen commented 7 years ago

Yeah it seems like we are requesting (what will become) excessive amounts of data when the main containers load. I'll check it out.

JakeIwen commented 7 years ago

SO the page component wouldn't actually fetch the message text - just the date and ID?

carlpeaslee commented 7 years ago

Correct. You just have the page get the list of ids and then each individual message component fetches it’s own message text. On Sat, Oct 14, 2017 at 3:04 AM Jake Iwen notifications@github.com wrote:

SO the page component wouldn't actually fetch the message text - just the date and ID?

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/BounceTribe/bouncetribe/issues/11#issuecomment-336526304, or mute the thread https://github.com/notifications/unsubscribe-auth/AOhs3jXx_8vkX_AFTeWcRT51g62o0oRtks5sr6Y0gaJpZM4P01e8 .

JakeIwen commented 7 years ago

@carlpeaslee what do you think about assigning a new sessionID to direct message conversations? or should I make another field directID?

My original plan was to leave it blank, but that doesn't look so smart now. I think the query would need some sort of SQL-style table join to grab a conversation between 2 people.

Giving every conversation a sessionID is the way I was thinking now.

JakeIwen commented 7 years ago

Ok looks like CreateSession generates a session ID based off of the two project Ids.

<ButtonWrapper
  title={`Start Session`}
  onClick={()=>{
    let projectsIds = []
    projectsIds.push(project.id)
    projectsIds.push(currentProject.id)
    this.props.relay.commitUpdate(
      new CreateSession({projectsIds}),{
        onSuccess: (success) => {
          let {id: sessionId} = success.createSession.session
          this.props.router.push(`/session/${this.props.viewer.user.handle}/${sessionId}/theirs`)
        }
      }
    )
  }}
>

Maybe it would be better to create a new mutation CreateDirect that uses user Ids to create a directId?

carlpeaslee commented 7 years ago

Wait, was there a reason why we don't just get all the messages between two people when we want to get their direct messages? Is their a problem with including the sesssion messages? Seems like people might expect those to be included in their dms? If not, you could just filter for all messages between two people that don't have sessionIds attached to them.

JakeIwen commented 7 years ago

Because relay doesn't seem to work with nested variables (filters).

JakeIwen commented 7 years ago

image Leads to this console error:

Relay transform error ``Unexpected nested variable `userHandle`; variables are supported as top-level arguments - `node(id: $id)` - or directly within lists - `nodes(ids: [$id])`.`` in file `/Users/jacobr/dev/bouncetribe/src/containers/DirectMessages.js`. Try updating your GraphQL schema if an argument/field/type was recently added.

If I replace $userHandle with a hardcoded string it works fine.

carlpeaslee commented 7 years ago

Hmm but what if you made the container query ‘allMessages” instead of ‘user’? On Tue, Oct 17, 2017 at 12:08 PM Jake Iwen notifications@github.com wrote:

[image: image] https://user-images.githubusercontent.com/20415982/31644871-53b6fdf2-b2be-11e7-962c-aae074cf5f09.png Leads to this console error: Uncaught Error: Relay transform error Unexpected nested variable userHandle; variables are supported as top-level arguments -node(id: $id)- or directly within lists -nodes(ids: [$id]). in file /Users/jacobr/dev/bouncetribe/src/containers/DirectMessages.js. Try updating your GraphQL schema if an argument/field/type was recently added.

If i replace $userHandle with a hardcoded string it works fine.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/BounceTribe/bouncetribe/issues/11#issuecomment-337104641, or mute the thread https://github.com/notifications/unsubscribe-auth/AOhs3muc2gpWPk13JT9S-nkBEHjLjQ_tks5stBodgaJpZM4P01e8 .

JakeIwen commented 7 years ago

That's what I did with the original screencap on this thread - same issue; still too deeply nested image

From what I can understand, we can't actually use filters with our query method because it involves a deeper level of nesting.

JakeIwen commented 7 years ago

is it possible to modify the schema to have a paramater called senderHandle?

allMessages (
  userHandle: $userHandle
  theirHandle: $theirHandle
) {
  text
  id
}

that would be only one level of nesting which should work

RPowerHimself commented 7 years ago

How would that be different than "sender"?

RPowerHimself commented 7 years ago

type Message @model { createdAt: DateTime! directId: Int @isUnique id: ID! @isUnique recipient: User @relation(name: "MessageOnUser1") sender: User @relation(name: "MessageOnUser") sessionParent: Session @relation(name: "MessageOnSession") text: String updatedAt: DateTime! }

This is what the Message Model looks like now.

JakeIwen commented 7 years ago

because that requires a nested variable in the query, filter -> sender -> handle which gives us this error:

``Unexpected nested variable `userHandle`; variables are supported as top-level arguments - `node(id: $id)` - or directly within lists - `nodes(ids: [$id])`.``

my above suggestion would be top-level

JakeIwen commented 7 years ago

image image I can query an individual message with an ID but using the filter part of allMessages() requires variable nesting, so the 'filter' argument is not useful with these queries

carlpeaslee commented 7 years ago

It’s possible that the new version of Relay allows you to make nested queries? I haven’t looked at the api yet. On Fri, Oct 20, 2017 at 7:07 AM Jake Iwen notifications@github.com wrote:

[image: image] https://user-images.githubusercontent.com/20415982/31819961-9eecdadc-b564-11e7-852f-2dd23cff41b0.png [image: image] https://user-images.githubusercontent.com/20415982/31820052-07b0cefc-b565-11e7-964a-58d4b37ca9f3.png I can query an individual message with an ID but using the filter part of allMessages() requires variable nesting, so the 'filter' argument is not useful with these queries

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/BounceTribe/bouncetribe/issues/11#issuecomment-338188037, or mute the thread https://github.com/notifications/unsubscribe-auth/AOhs3kTX_GJeme2vZzNE5tL9QyS7_oD4ks5suIzjgaJpZM4P01e8 .

JakeIwen commented 7 years ago

Its on my list to get that installed. Maybe prioritize getting Relay 4 figured out as routing is getting to be a bit messy. Here's my bandaid fix for the time being. Shouldn't be TOO heavy on data for now. It queries all received messages for user and User, then filters like so containers/DirectMessages.js image

JakeIwen commented 7 years ago

will follow example from $projectfilter