Pass user_id instead of the whole context to CouchersSelect methods

[WouldYouKindly]

There's no reason to pass the whole context object from the servicers layer

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Preview	Updated (UTC)
couchers	Ready	Preview	Nov 22, 2025 2:35pm

[aapeliv]

what is the downside of passing the whole context object?

The thinking here was that this context object was the abstraction that contains the information needed to figure out e.g. visibility. Down the line it could use some other contextual information too (though I understand right now it doesn't). (I think at some point I was thinking of just caching the hidden users in the context object during the call.) I mean if there is some downside to doing this, it can be simplified, but whether you access .user_id at the call site or in the function, I don't think there is a performance impact?

[WouldYouKindly]

what is the downside of passing the whole context object?

The thinking here was that this context object was the abstraction that contains the information needed to figure out e.g. visibility. Down the line it could use some other contextual information too (though I understand right now it doesn't). (I think at some point I was thinking of just caching the hidden users in the context object during the call.) I mean if there is some downside to doing this, it can be simplified, but whether you access .user_id at the call site or in the function, I don't think there is a performance impact?

You're right, there's no performance impact. My issue here is CouchersContext depends on GRPC (e.g., it has an abort method, it has cookies, tokens etc). So now our database code knows about our API layer. If we want to reuse the same database code in a worker that doesn't use GRPC, we have to fake the context, and then even though you have context.abort(), you can't use it.

If we want to have a context object, I would decouple it from grpc, at least on the interface level. Something like

class CouchersContext(Procotol):
   user_id: int
  ...

class GRPCCouchersContext(CouchersContext):
   user_id: int
   
  def abort(self): ...

def where_users_visible(self, context: CouchersContext) -> Self:
   context.user_id  # fine
   context.abort()  # type error, because the interface doesn't have abort()

I don't have a strong opinion here, but I think a bit of separation of layers (api, db, business logic) wouldn't be too bad. But if you think it's a waste of time, I'll be fine with closing this PR :)

[aapeliv]

I like the idea of separating the db/api/business layer. I think we should then separate the context into business and api contexts then? The context in this case is used as the “business context”, and I think that would be a clean abstraction.

What do you reckon?