Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
22 changes: 17 additions & 5 deletions api/main_endpoints/routes/User.js
Original file line number Diff line number Diff line change
Expand Up @@ -143,12 +143,24 @@ router.post('/users', async function(req, res) {
};
const sortOrder = orderToInteger[req.query.order] || orderToInteger.default;

// make sure that the page we want to see is 0 by default
// and avoid negative page numbers
let skip = Math.max(Number(req.body.page) || 0, 0);
skip *= ROWS_PER_PAGE;
// Handle pagination: defaults to page 0 if not specified
// Special case: page -1 fetches all users without pagination
// All other negative page numbers are clamped to 0
let skip, limit;
const pageNum = Number(req.body.page);

if (pageNum === -1) {
// Fetch all users (no pagination)
skip = 0;
limit = 0; // MongoDB uses 0 to mean "no limit"
} else {
Comment on lines +152 to +156
Copy link
Collaborator

Choose a reason for hiding this comment

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

is there a situation where we want to fetch every user ever

the pagination approach we have seems fine to me

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 rationale for this was that the initial load fetches all users and the client will handle filtering. I think the other approach to get the ux I want for search would be to debounce api calls as user types query but filtering on client seemed more appropriate.

Copy link
Collaborator

Choose a reason for hiding this comment

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

imagine its 3000+ rows to sort on, every time the user changes their input, thats a lot to ask for the browser when the backend is attached to mongodb

can we move this to the backend, and keep the pagination approach, we only ever need to return like the first 25 records

// Regular pagination: clamp negative pages to 0
skip = Math.max(pageNum || 0, 0) * ROWS_PER_PAGE;
limit = ROWS_PER_PAGE;
}

const total = await User.count(maybeOr);
User.find(maybeOr, { password: 0, }, { skip, limit: ROWS_PER_PAGE, })
User.find(maybeOr, { password: 0, }, { skip, limit })
.sort({ [sortColumn] : sortOrder })
.then(items => {
res.status(OK).send({ items, total, rowsPerPage: ROWS_PER_PAGE, });
Expand Down
223 changes: 122 additions & 101 deletions src/Pages/Overview/Overview.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,11 @@ export default function Overview() {
const { user } = useSCE();
const [toggleDelete, setToggleDelete] = useState(false);
const [loading, setLoading] = useState(false);
const [paginationText, setPaginationText] = useState('');
const [users, setUsers] = useState([]);
const [allUsers, setAllUsers] = useState([]);
const [filteredUsers, setFilteredUsers] = useState([]);
const [page, setPage] = useState(0);
const [total, setTotal] = useState(0);
const [userToDelete, setUserToDelete] = useState({});
const [queryResult, setQueryResult] = useState([]);
const [rowsPerPage, setRowsPerPage] = useState(0);
const [query, setQuery] = useState('');
const [currentSortColumn, setCurrentSortColumn] = useState('joinDate');
const [currentSortOrder, setCurrentSortOrder] = useState('desc');
Expand All @@ -29,56 +27,47 @@ export default function Overview() {
// const [currentQueryType, setCurrentQueryType] = useState('All');
// const queryTypes = ['All', 'Pending', 'Officer', 'Admin'];

async function callDatabase() {
setLoading(true);
const apiResponse = await getAllUsers({
token: user.token,
query: '', // Filter on client, not server
page: -1, // Special value to fetch all users
sortColumn: 'joinDate',
sortOrder: 'desc'
});
if (!apiResponse.error) {
setAllUsers(apiResponse.responseData.items);
setTotal(apiResponse.responseData.total);
}
setLoading(false);
}

async function deleteUser(userToDel) {
const response = await deleteUserByID(
userToDel._id,
user.token
);
if (response.error) {
alert('unable to delete user, check logs');
return;
}
if (userToDel._id === user._id) {
// logout
window.localStorage.removeItem('jwtToken');
window.location.reload();
return window.alert('Self-deprecation is an art');
}
setUsers(
users.filter(
child => !child._id.includes(userToDel._id)
)
);
setTotal(total - 1);
setQueryResult(
queryResult.filter(
child => !child._id.includes(userToDel._id)
)
);

// Refetch all users after deletion
await callDatabase();
// The filtering useEffect will automatically reapply current search
}

function mark(bool) {
return bool ? svg.checkMark() : svg.xMark();
}

async function callDatabase() {
setLoading(true);
const sortColumn = currentSortOrder === 'none' ? 'joinDate' : currentSortColumn;
const sortOrder = currentSortOrder === 'none' ? 'desc' : currentSortOrder;
const apiResponse = await getAllUsers({
token: user.token,
query: query,
page: page,
sortColumn: sortColumn,
sortOrder: sortOrder
});
if (!apiResponse.error) {
setUsers(apiResponse.responseData.items);
setTotal(apiResponse.responseData.total);
setRowsPerPage(apiResponse.responseData.rowsPerPage);
}
setLoading(false);
}

async function getClubRevenueData() {
const response = await getNewPaidMembersThisSemester(user.token);
if(!response.error) {
Expand All @@ -89,25 +78,61 @@ export default function Overview() {
useEffect(() => {
callDatabase();
getClubRevenueData();
}, [page, currentSortColumn, currentSortOrder]);
}, []);

// Client-side filtering and sorting
useEffect(() => {
if (!allUsers.length) {
setFilteredUsers([]);
return;
}

const amountOfUsersOnCurrentPage = Math.min((page + 1) * rowsPerPage, users.length);
const pageOffset = page * rowsPerPage;
const startingElementNumber = (page * rowsPerPage) + 1;
const endingElementNumber = amountOfUsersOnCurrentPage + pageOffset;
setPaginationText(
<>
<p className='md:hidden text-gray-700 dark:text-white'>
{startingElementNumber} - {endingElementNumber} / {total}
</p>
<p className="hidden md:inline-block text-gray-700 dark:text-white">
Showing <span className='font-medium'>{startingElementNumber}</span> to <span className='font-medium'>{endingElementNumber}</span> of <span className='font-medium'>{total}</span> results
</p>
</>
);
}, [page, rowsPerPage, users, total]);
// Filter users based on query
let filtered = allUsers;
if (query.trim()) {
const searchTerm = query.trim().toLowerCase();
filtered = allUsers.filter(user => {
return (
user.firstName?.toLowerCase().includes(searchTerm) ||
user.lastName?.toLowerCase().includes(searchTerm) ||
user.email?.toLowerCase().includes(searchTerm)
);
Comment on lines +91 to +99
Copy link
Collaborator

Choose a reason for hiding this comment

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

why not have the backend do this filtering? thats what wee did before this pr

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, but I filter here on client so I can do a live search result update as user edits query, rather than making an API after a user clicks enter each time.

});
}

// Sort filtered results
if (currentSortOrder !== 'none') {
filtered = [...filtered].sort((a, b) => {
const aVal = a[currentSortColumn];
const bVal = b[currentSortColumn];

// Handle null/undefined
if (aVal == null && bVal == null) return 0;
if (aVal == null) return 1;
if (bVal == null) return -1;

// Compare based on type
let comparison = 0;
if (typeof aVal === 'string') {
comparison = aVal.localeCompare(bVal);
} else if (typeof aVal === 'number') {
comparison = aVal - bVal;
} else {
// Handle dates
const dateA = new Date(aVal);
const dateB = new Date(bVal);
comparison = dateA.getTime() - dateB.getTime();
}

return currentSortOrder === 'asc' ? comparison : -comparison;
});
}
Comment on lines +104 to +129
Copy link
Collaborator

Choose a reason for hiding this comment

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

100% the job of the backend, not here


setFilteredUsers(filtered);
setTotal(filtered.length);
setPage(0);

}, [allUsers, query, currentSortColumn, currentSortOrder]);

function handleSortUsers(columnName) {
if (columnName === null) {
Expand Down Expand Up @@ -179,37 +204,48 @@ export default function Overview() {
// }

function maybeRenderPagination() {
const amountOfUsersOnCurrentPage = Math.min((page + 1) * rowsPerPage, users.length);
const pageOffset = page * rowsPerPage;
const endingElementNumber = amountOfUsersOnCurrentPage + pageOffset;
if (users.length) {
return (
<nav className='flex justify-start py-6'>
<div className='flex items-center navbar-start'>
<span className="text-gray-700 dark:text-white">
{loading ? '...' : paginationText}
</span>
</div>
<div className='flex justify-end space-x-3 navbar-end'>
<button
className='btn btn-neutral text-gray-800 bg-gray-500 hover:bg-gray-300 dark:text-white dark:bg-gray-700 dark:hover:bg-gray-600'
onClick={() => setPage(page - 1)}
disabled={page === 0 || loading}
>
previous
</button>
<button
className='btn btn-neutral text-gray-800 bg-gray-200 hover:bg-gray-300 dark:text-white dark:bg-gray-700 dark:hover:bg-gray-600'
onClick={() => setPage(page + 1)}
disabled={endingElementNumber >= total || loading}
>
next
</button>
</div>
</nav>
);
}
return <></>;
const ROWS_PER_PAGE = 20;
const startIdx = page * ROWS_PER_PAGE;
const endIdx = Math.min(startIdx + ROWS_PER_PAGE, total);

if (filteredUsers.length === 0) return <></>;

return (
<nav className='flex justify-start py-6'>
<div className='flex items-center navbar-start'>
<span className="text-gray-700 dark:text-white">
{loading ? '...' : (
<>
<p className='md:hidden text-gray-700 dark:text-white'>
{startIdx + 1} - {endIdx} / {total}
</p>
<p className="hidden md:inline-block text-gray-700 dark:text-white">
Showing <span className='font-medium'>{startIdx + 1}</span> to{' '}
<span className='font-medium'>{endIdx}</span> of{' '}
<span className='font-medium'>{total}</span> results
</p>
</>
)}
</span>
</div>
<div className='flex justify-end space-x-3 navbar-end'>
<button
className='btn btn-neutral text-gray-800 bg-gray-500 hover:bg-gray-300 dark:text-white dark:bg-gray-700 dark:hover:bg-gray-600'
onClick={() => setPage(page - 1)}
disabled={page === 0 || loading}
>
previous
</button>
<button
className='btn btn-neutral text-gray-800 bg-gray-200 hover:bg-gray-300 dark:text-white dark:bg-gray-700 dark:hover:bg-gray-600'
onClick={() => setPage(page + 1)}
disabled={endIdx >= total || loading}
>
next
</button>
</div>
</nav>
);
}

return (
Expand Down Expand Up @@ -254,29 +290,14 @@ export default function Overview() {
<div className='py-6'>
<label className="w-full form-control">
<div className="label">
<span className="label-text text-md text-gray-700 dark:text-white">Type a search, followed by the enter key</span>
<span className="label-text text-md text-gray-700 dark:text-white">Type a search</span>
</div>
<input
className="w-full text-sm input input-bordered text-gray-900 dark:text-white sm:text-base"
type="text"
placeholder="search by first name, last name, or email"
onKeyDown={(event) => {
if (event.key === 'Enter') {
// instead of calling the backend directory, set
// the page we are on to zero if the current page
// we are on isn't the first page (value of 0).
// by doing this, the useEffect will call the backend
// for us with the correct page and query.
if (page) {
setPage(0);
} else {
callDatabase();
}
}
}}
onChange={event => {
setQuery(event.target.value);
}}
value={query}
onChange={event => setQuery(event.target.value)}
/>
</label>
</div>
Expand Down Expand Up @@ -308,7 +329,7 @@ export default function Overview() {
</tr>
</thead>
<tbody>
{users.map((user) => (
{filteredUsers.slice(page * 20, (page + 1) * 20).map((user) => (
<tr className='break-all !rounded md:break-keep hover:bg-gray-100 dark:hover:bg-white/10' key={user.email}>
<td className=''>
<a className='link link-hover text-blue-600 dark:text-blue-400' target="_blank" rel="noopener noreferrer" href={`/user/edit/${user._id}`}>
Expand Down Expand Up @@ -349,7 +370,7 @@ export default function Overview() {

</tbody>
</table>
{users.length === 0 && (
{filteredUsers.length === 0 && (
<div className='flex flex-row w-100 justify-center'>
<p className='text-lg text-gray-700 dark:text-white/70 mt-5 mb-5'>No results found!</p>
</div>
Expand Down