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

wip/Optional Assessments on Progress Reports #422

Merged
merged 14 commits into from
Apr 1, 2024
Merged

Conversation

ksmontville
Copy link
Collaborator

This PR builds out the functionality for displaying optional assessments on progress reports, including exporting scores.

@ksmontville
Copy link
Collaborator Author

image

Copy link

github-actions bot commented Mar 13, 2024

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

https://roar-staging--pr422-optional-reports-ekv5zl3k.web.app

(expires Thu, 04 Apr 2024 16:40:51 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 2631e9c58fd0104ecbfddd72a62245ddac467460

Copy link

cypress bot commented Mar 13, 2024

Passing run #853 ↗︎

0 34 0 0 Flakiness 0

Details:

Tests for PR 422 "wip/Optional Assessments on Progress Reports" from commit "cbe...
Project: roar-dashboard-e2e Commit: cbe770e869
Status: Passed Duration: 34:19 💡
Started: Mar 28, 2024 4:42 PM Ended: Mar 28, 2024 5:17 PM

Review all test suite changes for PR #422 ↗︎

@ksmontville ksmontville changed the title Optional Assessments on Progress Reports wip/Optional Assessments on Progress Reports Mar 13, 2024
Copy link
Contributor

@richford richford left a comment

Choose a reason for hiding this comment

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

Thanks @ksmontville! I left some suggestions. Also, I think that the order of evaluation for all of the if blocks should be to check for

  • completion first
  • else if (optional)
  • else if (started)
  • else (assigned)

My reasoning is that we want to show if a student has completed even if it's optional for them.

src/pages/ProgressReport.vue Outdated Show resolved Hide resolved
src/pages/ProgressReport.vue Outdated Show resolved Hide resolved
src/pages/ProgressReport.vue Outdated Show resolved Hide resolved
src/pages/ProgressReport.vue Outdated Show resolved Hide resolved
@richford
Copy link
Contributor

@ksmontville , thanks! LGTM. The title still says WIP, but if this is ready, please feel free to merge.

@ksmontville ksmontville merged commit 355a906 into main Apr 1, 2024
14 checks passed
@ksmontville ksmontville deleted the optional-reports branch April 1, 2024 16:10
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