[#11015] break down question result fetch by section

[moziliar]

Fixes #11015

Refactor of #11017 due to branch diversion

Original problem

Feedback result page from session with too many responses will cause backend timeout or OOM. This is because of the sheer amount of data to be processed and kept in memory within one API call, and therefore suffers from scalability problem.

Outline of Solution

Frontend

When fetching feedback result with FULL_DETAIL intent, fetch by giver's and receiver's section separately. This is to ensure the scalability of result fetch action from the frontend and is guaranteed by the section size limit.

Section was created to limit the number of students per API call. Using it as the unit of result fetch will likely not run into OOM issue.
Currently, only expandQuestion will have such breakdown; expandSection will not be affected as it already fetches by section.

Backend

Adds ResultFetchType parameter which fetches feedback responses by giverSection, receiverSection, or both. This is only applied in FULL_DETAIL intent, and will be expanded to forUser result fetch should there be concerns arising from that data path.

Testing

FeedbackResultsPageE2E test should cover the frontend correctness of this solution.

Unit tests are created to ensure the parameters are working as intended.

[jianhandev]

LGTM

[wkurniawan07]

Need more tests. There are no tests for fetch type that is not BOTH right now.

[moziliar]

@wkurniawan07 src/test/java/teammates/ui/webapi/GetSessionResultsActionTest.java
Added test here; merging of the results (in SessionResultData) from both GIVER and RECEIVER types is hard at the backend as it will involve complicated wiring of deserializing the data. The test should be covered by e2e where the same result fetch is still tested.

[wkurniawan07]

src/test/java/teammates/ui/webapi/GetSessionResultsActionTest.java
Added test here

This is merely testing that the API returns the value as returned by logic.getSessionResultsForCourse. Which tests guarantee that logic.getSessionResultsForCourse and the DB layer method return the right value, given the fetch type?

The test should be covered by e2e where the same result fetch is still tested.

E2E tests cannot replace lower level tests.

[nusoss-bot]

Guys, This PR seems to be stalling (no activities for the past 7 days). 🐌 😢
Hope someone can get it to move forward again soon...

[nusoss-bot]

Guys, This PR seems to be stalling (no activities for the past 7 days). 🐌 😢
Hope someone can get it to move forward again soon...

[wkurniawan07]

Apologies for the delayed review. Just one change requested.

Also, please ensure that the section results are fetched sequentially in order to prevent burst of requests in courses with large number of sections. (I did see the concatMap, so just need to double confirm)

[wkurniawan07]

There shouldn't be a need to split by giver/receiver here. There is a high likelihood for large amount of duplicates. Splitting by section is enough to give the needed performance boost.

It does mean that the back-end split of giver/receiver is not actually needed right now, but they can and will have their uses in other view types (GRQ and alike) eventually, so no work is wasted here.

[moziliar]

Updated to 'both'. Works as expected.

[moziliar]

Currently, the result fetch is in a sequential manner. Probably need to set up jest tests to verify too.

[wkurniawan07]

No outstanding change requests. I will give the official approval once the lint violation is fixed. (very minor)