[#11302] Increase search result limit with message prompt when limit is reached

[tjtanjin]

Fixes #11302

Outline of Solution

• Raise the current search result limit from 20 to 50.
• Should 50 results be returned and shown to the user, an additional message is shown to inform the user that there may be more results with a suggestion to search with more specific terms.

An example of how it would look like for a testing limit of 2 (instead of 50) is shown below for the admin search (the same behaviour is implemented for instructor search):

[damithc]

I'm OK with the proposed UX

[tjtanjin]

Hello! Is anyone able to advice on the updating of snapshot tests?

I've followed the official jest documentation and tried the following 2 commands:

• yarn jest --updateSnapshot
• ./node_modules/.bin/jest --updateSnapshot

Both commands however produce an error of the following form for all 249 test cases (all failed to run):

• SyntaxError: /path/to/teammates/jest-setup.ts: Unexpected token, expected "," (4:8)

Thus far, I have tried updating package.json and jest.config.js as suggested here. However, this did not resolve the issue. Any help/pointers would be greatly appreciated!

[damithc]

@TEAMMATES/active-mentors any help?

[samuelfangjw]

Hello! Is anyone able to advice on the updating of snapshot tests?

I've followed the official jest documentation and tried the following 2 commands:

• yarn jest --updateSnapshot
• ./node_modules/.bin/jest --updateSnapshot

Both commands however produce an error of the following form for all 249 test cases (all failed to run):

• SyntaxError: /path/to/teammates/jest-setup.ts: Unexpected token, expected "," (4:8)

Thus far, I have tried updating package.json and jest.config.js as suggested here. However, this did not resolve the issue. Any help/pointers would be greatly appreciated!

Hey @tjtanjin! I usually do npm run test and wait till I get to this screen (see screenshot) then follow the on screen instructions to update the snapshots (either u to update all or i to update interactively).

Hope this helps!

[tjtanjin]

@samuelfangjw Your suggestion helped, thank you! :)

[jianhandev]

Hi @tjtanjin let us know once the PR is ready for review, thanks!

[tjtanjin]

Hi @jianhandev the PR is ready for review!

[daongochieu2810]

@tjtanjin seems like there are some merge conflicts,can you resolve them first? thanks

[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...

[t-cheepeng]

@daongochieu2810 @jianhandev For your kind review. Thanks

[moziliar]

Hi, just some clarification here

[daongochieu2810]

@tjtanjin can you create more data so that the search will return 50 or more results? so that we know it works as intended

[tjtanjin]

@tjtanjin can you create more data so that the search will return 50 or more results? so that we know it works as intended

Hi! Here's a video showing the search return for 50 or more results:

Screen.Recording.2021-08-23.at.9.08.55.PM.mov

[jianhandev]

LGTM

[jianhandev]

Thanks @wkurniawan07 @rrtheonlyone for your inputs. @tjtanjin moving forward, perhaps you can:

1. Find out how to propagate the constant in the BE to the FE (see ApiConst.java)
2. In admin-search-page.component.ts, check if the number of student and instructor results are each greater than their individual limit of 50, and display a toast message respectively for each scenario, for example:

Both exceed:

Instructor results exceed:

Student results exceed:

[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...

[moziliar]

@tjtanjin Hi, any updates for this PR?

[tjtanjin]

@t-cheepeng @jianhandev @moziliar Apologies for the late update, I have committed changes based on given suggestions, kindly advice if the changes are appropriate. Thank you!

[tjtanjin]

@wkurniawan07 Thank you for the suggestions, I have made the changes accordingly.

[wkurniawan07]

I'm ok to accept the PR as is, but I'll leave behind a suggestion

[wkurniawan07]

Here's how I'm going to refactor this part:

const limit: number = ApiConst.SEARCH_QUERY_SIZE_LIMIT;
const limitsReached: string[] = [];
if (this.students.length >= limit) {
  limitsReached.push(`${limit} student results`);
}
if (this.instructors.length >= limit) {
  limitsReached.push(`${limit} instructor results`);
}
if (limitsReached.length) {
  this.statusMessageService.showWarningToast(`${limitsReached.join(' and ')} have been shown on this page
      but there may be more results not shown. Consider searching with more specific terms.`);
}

[tjtanjin]

I've updated the code according to your suggestion, thank you!

[madanalogy]

Changes seem fine, all comments so far addressed

Resolved