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

Jump to bottom

fix: Under review tab pagination bug. #2552

Merged
merged 6 commits into from
Sep 3, 2021
Merged

fix: Under review tab pagination bug. #2552

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
12f1c4d
fix under review tab pagination bug: fetch 5 proposals per request ti…
vibros68 Aug 26, 2021
2d11125
add condition variable, fix scanNextStatusTokens function bug, remove…
vibros68 Aug 27, 2021
ad10ad3
reproduce scan tokens logic
vibros68 Aug 28, 2021
e70c680
batch pagination testing
vibros68 Aug 30, 2021
61a94d2
fix e2e test: records and inventory pagination
vibros68 Aug 31, 2021
2636412
clean up code
vibros68 Aug 31, 2021
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
4 changes: 2 additions & 2 deletions src/containers/Proposal/Actions/UnvettedActionsProvider.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ const UnvettedActionsProvider = ({ children, history }) => {
successTitle: "Proposal approved",
successMessage: (
<Text>
The proposal has been successfully approved! Now it will appear under{" "}
<Link to="/?tab=in%20discussion">In discussion</Link> tab among Public
The proposal has been successfully approved! Now it will appear in{" "}
<Link to="/?tab=under%20review">Under review</Link> tab among Public
Proposals.
</Text>
),
Expand Down
122 changes: 85 additions & 37 deletions src/hooks/api/useProposalsBatch.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ export default function useProposalsBatch({
}) {
const [remainingTokens, setRemainingTokens] = useState([]);
const [voteStatuses, setStatuses] = useState(statuses);
const [previousPage, setPreviousPage] = useState(0);
const [initializedInventory, setInitializedInventory] = useState(false);
const isByRecordStatus = isUndefined(voteStatuses);
const proposals = useSelector(sel.proposalsByToken);
const recordState = unvetted
Expand All @@ -115,8 +115,11 @@ export default function useProposalsBatch({
const allByStatus = useSelector(
isByRecordStatus ? sel.allByRecordStatus : sel.allByVoteStatus
);

const tokens = useMemo(
() => allByStatus[getProposalStatusLabel(currentStatus, isByRecordStatus)],
() =>
allByStatus[getProposalStatusLabel(currentStatus, isByRecordStatus)] ||
[],
[allByStatus, isByRecordStatus, currentStatus]
);
const page = useMemo(() => {
Expand All @@ -130,12 +133,52 @@ export default function useProposalsBatch({
const onFetchProposalsBatch = useAction(act.onFetchProposalsBatch);
const onFetchTokenInventory = useAction(act.onFetchTokenInventory);
const hasRemainingTokens = !isEmpty(remainingTokens);

const scanNextStatusTokens = (index, oldTokens) => {
const status = currentStatuses[index];
const newTokens = getUnfetchedTokens(
proposals,
uniq(allByStatus[getProposalStatusLabel(status, isByRecordStatus)] || [])
);

const tokens = [...oldTokens, ...newTokens];
if (tokens.length < proposalPageSize && index < currentStatuses.length) {
return scanNextStatusTokens(index + 1, tokens);
}
return {
index,
tokens
};
};

const [state, send] = useFetchMachine({
actions: {
initial: () => send(START),
start: () => {
if (hasRemainingTokens) return send(VERIFY);
if (page && page === previousPage) return send(RESOLVE);
if (remainingTokens.length >= proposalPageSize) return send(VERIFY);

// If remaining tokens length is smaller than proposal page size.
// Find more tokens from inventory or scan from the next status

// scanNextStatus: inventory is initialized and
// there are no tokens to be fetched from the next page
const scanNextStatus =
initializedInventory &&
!(tokens.length % INVENTORY_PAGE_SIZE === 0 && tokens.length > 0);
if (scanNextStatus) {
const { index, tokens } = scanNextStatusTokens(
statusIndex + 1,
remainingTokens
);
setStatusIndex(index);
setRemainingTokens(tokens);
if (isEmpty(tokens)) return send(RESOLVE);

return send(VERIFY);
}

// Fetch tokens from inventory. Fetch all statuses at the first page.
// Next times, fetch the next page of the current status.
onFetchTokenInventory(
recordState,
page && currentStatus, // first page fetches all status
Expand All @@ -144,19 +187,42 @@ export default function useProposalsBatch({
)
.catch((e) => send(REJECT, e))
.then(({ votes, records }) => {
setPreviousPage(page);
setInitializedInventory(true);
// prepare token batch to fetch proposal for given status
const proposalStatusLabel = getProposalStatusLabel(
currentStatus,
isByRecordStatus
);
const tokens = (isByRecordStatus ? records : votes)[
proposalStatusLabel
];
if (!tokens) return send(RESOLVE);
const newTokensByStatus = isByRecordStatus ? records : votes;
const newTokens = newTokensByStatus[proposalStatusLabel];
const tokens = [...newTokens, ...remainingTokens];
setRemainingTokens(tokens);

// Go to the next status when the remaining tokens still be smaller
// than proposal page size and current status is not the last one.
if (
tokens.length < proposalPageSize &&
statusIndex + 1 < currentStatuses.length
) {
const nextIndex = statusIndex + 1;
const nextStatus = currentStatuses[nextIndex];
const nextStatusTokens = getUnfetchedTokens(
proposals,
uniq(
(page ? allByStatus : newTokensByStatus)[
getProposalStatusLabel(nextStatus, isByRecordStatus)
] || []
)
);
setRemainingTokens([...tokens, ...nextStatusTokens]);
setStatusIndex(nextIndex);

return send(START);
}

return send(VERIFY);
});

return send(FETCH);
},
verify: () => {
Expand Down Expand Up @@ -192,34 +258,14 @@ export default function useProposalsBatch({
return send(VERIFY);
}
}

return send(RESOLVE);
})
.catch((e) => send(REJECT, e));

return send(FETCH);
},
done: () => {
// If there are not remaining tokens, check if have unscanned status.
// If yes, change the index to the new status and set up new tokens
if (!hasRemainingTokens && statusIndex + 1 < currentStatuses.length) {
const newIndex = statusIndex + 1;
const newStatus = currentStatuses[newIndex];
const unfetchedTokens = getUnfetchedTokens(
proposals,
uniq(
allByStatus[
getProposalStatusLabel(newStatus, isByRecordStatus)
] || []
)
);
setRemainingTokens(unfetchedTokens);
setStatusIndex(newIndex);
if (!values(proposals).length) {
// If no proposals are fetched yet.
// Continue the fetching cycle to fetch the first page.
return send(START);
}
}
}
done: () => {}
},
initialValues: {
status: "idle",
Expand All @@ -238,6 +284,7 @@ export default function useProposalsBatch({
let unfetchedTokens = [],
index = 0;
const statuses = newStatuses || currentStatuses;

const foundPreviousSessionStatus = statuses.find((status, i) => {
const statusLabel = getProposalStatusLabel(status, isByRecordStatus);
unfetchedTokens = getUnfetchedTokens(
Expand All @@ -250,6 +297,7 @@ export default function useProposalsBatch({
}
return false;
});

setRemainingTokens(unfetchedTokens);
setStatusIndex(index);
if (isByRecordStatus) {
Expand All @@ -269,14 +317,14 @@ export default function useProposalsBatch({
state.status === "success" &&
!getUnfetchedTokens(proposals, tokens).length;

const isAnotherInventoryCallRequired =
const isAnotherTokensScanningRequired =
tokens && tokens.length && tokens.length % INVENTORY_PAGE_SIZE === 0;

const onFetchMoreProposals = useCallback(() => {
if (isEmpty(remainingTokens) && isAnotherInventoryCallRequired)
return send(START);
if (remainingTokens.length < proposalPageSize) return send(START);

return send(VERIFY);
}, [send, remainingTokens, isAnotherInventoryCallRequired]);
}, [send, remainingTokens, proposalPageSize]);

const anyError = error || state.error;
useThrowError(anyError);
Expand All @@ -296,7 +344,7 @@ export default function useProposalsBatch({
onRestartMachine,
onFetchMoreProposals,
hasMoreProposals: !!remainingTokens.length && !!values(proposals).length,
isProposalsBatchComplete: isFetchDone && !isAnotherInventoryCallRequired,
isProposalsBatchComplete: isFetchDone && !isAnotherTokensScanningRequired,
machineCurrentState: state.status
};
}
17 changes: 11 additions & 6 deletions teste2e/cypress/e2e/records/recordsList.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ describe("Records list", () => {
});
cy.wait("@records.records");
// each proposal should be rendered accordingly to inventory response
cy.assertListLengthByTestId("record-title", RECORDS_PAGE_SIZE + 3) // first records batch
cy.assertListLengthByTestId("record-title", RECORDS_PAGE_SIZE) // first records batch
.each(([{ id }], position) => {
const tokens = getTokensByStatusTab(inventory, "Under Review");
const expectedToken = shortRecordToken(tokens[position]);
Expand All @@ -50,22 +50,27 @@ describe("Records list", () => {
it("can render records and inventory pagination correctly", () => {
cy.visit(`/`);
cy.wait("@ticketvote.inventory");
// Fetch 'started', 'authorized' and 'unauthorized' proposals
cy.wait("@records.records");
cy.assertListLengthByTestId("record-title", 8);
// 3 items of started status and 2 items of unauthorized status
cy.assertListLengthByTestId("record-title", 5);
cy.scrollTo("bottom");
cy.wait("@records.records");
cy.assertListLengthByTestId("record-title", 13);
cy.assertListLengthByTestId("record-title", 10);
cy.scrollTo("bottom");
cy.wait("@records.records");
cy.assertListLengthByTestId("record-title", 18);
cy.assertListLengthByTestId("record-title", 15);
cy.scrollTo("bottom");
cy.wait("@records.records");
cy.assertListLengthByTestId("record-title", 23);
cy.assertListLengthByTestId("record-title", 20);
// finished first inventory page
cy.scrollTo("bottom");
cy.wait("@ticketvote.inventory").its("request.body.page").should("eq", 2);
cy.wait("@records.records");
// records from second inventory page
cy.assertListLengthByTestId("record-title", 25);
cy.scrollTo("bottom");
cy.wait("@records.records");
cy.assertListLengthByTestId("record-title", 28);
cy.scrollTo("bottom");
// wait to see if no requests are done, since inventory is fully fetched
Expand All @@ -80,7 +85,7 @@ describe("Records list", () => {
// navigate to in discussion tab
cy.findByTestId("tab-0").click();
cy.wait("@records.records");
cy.assertListLengthByTestId("record-title", 8);
cy.assertListLengthByTestId("record-title", 5);
});
it("can list legacy proposals", () => {
// for approved proposals
Expand Down