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

Extension metrics #1878

Merged
merged 5 commits into from
May 17, 2023
Merged

Extension metrics #1878

merged 5 commits into from
May 17, 2023

Conversation

KesterTan
Copy link
Contributor

Description

Previously, an instructor had to go through every homework and manually track the number of extensions requests a student had per course. This change allows instructors to filter students in a course by the number of extensions that was given in Student Metrics.

How Has This Been Tested?

  1. Create a new course or use an existing course
  2. Upload a few assessments if the course does not have any already
  3. Click into an assessment, select manage extensions, and give extensions to any student enrolled in the course.
  4. Test it out with different assessments, different students, and different extension lengths (especially try giving the same student an extension in multiple assessments).
  5. Select Student Metrics, type in the amount of extensions requests you want to filter for, and click save
  6. Check the watchlist to see if students appear in the watchlist. If not, try refreshing the list using the refresh button or manually refreshing the page.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • I have run rubocop for style check. If you haven't, run overcommit --install && overcommit --sign to use pre-commit hook for linting
  • My change requires a change to the documentation, which is located at Autolab Docs
  • I have updated the documentation accordingly, included in this PR

@KesterTan KesterTan marked this pull request as ready for review April 15, 2023 08:35
@gitstream-cm gitstream-cm bot requested a review from victorhuangwq April 15, 2023 08:36
@victorhuangwq
Copy link
Contributor

Testing locally

Hover over error

  1. Gave User0 1 day extension on Lab0, and User1 2 day extension on Homework2
  2. Selected Students who have greater than or equal to 1 extensions.
  3. Both students appeared, but the hover over are both wrong

image

image

Did not clear out previous metrics / Regression

  1. Students who did not submit at least 1 assignment(s)
  2. Look at metrics, many students should appear
  3. Select Students who have greater than or equal to 1 extensions, and unselect Students who did not submit at least 1 assignment(s)
  4. Now you get students who have made 0 extension requests
    image.

@victorhuangwq
Copy link
Contributor

victorhuangwq commented Apr 22, 2023

On the front end, I am able to set, and also save that
image

although it seems to be clamped to 1 in the backend.

You should have field validation on the front end to allow only positive integers

Copy link
Contributor

@victorhuangwq victorhuangwq left a comment

Choose a reason for hiding this comment

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

Issue with ability to set to more than or equals to 0 on front end. As well as other nits.

app/models/risk_condition.rb Outdated Show resolved Hide resolved
app/models/watchlist_instance.rb Show resolved Hide resolved
app/models/watchlist_instance.rb Outdated Show resolved Hide resolved
@victorhuangwq
Copy link
Contributor

I don't think you took into account Considered categories when making your calculations:
image
image

I excluded Labs, and I get lab categories extensions

@reviewpad reviewpad bot mentioned this pull request May 16, 2023
Copy link
Contributor

@victorhuangwq victorhuangwq left a comment

Choose a reason for hiding this comment

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

Functionality works as expected. Small nit that was not addressed previously.

LGTM

app/models/watchlist_instance.rb Show resolved Hide resolved
@KesterTan KesterTan added this pull request to the merge queue May 17, 2023
Merged via the queue into master with commit 1940740 May 17, 2023
@KesterTan KesterTan deleted the extensions branch May 17, 2023 02:27
@damianhxy damianhxy linked an issue May 22, 2023 that may be closed by this pull request
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.

[Design Doc] Display / consider number of extensions in metrics
3 participants