implement pagination on VASP List #988

masskoder · 2023-03-27T15:35:04Z

Scope of changes

feat: implement pagination

Type of change

bug fix
new feature
documentation
other (describe)

Acceptance criteria

https://www.awesomescreenshot.com/video/15983926?key=520bc0334afaaa0566d399dca5c9c2db

Author checklist

I have manually tested the change and/or added automation in the form of unit tests or integration tests
I have updated the dependencies list
I have recompiled and included new protocol buffers to reflect changes I made
I have added new test fixtures as needed to support added tests
Check this box if a reviewer can merge this pull request after approval (leave it unchecked if you want to do it yourself)
I have moved the associated Shortcut story to "Ready for Review"

Reviewer(s) checklist

Any new user-facing content that has been added for this PR has been QA'ed to ensure correct grammar, spelling, and understandability.

elysee15 · 2023-03-27T21:23:32Z

web/gds-user-ui/src/modules/dashboard/organization/useOrganizationListQuery.tsx

@@ -13,7 +13,7 @@ export function useOrganizationListQuery(): OrganizationQuery {

  return {
    getAllOrganizations: query.refetch,
-    organizations: query.data?.data as Organization[],
+    organizations: query.data?.data as any,


I think it is a bad idea to add any here because it could make debugging harder. What if you create a type based on what you get from backend ?

elysee15 · 2023-03-27T21:24:34Z

web/gds-user-ui/src/modules/dashboard/organization/organizationType.ts

-    errorMessage?: any;
+  getAllOrganizations(): void;
+  reset?(): void;
+  organizations?: any;


Would you mind adding the right type instead of any ?

Sure thanks. But s story is planned to migrate all the any type definitions. it has been added when the backend is not ready yet and was the best way to go quickly.

elysee15 · 2023-03-27T21:24:58Z

web/gds-user-ui/src/modules/dashboard/organization/organizationType.ts

+  refreshToken?: string;
+}
+
+export interface organizationResponse {


Suggested change

export interface organizationResponse {

export interface OrganizationResponse {

good catch, we need to handle that with Eslint. i will create a story to check that.

elysee15 · 2023-03-27T21:26:06Z

web/gds-user-ui/src/modules/dashboard/organization/organizationService.ts

-
-export const GetAllOrganisations = async () => {
-  const response = await axiosInstance.get(`/organizations`);
+export const getAllOrganisations = async (page?: number) => {


Suggested change

export const getAllOrganisations = async (page?: number) => {

export const getAllOrganisations = async (page = 1) => {

elysee15 · 2023-03-27T21:27:29Z

web/gds-user-ui/src/modules/dashboard/organization/organizationService.ts

-export const GetAllOrganisations = async () => {
-  const response = await axiosInstance.get(`/organizations`);
+export const getAllOrganisations = async (page?: number) => {
+  const currentPage = page || 1;


You don't need anymore to add a default number.

https://github.com/ryanmcdermott/clean-code-javascript#use-default-parameters-instead-of-short-circuiting-or-conditionals

elysee15 · 2023-03-27T21:34:29Z

web/gds-user-ui/src/components/ChooseAnOrganization/index.tsx

+  const NextPage = () => {
+    setCurrentPage(currentPage + 1);
+  };
+
+  const PreviousPage = () => {
+    setCurrentPage(currentPage - 1);
+  };
+
+  useEffect(() => {
+    if (currentPage !== 1) {
+      getAllOrganizations();
+    }
+  }, [currentPage, getAllOrganizations]);
+
+  useEffect(() => {
+    if (page && page_size && count && page * page_size >= count) {
+      setWasLastPage(true);
+    }
+    return () => {
+      setWasLastPage(false);
+    };
+  }, [page, page_size, count]);


Put all this logic inside a hooks will make those logics more testable if you planned to write unit tests

elysee15 · 2023-03-27T21:37:00Z

web/gds-user-ui/src/components/ChooseAnOrganization/index.tsx

+  // eslint-disable-next-line @typescript-eslint/no-unused-vars
+  // useEffect(() => {
+  //   if (prevPage !== currentPage) {
+  //     setPrevPage(currentPage + 1);
+  //     if (organizations && organizations.organizations.length === 0) {
+  //       setOrgList([...orgList, ...organizations.organizations]);
+  //     }
+  //   }
+  // }, [currentPage, organizations, orgList, prevPage]);
+
+  // eslint-disable-next-line @typescript-eslint/no-unused-vars
+  // const onScroll = () => {
+  //   if (listInnerRef.current) {
+  //     const { scrollTop, scrollHeight, clientHeight } = listInnerRef.current;
+  //     // console.log('[scrollTop]', scrollTop);
+  //     // console.log('[scrollHeight]', scrollHeight);
+  //     // console.log('[clientHeight]', clientHeight);
+  //     // console.log('[scrollTop + clientHeight]', scrollTop + clientHeight);
+
+  //     if (scrollTop + clientHeight === scrollHeight) {
+  //       setCurrentPage(currentPage + 1);
+  //     }
+  //   }
+  // };
+


Did you planned to use this logic later ?

elysee15 · 2023-03-27T21:39:05Z

web/gds-user-ui/src/components/ChooseAnOrganization/index.tsx

@@ -76,6 +143,14 @@ function ChooseAnOrganization() {
          </Stack>
        </Stack>
      </Stack>
+      <HStack>
+        <Button onClick={PreviousPage} disabled={currentPage === 1}>


We can create here something like isCurrentPage and add it inside a hooks (#988 (comment))

Suggested change

<Button onClick={PreviousPage} disabled={currentPage === 1}>

<Button onClick={PreviousPage} disabled={isCurrentPage}>

elysee15

Looks good! I have asked some questions and suggested changements at some level

masskoder added 2 commits March 27, 2023 11:34

feat: implement pagination

a71628a

fix: remove history folder

98e8f5f

masskoder changed the title ~~feat: implement pagination~~ implement pagination on VASP List Mar 27, 2023

fix: t.replace is not a function

c21497c

masskoder force-pushed the sc-12056 branch from c5103d7 to c21497c Compare March 27, 2023 16:15

refact pagination workflow

0d50b35

masskoder requested a review from elysee15 March 27, 2023 19:58

masskoder added 4 commits March 27, 2023 16:00

refact: improve data fetching

4e12461

refact: disabled button if last page

136590f

refact: improve next page button action

4590fee

clean up

0573702

elysee15 reviewed Mar 27, 2023

View reviewed changes

elysee15 suggested changes Mar 27, 2023

View reviewed changes

elysee15 self-requested a review March 27, 2023 23:16

elysee15 approved these changes Mar 27, 2023

View reviewed changes

refact: add usePaginate hooks to handle pagination logic

71c12b1

masskoder merged commit 9bbfe8d into main Mar 28, 2023

masskoder deleted the sc-12056 branch March 28, 2023 13:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

implement pagination on VASP List #988

implement pagination on VASP List #988

masskoder commented Mar 27, 2023 •

edited

Loading

elysee15 Mar 27, 2023

elysee15 Mar 27, 2023

masskoder Mar 28, 2023

elysee15 Mar 27, 2023

masskoder Mar 28, 2023

elysee15 Mar 27, 2023

elysee15 Mar 27, 2023 •

edited

Loading

elysee15 Mar 27, 2023

elysee15 Mar 27, 2023

elysee15 Mar 27, 2023 •

edited

Loading

elysee15 left a comment

	export interface organizationResponse {
	export interface OrganizationResponse {

	export const getAllOrganisations = async (page?: number) => {
	export const getAllOrganisations = async (page = 1) => {

	<Button onClick={PreviousPage} disabled={currentPage === 1}>
	<Button onClick={PreviousPage} disabled={isCurrentPage}>

implement pagination on VASP List #988

implement pagination on VASP List #988

Conversation

masskoder commented Mar 27, 2023 • edited Loading

Scope of changes

Type of change

Acceptance criteria

Author checklist

Reviewer(s) checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elysee15 Mar 27, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elysee15 Mar 27, 2023 • edited Loading

Choose a reason for hiding this comment

elysee15 left a comment

Choose a reason for hiding this comment

masskoder commented Mar 27, 2023 •

edited

Loading

elysee15 Mar 27, 2023 •

edited

Loading

elysee15 Mar 27, 2023 •

edited

Loading