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

Feat/2387 improve eligibility report v2 #2390

Merged
merged 19 commits into from
May 31, 2024

Conversation

KoWeiJAC
Copy link
Contributor

@KoWeiJAC KoWeiJAC commented May 9, 2024

What's included?

closed #2387

  • Admins can make separate recommendations for Stat criteria, PJE and RLoS on Legal exercises
  • Admins can view candidate responses where they haven't met PJE or RLoS on Legal exercises
  • Admins can add separate comments about their separate recommendations for Stat criteria, PJE and RLoS on Legal exercises

Who should test?

✅ Product owner
✅ Developers
✅ UTG

How to test?

Risk - how likely is this to impact other areas?

🟢 No risk - this is a self-contained piece of work

Additional context

Include screen grabs, video demo, notes etc.

Related permissions

Have permissions been considered for this functionality?

  • No permission changes required
  • Permissions have been added / updated. Details:

PREVIEW:DEVELOP
can be OFF, DEVELOP or STAGING

Copy link

github-actions bot commented May 9, 2024

Visit the preview URL for this PR (updated for commit cb92b0e):

https://jac-admin-develop--pr2390-feat-2387-improve-el-s0bfjq10.web.app

(expires Sun, 30 Jun 2024 08:39:41 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 4e92cf51659207b0ae3509dc5c40edde50edfec0

@KoWeiJAC KoWeiJAC marked this pull request as ready for review May 9, 2024 11:40
@KoWeiJAC KoWeiJAC requested review from a team as code owners May 9, 2024 11:40
@nickaddy
Copy link
Contributor

@KoWeiJAC I'm afraid you've misread this ticket: pls look again - for Statutory eligibility, we are asking to make a single recommendation for Statutory criteria, i.e. we need to display the flags for Prof Qualification and PQE but have only one recommendation dropdown and one reason free text field for the Statutory section.

Non-statutory section looks correct.

One other comment: is it possible to have a heavier line between candidates than we have within each candidate section?

image.png

I think this will help to pick out separate candidate entries.

@nickaddy
Copy link
Contributor

@KoWeiJAC In reviewing the report download, I've noticed a couple of issues on this ticket:

  • When content of candidate’s application is updated, e.g. RLoS and PJE sections, then the eligibility report refreshed, the updates are not see on the eligibility report page
  • When you click View application (to the right of candidates' names,) the page should open in a new tab
  • Under Non-statutory section, Reasons not satisfied title of both free text fields should read JAC Comments

@nickaddy
Copy link
Contributor

@KoWeiJAC Some more comments from last week's UT:

  • The test exercise had no PQE eligibility set so how was it awarding Met flags to candidates? I’ve changed it to 5y and refreshed but the calculation of PQE does not seem to be working properly
  • Related to the first point: Duration content in brackets missing after first candidate is missing
  • image.png
  • We need to rethink this filter; it was designed for just one recommendation dropdown: could it include candidates where 1 or more of the recommendation dropdowns is unassigned?
  • image.png
  • Once you have sorted the above, could you pls create an application that does meet all the criteria so that we can test the Display only candidates with Eligibility issues tickbox?

@KoWeiJAC
Copy link
Contributor Author

@KoWeiJAC Some more comments from last week's UT:

  • The test exercise had no PQE eligibility set so how was it awarding Met flags to candidates? I’ve changed it to 5y and refreshed but the calculation of PQE does not seem to be working properly
  • Related to the first point: Duration content in brackets missing after first candidate is missing
  • image.png
  • We need to rethink this filter; it was designed for just one recommendation dropdown: could it include candidates where 1 or more of the recommendation dropdowns is unassigned?
  • image.png
  • Once you have sorted the above, could you pls create an application that does meet all the criteria so that we can test the Display only candidates with Eligibility issues tickbox?

@nickaddy
The refresh button is a little bit tricky, to prevent the system reset the recommendations, it only set one application for one time. It means if the application is added to report, this application won't be refreshed for not overwriting the existing recommendations. But the refresh button can add newly added applications to report. This mechanism is the same as the character issue report.

To improve it, maybe we can refresh(update) the application which have no recommendations. If the application have any recommendation being set, the refresh would skip this application for not overwriting those recommendations.
Would you think it's workable ?

@KoWeiJAC
Copy link
Contributor Author

  • The test exercise had no PQE eligibility set so how was it awarding Met flags to candidates?

@nickaddy The test exercise had no PQE eligibility set so how was it awarding Met flags to candidates? => The system will make the PQE to be Met as default if the PQE years is not set.
Do we need to have change on this behaviour?

@nickaddy
Copy link
Contributor

It looks like the only candidates that were flagged as Not Met were the ones that entered no work experience at all. If you check Exercise setup, the options under Eligibility Information are 5y, 7y or other - where you enter the number of years required. I don't understand how this exercise was created with 0 years PQE. If that is correct, even the candidates with no work experience should be flagged as Met.

@KoWeiJAC
Copy link
Contributor Author

KoWeiJAC commented May 20, 2024

It looks like the only candidates that were flagged as Not Met were the ones that entered no work experience at all. If you check Exercise setup, the options under Eligibility Information are 5y, 7y or other - where you enter the number of years required. I don't understand how this exercise was created with 0 years PQE. If that is correct, even the candidates with no work experience should be flagged as Met.

Just had a test, when creating the legal exercise, the PQE field is not required, so user can leave it empty and the PQE will be 0 as default.
I was using this exercise for test: https://jac-admin-develop--pr2390-feat-2387-improve-el-s0bfjq10.web.app/exercise/l6idrKzbXLr7BdJxmZvk/details/eligibility

Not sure if all the legal exercises need to look on PQE? If so, maybe we can make the PQE field required when creating exercise. To ensure PQE is greater than 0 and the candidates with no experience will be fagged Not Met.

Screenshot 2024-05-20 at 16 34 09

@nickaddy
Copy link
Contributor

@KoWeiJAC I agree with your proposal above - the PQE field for legal exercises must be mandatory and any candidates that do not enter any experience flagged as Not Met (in addition to those that do not meet the required PQE.)

For reference, non-legal exercises do not request PQE according to my understanding.

@HalcyonJAC
Copy link
Contributor

HalcyonJAC commented May 29, 2024

@KoWeiJAC I agree with your proposal above - the PQE field for legal exercises must be mandatory and any candidates that do not enter any experience flagged as Not Met (in addition to those that do not meet the required PQE.)

For reference, non-legal exercises do not request PQE according to my understanding.

@nickaddy

  1. The PQE field has been mandatory for legal exercises.
  2. The refresh button should be working now and it will keep the data of the recommendations.

@nickaddy
Copy link
Contributor

@HalcyonJAC I can't find my comment, maybe it was in Slack, regarding the filter for unassigned, has this been fixed? The dropdown options should be All issues and Unassigned - the latter would show candidates where any of the 3 recommendations are not selected or any of the 3 free text fields not populated. Could you address the above pls?

@HalcyonJAC
Copy link
Contributor

@HalcyonJAC I can't find my comment, maybe it was in Slack, regarding the filter for unassigned, has this been fixed? The dropdown options should be All issues and Unassigned - the latter would show candidates where any of the 3 recommendations are not selected or any of the 3 free text fields not populated. Could you address the above pls?

@nickaddy I have fixed the filter. Could you retest it, please?

@nickaddy
Copy link
Contributor

@HalcyonJAC Looking good, Ryan. Can you pls:

  1. Remove the Generate report button
  2. Confirm how you have approached the Refresh button issue, i.e. what happens if content has already been input to the Eligibility report?

@HalcyonJAC
Copy link
Contributor

@nickaddy

  1. The "Generate report" button has been removed.
  2. When users hit the Refresh button, it will populate data from applications again but also keep the fields (Recommendation, Reasons not satisfied, JAC comments).

@HalcyonJAC HalcyonJAC merged commit d5fce48 into main May 31, 2024
6 checks passed
@HalcyonJAC HalcyonJAC deleted the feat/2387-improve-eligibility-report-v2 branch May 31, 2024 08:55
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.

Improve Eligibility report v2
8 participants