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.

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

Fix pagination #5605 #5607

Merged
merged 10 commits into from
Apr 6, 2022
Merged

Fix pagination #5605 #5607

merged 10 commits into from
Apr 6, 2022

Conversation

kinjalravel
Copy link

@kinjalravel kinjalravel commented Apr 1, 2022

Summary

Change page size, fixed second page empty issue, fixed users table sort issue and sort users and location data from server

Checklist

  • Pre-push checks pass npm run check
    • Linter passing via npm run lint
    • Unit & Integration tests passing via npm run test:packages
    • Docker build process passing via npm run build-client
  • If this PR is still a WIP, convert to a draft
  • When this PR is ready, mark it as "Ready for review"
  • Changes have been manually QA'd
  • Changes reviewed by at least 2 approved reviewers

References

closes #5605

QA Steps

  1. git checkout pr_branch_name
  2. npm install
  3. npm run dev-reinit
  4. npm run dev

List any additional steps required to QA the changes of this PR, as well as any supplemental images or videos.

Reviewers

Reviewers for this PR

…rt issue and sort users and location data from server #5605
@zulqarnainhanif zulqarnainhanif self-requested a review April 1, 2022 10:31
@zulqarnainhanif
Copy link
Contributor

@kinjalravel as far as I understood the 4th point of the issue; We need sorting on multiple fields and every table of admin panel. Currently it's only done for name field of User and Location Table.

@HexaField Please let us know if my understanding is correct or not.

@kinjalravel
Copy link
Author

@kinjalravel as far as I understood the 4th point of the issue; We need sorting on multiple fields and every table of admin panel. Currently it's only done for name field of User and Location Table.

@HexaField Please let us know if my understanding is correct or not.

@HexaField ?

@HexaField
Copy link
Member

@kinjalravel as far as I understood the 4th point of the issue; We need sorting on multiple fields and every table of admin panel. Currently it's only done for name field of User and Location Table.
@HexaField Please let us know if my understanding is correct or not.

@HexaField ?

no, this is referring to the fact that at least one of the tables is sorting the data it has been given from the server, rather than sending the feathers database query information for the pagination

@@ -19,6 +19,10 @@ interface Props {
page: number
rowsPerPage: number
count: number
orderby?: string
Copy link
Contributor

Choose a reason for hiding this comment

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

proper camel case would be orderBy

orderby?: string
allowSort?: boolean
setSortField?: (fueld: string) => void
setOrderby?: (order: string) => void
Copy link
Contributor

Choose a reason for hiding this comment

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

proper camel case would be setOrderBy

@@ -143,7 +161,7 @@ const TableComponent = (props: Props) => {
columns={column}
/>
<TableBody>
{stableSort(rows, getComparator(order, orderBy))
{(allowSort === false ? rows : stableSort(rows, getComparator(order, orderBy)))
Copy link
Contributor

Choose a reason for hiding this comment

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

allowSort ? stableSort(rows, getComparator(order, orderBy) : rows

LocationService.fetchAdminLocations('increment', search)
// }
//if (user?.id?.value && adminLocationState.updateNeeded.value && !adminScopeReadErrMsg?.value) {
// LocationService.fetchAdminLocations(search, 0, orderby)
Copy link
Contributor

Choose a reason for hiding this comment

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

remove commented code if it isn't required anymore.

}
UserService.fetchUsersAsAdmin('increment', search)
//if (user?.id.value && adminUserState.updateNeeded.value) {
// UserService.fetchUsersAsAdmin(search, 0, orderby)
Copy link
Contributor

Choose a reason for hiding this comment

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

remove commented code if it isn't required anymore.

}
PartyService.fetchAdminParty('increment', search)
//if (user?.id?.value && adminParty.updateNeeded.value) {
//PartyService.fetchAdminParty('increment', null)
Copy link
Contributor

Choose a reason for hiding this comment

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

remove commented code if it isn't required anymore.

@@ -36,15 +36,23 @@ const AvatarTable = (props: Props) => {
const [popConfirmOpen, setPopConfirmOpen] = useState(false)
const [avatarId, setAvatarId] = useState('')
const [avatarName, setAvatarName] = useState('')
const [orderby, setOrderby] = useState('asc')
Copy link
Contributor

Choose a reason for hiding this comment

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

fix camelcase here as well

@@ -24,18 +24,26 @@ const GroupTable = (props: Props) => {
const [rowsPerPage, setRowsPerPage] = useState(GROUP_PAGE_LIMIT)
const [groupId, setGroupId] = useState('')
const [groupName, setGroupName] = useState('')
const [orderby, setOrderby] = useState('asc')
Copy link
Contributor

Choose a reason for hiding this comment

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

fix camelcase here as well

const [popConfirmOpen, setPopConfirmOpen] = useState(false)
const [instanceId, setInstanceId] = useState('')
const [instanceName, setInstanceName] = useState('')
const [orderby, setOrderby] = useState('asc')
Copy link
Contributor

Choose a reason for hiding this comment

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

fix camelcase here as well

@@ -42,22 +44,28 @@ const LocationTable = (props: LocationProps) => {
// Call custom hooks
const { t } = useTranslation()
const adminUserState = useUserState()
useFetchLocation(user, adminLocationState, adminScopeReadErrMsg, search, LocationService)
useFetchLocation(user, adminLocationState, adminScopeReadErrMsg, search, LocationService, sortField, orderby)
Copy link
Contributor

Choose a reason for hiding this comment

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

this line is failing pipeline due to extra params in functions

@@ -19,6 +19,8 @@ const UserTable = (props: UserProps) => {
const [popConfirmOpen, setPopConfirmOpen] = useState(false)
const [userId, setUserId] = useState('')
const [userName, setUserName] = useState('')
const [orderby, setOrderby] = useState('asc')
Copy link
Contributor

Choose a reason for hiding this comment

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

fix camelcase here as well

@@ -20,6 +20,8 @@ const PartyTable = (props: PartyPropsTable) => {
const [popConfirmOpen, setPopConfirmOpen] = useState(false)
const [partyName, setPartyName] = useState('')
const [partyId, setPartyId] = useState('')
const [orderby, setOrderby] = useState('asc')
Copy link
Contributor

Choose a reason for hiding this comment

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

fix camelcase here as well

const [popConfirmOpen, setPopConfirmOpen] = useState(false)
const [locationId, setLocationId] = useState('')
const [locationName, setLocationName] = useState('')
const [orderby, setOrderby] = useState('asc')
Copy link
Contributor

Choose a reason for hiding this comment

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

fix camelcase here as well

@zulqarnainhanif
Copy link
Contributor

zulqarnainhanif commented Apr 6, 2022

getting this error on Sorting Location Table Type column

similar is happening with instanceMediaChatEnabled and videoEnabled field

Screenshot 2022-04-06 at 1 34 25 PM

@zulqarnainhanif zulqarnainhanif merged commit 081691b into dev Apr 6, 2022
@zulqarnainhanif zulqarnainhanif deleted the #5605 branch April 6, 2022 16:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix pagination
3 participants