feat: multi squad moderation #2617
Conversation
AmarTrebinjac
requested review from
rebelchris,
omBratteng,
ilasw,
sshanzel and
nensidosari
| @@ -819,7 +821,7 @@ export const typeDefs = /* GraphQL */ ` | |||
| """ | |||
| Id of the source | |||
| """ | |||
| sourceId: ID! | |||
| sourceId: ID | |||
There was a problem hiding this comment.
In what case do you not send it?
There was a problem hiding this comment.
I answered it here
There was a problem hiding this comment.
I guess when you want to ask for all moderations.
There was a problem hiding this comment.
Personally, I'd also go for a separate query if it would make things much simpler.
src/schema/posts.ts
Outdated
| if (args?.sourceId) { | ||
| const isModerator = await isPrivilegedMember(ctx, args.sourceId); | ||
| if (isModerator) { | ||
| return graphorm.queryPaginated( |
There was a problem hiding this comment.
What exactly is this for?
Not full following this addition here?
There was a problem hiding this comment.
If sourceId is provided, we continue as before. If not, we fetch all the items the current user is allowed to moderate for the shared moderation page
There was a problem hiding this comment.
Thinking maybe we want a new mutation for multi moderation, its a bit hard to follow what is going on and I would bet we again missed some branch of the logic?
There was a problem hiding this comment.
It's all made from the same page on the frontend. Rather than creating multiple endpoints and having to deal with that on the frontend as well, how about maybe abstracting the logic instead?
There was a problem hiding this comment.
I think main issue is 3 big queries with queryPaginated, they share some params but its hard to see what and where so if those can be extracted it would help.
| @@ -1351,7 +1351,7 @@ describe('storeNotificationBundle', () => { | |||
| ); | |||
| expect(actual.notification.description).toEqual('Lacks value.'); | |||
| expect(actual.notification.targetUrl).toEqual( | |||
| 'http://localhost:5002/squads/a/moderate', | |||
There was a problem hiding this comment.
did we add a redirect on frontend for this since some old notifications might be wrong?
There was a problem hiding this comment.
Yeah, we might have to add it.
There was a problem hiding this comment.
@AmarTrebinjac just a reminder if you did not add it already 🙏 if yes ignore
|
@capJavert I've made some changes, take a look. I managed to solve the count stuff on the frontend without needing to update boot :) |
src/schema/posts.ts
Outdated
| Source to moderate the post in | ||
| """ | ||
| sourceId: ID! | ||
| """ |
There was a problem hiding this comment.
This is breaking change so we can't really merge before frontend?
There was a problem hiding this comment.
Also please check extension is not using this query since for it we have to keep backward compatibility.
There was a problem hiding this comment.
@capJavert This is not an extension page, so should be fine
The plan is to merge both frontend and backend at the same time
There was a problem hiding this comment.
Ok
The plan is to merge both frontend and backend at the same time
Yeah this is not ideal because if we have to revert it will take more time, but let's keep it in mind for the next time, we should always be backward compatible, thats why sometimes its better to add separate query.
All in all, I recommend we merge API first and then frontend (since that one is faster to deploy).
Enables a moderator to view moderation items from multiple squads at once, as well as bulk approve for multiple squads at once.