Return accessible page ids #106
Conversation
|
Code bc62252 labeled version 0.3.11-rc-filter-page-not.3 |
|
Running downstream job at https://github.com/charmverse/app.charmverse.io/actions/runs/5978369609 |
|
Running downstream job at https://github.com/charmverse/permissions.charmverse.io/actions/runs/5978369794 |
…e into filter-page-notifications
| @@ -28,6 +28,11 @@ export type PermissionCompute = { | |||
| userId?: string; | |||
| }; | |||
|
|
|||
| export type BulkPermissionCompute = { | |||
There was a problem hiding this comment.
(just a suggestion) When using typesafe language, one way to approach cit is to "think in types". As in, you should make it possible to understand the program just by looking at types. Then everything else is type algebra.
i know the point of 'resourceIds' is to be flexible for data models, but a type like this, with no 'resourceType' attached or nearby, makes it actually harder to grok in my head, because i don't know what types of resources it supports. I have to go read all the places it's used to unerstand how it relates to the other types. But then looking at the usage, it seems very specific to pageIds. I don't see the benefit of using a generic property name here? For example why not call it "BulkPagePermissionCompute"?
How would it protect us if in the future we wanted them to be bountyIds instead of pageIds?
There was a problem hiding this comment.
ok sorry end rant :) and this is not a "MUST" but "I would have"
There was a problem hiding this comment.
I get your point on this one, the abstract compute interfaces only really make sense in the context they are used in. This also allows method sharing (for example our policyBuilder which works across compute methods)
That said, I can see your point, bulkCompute really is a terminal method, so in this case, I'll go back and refactor to use page Ids
|
Running downstream job at https://github.com/charmverse/permissions.charmverse.io/actions/runs/6006179322 |
|
Running downstream job at https://github.com/charmverse/app.charmverse.io/actions/runs/6006180003 |
…e into filter-page-notifications
|
Running downstream job at https://github.com/charmverse/permissions.charmverse.io/actions/runs/6012359852 |
|
Running downstream job at https://github.com/charmverse/app.charmverse.io/actions/runs/6012360162 |
|
Running downstream job at https://github.com/charmverse/app.charmverse.io/actions/runs/6014507845 |
|
Running downstream job at https://github.com/charmverse/permissions.charmverse.io/actions/runs/6014507942 |
…e into filter-page-notifications
WHAT
copilot:summary
WHY
Extend permissions clients to list accessible pages