8000 fix: squad post moderation list not updating by AmarTrebinjac · Pull Request #4458 · dailydotdev/apps · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

fix: squad post moderation list not updating #4458

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
8000
from
+44 −16
Open

fix: squad post moderation list not updating #4458

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9eca54d
fix: update list when approving or rejecting moderation item
AmarTrebinjac Apr 28, 2025
9632b9b
refactor: create a different key for all squads
AmarTrebinjac Apr 29, 2025
dcf2b7c
Merge branch 'main' into MI-825
AmarTrebinjac Apr 29, 2025
9fd663d
Merge branch 'main' into MI-825
AmarTrebinjac May 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions packages/shared/src/components/squads/SquadTabs.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { webappUrl } from '../../lib/constants';
import { SourcePermissions } from '../../graphql/sources';
import { verifyPermission } from '../../graphql/squads';
import { useSquad } from '../../hooks';
import { useSquadPendingPosts } from '../../hooks/squads/useSquadPendingPosts';

export enum SquadTab {
Settings = 'Settings',
Expand All @@ -20,10 +21,13 @@ export function SquadTabs({ active, handle }: SquadTabsProps): ReactElement {
const { squad } = useSquad({
handle,
});
const { count } = useSquadPendingPosts({
squadId: squad?.id,
});
const isModerator = verifyPermission(squad, SourcePermissions.ModeratePost);
const squadLink = `${webappUrl}squads/${handle}`;
const pendingTabLabel = squad?.moderationPostCount
? `${SquadTab.PendingPosts} (${squad?.moderationPostCount})`
const pendingTabLabel = count
? `${SquadTab.PendingPosts} (${count})`
: SquadTab.PendingPosts;

const links = [
Expand Down
45 changes: 37 additions & 8 deletions packages/shared/src/hooks/squads/useSourceModerationList.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,19 +99,29 @@ export const useSourceModerationList = ({
const { logEvent } = useLogContext();
const { user } = useAuthContext();
const queryClient = useQueryClient();
const listQueryKey = generateQueryKey(
const currentListKey = generateQueryKey(
RequestKey.SquadPostRequests,
user,
squad?.id,
);
Comment on lines -102 to 106
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could use only one using the squad.id as discriminant field. Basically we have the squad list that has a specific id and the "all" list where this id is undefined.
Have we tried to pass something like: squad?.id ?? "all" instead?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think the keys used are not matching with one another. It would be great if we continue the pattern Luca suggested above.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't, because when the user is doing moderation for a single squad in for example /moderation/my-favorite-squad, when they approve/reject a post, the sidebar will not be updated because it uses a different key than the list that is being updated.

Remember, we are manually updating these in this component for optimistic updates. We need to access the keys specifically.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case, we will need to update two keys here. One for the specific squad and one for all.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can also just invalidate [RequestKey.SquadPostRequests, user?.id] that will invalidate both

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we do prefer to invalidate and refetch over manually updating then that is a possibility, yes

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was thinking Lee is talking about invalidation here, my bad, if you want you can try new hooks for updating that I created after our gathering talks (they are merged already), WIP docs are https://dailydotdev.atlassian.net/wiki/spaces/HAN/pages/1556185091/Data+fetching+mutations+and+optimistic+updates+with+typed+queries

Copy link
Member
@sshanzel sshanzel May 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, invalidation works, but the point about the query keys is my main concern. The issue is that there is a mismatch with the keys used across places, which makes the invalidation of one or the other not reflect the 100% correct data. So even if we invalidate or setData, the root cause is still there as you might not hit the right cache for the related areas.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From my testing, with this setup, it does indeed reflect the correct key across all places both when moderation all squads and a single one:

  • Sidebar
  • Squad header
  • Moderation approve all posts button
  • Moderation tab button

const squadQueryKey = generateQueryKey(RequestKey.Squad, user, squad?.handle);
const multiSquadKey = generateQueryKey(
RequestKey.SquadPostRequests,
user,
undefined,
);
const squadQueryKey = generateQueryKey(RequestKey.Squad, user, squad?.id);

const handleOptimistic = useCallback(
(data: SquadPostModerationProps) => {
const currentData =
queryClient.getQueryData<
InfiniteData<Connection<SourcePostModeration>>
>(listQueryKey);
>(currentListKey);

const multiSquadData =
queryClient.getQueryData<
InfiniteData<Connection<SourcePostModeration>>
>(multiSquadKey);

const currentSquad = queryClient.getQueryData<Squad | null>(
squadQueryKey,
Expand All @@ -127,7 +137,25 @@ export const useSourceModerationList = ({
}

queryClient.setQueryData<InfiniteData<Connection<SourcePostModeration>>>(
listQueryKey,
multiSquadKey,
(oldData) => {
if (!oldData) {
return oldData;
}
return {
...oldData,
pages: oldData.pages.map((page) => ({
...page,
edges: page.edges.filter(
(edge) => !data.postIds.includes(edge.node.id),
),
})),
};
},
);

queryClient.setQueryData<InfiniteData<Connection<SourcePostModeration>>>(
currentListKey,
(oldData) => {
if (!oldData) {
return oldData;
Expand All @@ -144,9 +172,9 @@ export const useSourceModerationList = ({
},
);

return { currentData, currentSquad };
return { currentData, currentSquad, multiSquadData };
},
[queryClient, listQueryKey, squadQueryKey],
[queryClient, currentListKey, squadQueryKey, multiSquadKey],
);

const {
Expand All @@ -170,8 +198,9 @@ export const useSourceModerationList = ({
);
return;
}
queryClient.setQueryData(listQueryKey, context?.currentData);
queryClient.setQueryData(currentListKey, context?.currentData);
queryClient.setQueryData(squadQueryKey, context?.currentSquad);
queryClient.setQueryData(multiSquadKey, context?.multiSquadData);
displayToast('Failed to approve post(s)');
},
});
Expand Down Expand Up @@ -212,7 +241,7 @@ export const useSourceModerationList = ({
});
},
onError: (_, __, context) => {
queryClient.setQueryData(listQueryKey, context.currentData);
queryClient.setQueryData(currentListKey, context.currentData);
queryClient.setQueryData(squadQueryKey, context.currentSquad);
},
});
Expand Down
7 changes: 1 addition & 6 deletions packages/shared/src/hooks/squads/useSquadPendingPosts.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,7 @@ export const useSquadPendingPosts = ({
}, [squads]);

const query = useInfiniteQuery<Connection<SourcePostModeration[]>>({
queryKey: generateQueryKey(
RequestKey.SquadPostRequests,
user,
squadId ?? 'all',
status,
),
queryKey: generateQueryKey(RequestKey.SquadPostRequests, user, squadId),
Copy link
Member
@sshanzel sshanzel Apr 29, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is ideal to include the params we send as part of the query key. Let's keep the status.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think including status requires us to pass it through a lot of places where we don't currently. It works well without it, so I think its better to leave it out for now and rather implement it when needed.

Copy link
Member
@sshanzel sshanzel May 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to have it on all places. Otherwise, we will just have issues when we try to toggle between statuses. It is always best to have them. If we want them to ensure consistency, we can introduce a generic function - but every param should be part of the query key.

queryFn: async ({ pageParam }) => {
return gqlClient
.request<{
Expand Down
0