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

fix: Update metrics calculations and related UI components #1172

Merged
merged 11 commits into from
Jul 25, 2024

Conversation

howard-e
Copy link
Contributor

@howard-e howard-e commented Jul 24, 2024

See #1173 and #1174

This PR addresses several issues:

  • Persist proper metrics values to database when assertionExceptions exist (this wasn't being fully handled during a server calculation which could lead to incorrect values when referencing the database stored metrics values -- but was already being recalculated and accounted for when displaying the values on the client so there should be no noticeable change in those cases)
  • Historically, ARIA-AT App was presenting only the calculated value of the MUST assertions for the progress bars and the associated messages on the Reports and Candidate Review Page. I can only guess this was because of the previously used REQUIRED (now MUST) and OPTIONAL (now SHOULD) priorities. But now that there is MUST, SHOULD and MAY, the representation of that progress bar now additionally includes SHOULD priority assertions.
  • Replaces the usages of Math.round() with Math.floor() in presenting metrics' supportPercent.
  • Adds additional tests for metrics calculations.
  • Revised use of aria-label on progressbar component.

@howard-e howard-e changed the title Update metrics calculations and UI components Update metrics calculations and related UI components Jul 24, 2024
@howard-e howard-e requested a review from stalgiag July 25, 2024 14:25
Comment on lines +11 to +13
testPlanReport.testPlanRuns.find(testPlanRun =>
testPlanRun.testResults?.some(testResult => !!testResult.completedAt)
) ||
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For reviewers: Found odd behaviors when doing the following:

  • Assigning self to report row on Test Queue
  • Assigning other tester to report row on Test Queue
  • Opening the report as the other tester navigating and this resolver could silently throw errors when navigating so attempted to resolve that

@howard-e howard-e changed the title Update metrics calculations and related UI components fix: Update metrics calculations and related UI components Jul 25, 2024
Copy link
Contributor

@stalgiag stalgiag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally and this resolves the issues as listed. The code looks good and the unit tests are welcome since it is difficult to get the DB into the various related testing states. Looks good to me!

@stalgiag stalgiag merged commit e4242bc into development Jul 25, 2024
2 checks passed
@stalgiag stalgiag deleted the update-metricsCalculation-and-progressbar branch July 25, 2024 20:04
howard-e added a commit that referenced this pull request Jul 29, 2024
Create July 29, Release

Includes the following changes:

* #1172, to address #1173 and #1174
* #1170, to address #1171
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.

2 participants