Skip to content
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

Refactor users table and pagination #381

Open
wants to merge 2 commits into
base: filip-refactor-user-module
Choose a base branch
from
Open

Refactor users table and pagination #381

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a1e7514
Refactor UserTable and getPaginatedUsers query
sodic Feb 21, 2025
0543c9f
Remove reminder comment
sodic Feb 21, 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
2 changes: 1 addition & 1 deletion template/app/schema.prisma
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,4 +110,4 @@ model ContactFormMessage {
content String
isRead Boolean @default(false)
repliedAt DateTime?
}
}
114 changes: 60 additions & 54 deletions template/app/src/admin/dashboards/users/UsersTable.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,25 +12,28 @@ const AdminSwitch = ({ id, isAdmin }: Pick<User, 'id' | 'isAdmin'>) => {
};

const UsersTable = () => {
const [skip, setskip] = useState(0);
const [page, setPage] = useState(1);
const [email, setEmail] = useState<string | undefined>(undefined);
const [currentPage, setCurrentPage] = useState(1);
const [emailFilter, setEmailFilter] = useState<string>('');
Copy link
Collaborator

Choose a reason for hiding this comment

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

If we have undefined for isAdmin as empty state, I'd expect it to have for the emailFilter as well.

Below you are counting on the behavior when emailFilter is fasly anyways (emailFilter &&) - you could go all the way with explicit undefined.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Okay, I guess it's because of setEmailFilter(e.currentTarget.value);, okay this looks like it's more justified. But in the name of consistency, maybe you could go for:

if (e.currentTarget.value === '') {
setEmailFilter(undefined);
} else {
setEmailFilter(e.currentTarget.value);
}

const [isAdminFilter, setIsAdminFilter] = useState<boolean | undefined>(undefined);
const [statusOptions, setStatusOptions] = useState<SubscriptionStatus[]>([]);
const [subscriptionStatusFilter, setSubcriptionStatusFilter] = useState<SubscriptionStatus[]>([]);

const skipPages = currentPage - 1;

const { data, isLoading } = useQuery(getPaginatedUsers, {
skip,
emailContains: email,
isAdmin: isAdminFilter,
subscriptionStatus: statusOptions?.length > 0 ? statusOptions : undefined,
skipPages,
filter: {
...(emailFilter && { emailContains: emailFilter }),
...(isAdminFilter !== undefined && { isAdmin: isAdminFilter }),
...(subscriptionStatusFilter?.length > 0 && { subscriptionStatusIn: subscriptionStatusFilter }),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can subscriptionStatusFilter ever be undefined or null?

},
});

useEffect(() => {
setPage(1);
}, [email, statusOptions]);

useEffect(() => {
setskip((page - 1) * 10);
}, [page]);
useEffect(
function backToPageOne() {
setCurrentPage(1);
},
[emailFilter, subscriptionStatusFilter, isAdminFilter]
);
Comment on lines +31 to +36
Copy link
Collaborator

Choose a reason for hiding this comment

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

For some reason, this, to me, feels like an odd way of writing it (I'm not saying it's wrong), I'd write something like this:

  function backToPageOne() {
    setCurrentPage(1);
  },
  useEffect(() => {
    backToPageOne();
  }, [emailFilter, subscriptionStatusFilter, isAdminFilter]);

Copy link
Collaborator Author

@sodic sodic Feb 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I only learned about this trick recently. I like it because it allows you to name your inline useEffect function without forcing you to jump around with your eyes - you still read it sequentially, but can now tell what it's supposed to do.

Also, the anonymous useEffect arrow function that wraps backToPageOne can easily grow and include more logic, and then we're back at square one - an anonymous inline function

The only thing I don't like is the formatting. This is how I'd like it to look:

useEffect(function backToPageOne() {
  setCurrentPage(1);
}, [emailFilter, subscriptionStatusFilter, isAdminFilter]);

But prettier doesn't allow it.

Regardless of the formatting, I still like this trick above reasons. That said, if you think it's weird enough to throw people off, I'll change it.

Btw, this isn't supposed to be a useEffect anyway - we should be calling backToPageOne on each filter change, but I didn't have time to refactor that.


return (
<div className='flex flex-col gap-4'>
Expand All @@ -47,7 +50,7 @@ const UsersTable = () => {
id='email-filter'
placeholder='[email protected]'
onChange={(e) => {
setEmail(e.currentTarget.value);
setEmailFilter(e.currentTarget.value);
}}
className='rounded border border-stroke py-2 px-5 bg-white outline-none transition focus:border-primary active:border-primary disabled:cursor-default disabled:bg-whiter dark:border-form-strokedark dark:bg-form-input dark:focus:border-primary'
/>
Expand All @@ -56,8 +59,8 @@ const UsersTable = () => {
</label>
<div className='flex-grow relative z-20 rounded border border-stroke pr-8 outline-none bg-white transition focus:border-primary active:border-primary dark:border-form-strokedark dark:bg-form-input'>
<div className='flex items-center'>
{!!statusOptions && statusOptions.length > 0 ? (
statusOptions.map((opt) => (
{!!subscriptionStatusFilter && subscriptionStatusFilter.length > 0 ? (
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd drop the !!subscriptionStatusFilter here since it can't be undefined or null.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, this is old, the change is included because of formatting.

I'll fix it up.

subscriptionStatusFilter.map((opt) => (
<span
key={opt}
className='z-30 flex items-center my-1 mx-2 py-1 px-2 outline-none transition focus:border-primary active:border-primary disabled:cursor-default disabled:bg-whiter dark:border-form-strokedark dark:bg-form-input dark:focus:border-primary'
Expand All @@ -66,26 +69,13 @@ const UsersTable = () => {
<span
onClick={(e) => {
e.stopPropagation();
setStatusOptions((prevValue) => {
setSubcriptionStatusFilter((prevValue) => {
return prevValue?.filter((val) => val !== opt);
});
}}
className='z-30 cursor-pointer pl-2 hover:text-danger'
>
<svg
width='14'
height='14'
viewBox='0 0 12 12'
fill='none'
xmlns='http://www.w3.org/2000/svg'
>
<path
fillRule='evenodd'
clipRule='evenodd'
d='M9.35355 3.35355C9.54882 3.15829 9.54882 2.84171 9.35355 2.64645C9.15829 2.45118 8.84171 2.45118 8.64645 2.64645L6 5.29289L3.35355 2.64645C3.15829 2.45118 2.84171 2.45118 2.64645 2.64645C2.45118 2.84171 2.45118 3.15829 2.64645 3.35355L5.29289 6L2.64645 8.64645C2.45118 8.84171 2.45118 9.15829 2.64645 9.35355C2.84171 9.54882 3.15829 9.54882 3.35355 9.35355L6 6.70711L8.64645 9.35355C8.84171 9.54882 9.15829 9.54882 9.35355 9.35355C9.54882 9.15829 9.54882 8.84171 9.35355 8.64645L6.70711 6L9.35355 3.35355Z'
fill='currentColor'
></path>
</svg>
<XIcon />
</span>
</span>
))
Expand All @@ -98,7 +88,7 @@ const UsersTable = () => {
<select
onChange={(e) => {
const targetValue = e.target.value === '' ? null : e.target.value;
setStatusOptions((prevValue) => {
setSubcriptionStatusFilter((prevValue) => {
if (prevValue?.includes(targetValue as SubscriptionStatus)) {
return prevValue?.filter((val) => val !== targetValue);
} else if (!!prevValue) {
Expand All @@ -113,8 +103,8 @@ const UsersTable = () => {
className='absolute top-0 left-0 z-20 h-full w-full bg-white opacity-0'
>
<option value=''>Select filters</option>
{['past_due', 'canceled', 'active', 'deleted', null].map((status) => {
if (!statusOptions.includes(status as SubscriptionStatus)) {
{['past_due', 'cancel_at_period_end', 'active', 'deleted', null].map((status) => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think this was a mistake, the filtered returned nothing.

I'm guessing canceled is an old status.

if (!subscriptionStatusFilter.includes(status as SubscriptionStatus)) {
return (
<option key={status} value={status || ''}>
{status ? status : 'has not subscribed'}
Expand All @@ -124,22 +114,7 @@ const UsersTable = () => {
})}
</select>
<span className='absolute top-1/2 right-4 z-10 -translate-y-1/2'>
<svg
width='24'
height='24'
viewBox='0 0 24 24'
fill='none'
xmlns='http://www.w3.org/2000/svg'
>
<g opacity='0.8'>
<path
fillRule='evenodd'
clipRule='evenodd'
d='M5.29289 8.29289C5.68342 7.90237 6.31658 7.90237 6.70711 8.29289L12 13.5858L17.2929 8.29289C17.6834 7.90237 18.3166 7.90237 18.7071 8.29289C19.0976 8.68342 19.0976 9.31658 18.7071 9.70711L12.7071 15.7071C12.3166 16.0976 11.6834 16.0976 11.2929 15.7071L5.29289 9.70711C4.90237 9.31658 4.90237 8.68342 5.29289 8.29289Z'
fill='#637381'
></path>
</g>
</svg>
<ChevronDownIcon />
</span>
</div>
<div className='flex items-center gap-2'>
Expand Down Expand Up @@ -168,11 +143,14 @@ const UsersTable = () => {
<span className='text-md mr-2 text-black dark:text-white'>page</span>
<input
type='number'
value={page}
min={1}
defaultValue={currentPage}
max={data?.totalPages}
onChange={(e) => {
setPage(parseInt(e.currentTarget.value));
const value = parseInt(e.currentTarget.value);
if (data?.totalPages && value <= data?.totalPages && value > 0) {
setCurrentPage(value);
}
Comment on lines +150 to +153
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The old code had a bug and would break if you manually deleted the page number.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm guessing you mean "manually edited"?

}}
className='rounded-md border-1 border-stroke bg-transparent px-4 font-medium outline-none transition focus:border-primary active:border-primary dark:border-form-strokedark dark:bg-form-input dark:focus:border-primary'
/>
Expand Down Expand Up @@ -238,4 +216,32 @@ const UsersTable = () => {
);
};

function ChevronDownIcon() {
return (
<svg width='24' height='24' viewBox='0 0 24 24' fill='none' xmlns='http://www.w3.org/2000/svg'>
<g opacity='0.8'>
<path
fillRule='evenodd'
clipRule='evenodd'
d='M5.29289 8.29289C5.68342 7.90237 6.31658 7.90237 6.70711 8.29289L12 13.5858L17.2929 8.29289C17.6834 7.90237 18.3166 7.90237 18.7071 8.29289C19.0976 8.68342 19.0976 9.31658 18.7071 9.70711L12.7071 15.7071C12.3166 16.0976 11.6834 16.0976 11.2929 15.7071L5.29289 9.70711C4.90237 9.31658 4.90237 8.68342 5.29289 8.29289Z'
fill='#637381'
></path>
</g>
</svg>
);
}

function XIcon() {
return (
<svg width='14' height='14' viewBox='0 0 12 12' fill='none' xmlns='http://www.w3.org/2000/svg'>
<path
fillRule='evenodd'
clipRule='evenodd'
d='M9.35355 3.35355C9.54882 3.15829 9.54882 2.84171 9.35355 2.64645C9.15829 2.45118 8.84171 2.45118 8.64645 2.64645L6 5.29289L3.35355 2.64645C3.15829 2.45118 2.84171 2.45118 2.64645 2.64645C2.45118 2.84171 2.45118 3.15829 2.64645 3.35355L5.29289 6L2.64645 8.64645C2.45118 8.84171 2.45118 9.15829 2.64645 9.35355C2.84171 9.54882 3.15829 9.54882 3.35355 9.35355L6 6.70711L8.64645 9.35355C8.84171 9.54882 9.15829 9.54882 9.35355 9.35355C9.54882 9.15829 9.54882 8.84171 9.35355 8.64645L6.70711 6L9.35355 3.35355Z'
fill='currentColor'
></path>
</svg>
);
}

export default UsersTable;
90 changes: 37 additions & 53 deletions template/app/src/user/operations.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { type UpdateIsUserAdminById, type GetPaginatedUsers } from 'wasp/server/operations';
import { type User } from 'wasp/entities';
import { HttpError } from 'wasp/server';
import { HttpError, prisma } from 'wasp/server';
import { type SubscriptionStatus } from '../payment/plans';
import { type Prisma } from '@prisma/client';

export const updateIsUserAdminById: UpdateIsUserAdminById<Pick<User, 'id' | 'isAdmin'>, User> = async (
{ id, isAdmin },
Expand All @@ -22,11 +23,12 @@ export const updateIsUserAdminById: UpdateIsUserAdminById<Pick<User, 'id' | 'isA
};

type GetPaginatedUsersInput = {
skip: number;
cursor?: number | undefined;
emailContains?: string;
isAdmin?: boolean;
subscriptionStatus?: SubscriptionStatus[];
skipPages: number;
filter: {
emailContains?: string;
isAdmin?: boolean;
subscriptionStatusIn?: SubscriptionStatus[];
};
Comment on lines +26 to +31
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Separating the data (filter params) from the metadata (paging params).

};

type GetPaginatedUsersOutput = {
Expand All @@ -41,39 +43,44 @@ export const getPaginatedUsers: GetPaginatedUsers<GetPaginatedUsersInput, GetPag
args,
context
) => {
if (!context.user?.isAdmin) {
throw new HttpError(401);
if (!context.user) {
throw new HttpError(401, 'Only authenticated users are allowed to perform this operation');
}

const allSubscriptionStatusOptions = args.subscriptionStatus as Array<string | null> | undefined;
const hasNotSubscribed = allSubscriptionStatusOptions?.find((status) => status === null);
const subscriptionStatusStrings = allSubscriptionStatusOptions?.filter((status) => status !== null) as
| string[]
| undefined;
if (!context.user.isAdmin) {
throw new HttpError(403, 'Only admins are allowed to perform this operation');
}

const {
skipPages,
filter: { subscriptionStatusIn: subscriptionStatus, emailContains, isAdmin },
} = args;
const includeUnsubscribedUsers = !!subscriptionStatus?.some((status) => status === null);
const desiredSubscriptionStatuses = subscriptionStatus?.filter((status) => status !== null);

const queryResults = await context.entities.User.findMany({
skip: args.skip,
take: 10,
const pageSize = 10;

const userPageQuery: Prisma.UserFindManyArgs = {
skip: skipPages * pageSize,
take: pageSize,
where: {
AND: [
{
email: {
contains: args.emailContains || undefined,
contains: emailContains,
mode: 'insensitive',
},
isAdmin: args.isAdmin,
isAdmin,
},
{
OR: [
{
subscriptionStatus: {
in: subscriptionStatusStrings,
in: desiredSubscriptionStatuses,
},
},
{
subscriptionStatus: {
equals: hasNotSubscribed,
},
subscriptionStatus: includeUnsubscribedUsers ? null : undefined,
},
],
},
Expand All @@ -88,41 +95,18 @@ export const getPaginatedUsers: GetPaginatedUsers<GetPaginatedUsersInput, GetPag
paymentProcessorUserId: true,
},
orderBy: {
id: 'desc',
username: 'asc',
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sorting seems less random because users are displayed in the table.

},
});
};

const totalUserCount = await context.entities.User.count({
where: {
AND: [
{
email: {
contains: args.emailContains || undefined,
mode: 'insensitive',
},
isAdmin: args.isAdmin,
},
{
OR: [
{
subscriptionStatus: {
in: subscriptionStatusStrings,
},
},
{
subscriptionStatus: {
equals: hasNotSubscribed,
},
},
],
},
],
},
});
const totalPages = Math.ceil(totalUserCount / 10);
const [pageOfUsers, totalUsers] = await prisma.$transaction([
context.entities.User.findMany(userPageQuery),
context.entities.User.count({ where: userPageQuery.where }),
]);
const totalPages = Math.ceil(totalUsers / pageSize);

return {
users: queryResults,
users: pageOfUsers,
totalPages,
};
};