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

Display badge indicating when a student dropped an assignment #6898

Merged
merged 1 commit into from
Dec 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions lms/static/scripts/frontend_apps/api-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,9 @@ export type Student = {
h_userid: string;
lms_id: string;
display_name: string | null;

/** Whether this student is active in the course/assignment or roster */
active: boolean;
};

export type AutoGradingGrade = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import FormattedDate from './FormattedDate';
import GradeIndicator from './GradeIndicator';
import type { OrderableActivityTableColumn } from './OrderableActivityTable';
import OrderableActivityTable from './OrderableActivityTable';
import StudentStatusBadge from './StudentStatusBadge';
import SyncGradesButton from './SyncGradesButton';

type StudentsTableRow = {
Expand All @@ -47,6 +48,9 @@ type StudentsTableRow = {
* If the assignment is not auto-grading, this will be ´undefined`.
*/
last_grade?: number | null;

/** Whether this student is active in the course/assignment or roster */
active: boolean;
};

/**
Expand Down Expand Up @@ -150,7 +154,8 @@ export default function AssignmentActivity() {

return students.data.students
.filter(
({ auto_grading_grade }) =>
({ auto_grading_grade, active }) =>
active &&
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Inactive users should not sync their grades.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, it does look simple, nice 👍

!!auto_grading_grade &&
auto_grading_grade.current_grade !== auto_grading_grade.last_grade,
)
Expand Down Expand Up @@ -381,15 +386,25 @@ export default function AssignmentActivity() {
);
case 'display_name':
return (
stats.display_name ?? (
<span className="flex flex-col gap-1.5">
<span className="italic">Unknown</span>
<span className="text-xs text-grey-7">
This student launched the assignment but didn{"'"}t
annotate yet
<div className="flex items-center justify-between gap-x-2">
{stats.display_name ?? (
<span className="flex flex-col gap-1.5">
<span className="italic">Unknown</span>
<span className="text-xs text-grey-7">
This student launched the assignment but didn{"'"}t
annotate yet
</span>
</span>
</span>
)
)}
{!stats.active && (
<div
className="-my-0.5"
title="This student is no longer in this assignment"
>
<StudentStatusBadge type="drop" />
</div>
)}
</div>
);
case 'current_grade':
return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import type {
StudentGradingSyncStatus,
} from '../../api-types';
import GradeStatusChip from './GradeStatusChip';
import type { StudentStatusType } from './StudentStatusBadge';
import StudentStatusBadge from './StudentStatusBadge';

type AnnotationCountProps = {
children: ComponentChildren;
Expand Down Expand Up @@ -57,27 +59,6 @@ function SectionTitle({ children }: { children: ComponentChildren }) {
);
}

type BadgeType = 'new' | 'error' | 'syncing';

function Badge({ type }: { type: BadgeType }) {
return (
<div
className={classnames(
'px-1 py-0.5 rounded cursor-auto font-bold uppercase text-[0.65rem]',
{
'bg-grey-7 text-white': type === 'new',
'bg-grade-error-light text-grade-error': type === 'error',
'bg-grey-2 text-grey-7': type === 'syncing',
},
)}
>
{type === 'new' && 'New'}
{type === 'error' && 'Error'}
{type === 'syncing' && 'Syncing'}
</div>
);
}

export type GradeIndicatorProps = {
grade: number;
lastGrade?: number | null;
Expand Down Expand Up @@ -114,7 +95,7 @@ export default function GradeIndicator({
// Checking typeof lastGrade to avoid number zero to be treated as false
const hasLastGrade = typeof lastGrade === 'number';
const gradeHasChanged = lastGrade !== grade;
const badgeType = ((): BadgeType | undefined => {
const badgeType = ((): StudentStatusType | undefined => {
if (status === 'in_progress') {
return 'syncing';
}
Expand Down Expand Up @@ -142,7 +123,7 @@ export default function GradeIndicator({
>
<GradeStatusChip grade={grade} />
</button>
{badgeType && <Badge type={badgeType} />}
{badgeType && <StudentStatusBadge type={badgeType} />}
</div>
<div aria-live="polite" aria-relevant="additions">
{popoverVisible && (
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import classnames from 'classnames';

/**
* Grade syncing:
* - `new`: A new grade exists that has not been synced.
* - `error`: Last attempt on syncing a grade failed.
* - `syncing`: Syncing grades is in progress for a particular student.
* Assignment participation:
* - `drop`: The student dropped the assignment.
*/
export type StudentStatusType = 'new' | 'error' | 'syncing' | 'drop';

/**
* Badge displaying different student-related statuses around assignment
* participation or grade syncing
*/
export default function StudentStatusBadge({
type,
}: {
type: StudentStatusType;
}) {
return (
<div
className={classnames(
'px-1 py-0.5 rounded cursor-auto font-bold uppercase text-[0.65rem]',
{
'bg-grey-7 text-white': type === 'new' || type === 'drop',
'bg-grade-error-light text-grade-error': type === 'error',
'bg-grey-2 text-grey-7': type === 'syncing',
},
)}
>
{type === 'new' && 'New'}
{type === 'error' && 'Error'}
{type === 'syncing' && 'Syncing'}
{type === 'drop' && 'Drop'}
</div>
Copy link
Member

Choose a reason for hiding this comment

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

Can this badge live update while the page is loaded? If so we may need to consider accessibility affordances for this. Leave that out of scope for this PR though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The ones related with grades sync can update at runtime.

When clicking sync grades, all badges showing "new" or "error" will transition into syncing, and once finished, they will either disappear or transition back to error.

I'll create a separate issue to wrap them into an aria-live container.

);
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ describe('AssignmentActivity', () => {
current_grade: 0.5,
last_grade: null,
},
active: true,
},
{
display_name: 'a',
Expand All @@ -35,6 +36,7 @@ describe('AssignmentActivity', () => {
current_grade: 0.8,
last_grade: 0.61,
},
active: true,
},
{
display_name: 'c',
Expand All @@ -47,6 +49,7 @@ describe('AssignmentActivity', () => {
current_grade: 0.4,
last_grade: null,
},
active: false,
},
];
const activeAssignment = {
Expand Down Expand Up @@ -121,6 +124,8 @@ describe('AssignmentActivity', () => {
// Do not mock FormattedDate, for consistency when checking
// rendered values in different columns
'./FormattedDate': true,
// Let badges render normally so that we can assert on their text
'./StudentStatusBadge': true,
});
$imports.$mock({
'../../utils/api': {
Expand Down Expand Up @@ -221,14 +226,35 @@ describe('AssignmentActivity', () => {
expectedValue: '',
studentStats: { last_activity: null },
},
// Render display_name when it's null
// Render fallback when display_name is null
{
fieldName: 'display_name',
expectedValue:
"UnknownThis student launched the assignment but didn't annotate yet",
studentStats: {
id: 'e4ca30ee27eda1169d00b83f2a86e3494ffd9b12',
display_name: null,
active: true,
},
},
// Render fallback when display_name is null and user is not active
{
fieldName: 'display_name',
expectedValue:
"UnknownThis student launched the assignment but didn't annotate yetDrop",
studentStats: {
id: 'e4ca30ee27eda1169d00b83f2a86e3494ffd9b12',
display_name: null,
active: false,
},
},
// Render inactive user's display name
{
fieldName: 'display_name',
expectedValue: 'Jane DoeDrop',
studentStats: {
display_name: 'Jane Doe',
active: false,
},
},
].forEach(({ fieldName, expectedValue, studentStats }) => {
Expand All @@ -238,6 +264,7 @@ describe('AssignmentActivity', () => {
last_activity: '2024-01-01T10:35:18',
annotations: 37,
replies: 25,
active: true,
};
const wrapper = createComponent();

Expand Down Expand Up @@ -566,6 +593,7 @@ describe('AssignmentActivity', () => {
auto_grading_grade: {
current_grade: 0.5,
},
active: true,
},
// Included, because last grade and current grade are different
{
Expand All @@ -575,11 +603,13 @@ describe('AssignmentActivity', () => {
current_grade: 0.87,
last_grade: 0.7,
},
active: true,
},
// Ignored, because auto_grading_grade is not set
{
display_name: 'c',
h_userid: 'baz',
active: true,
},
// Ignored, because last and current grades are the same
{
Expand All @@ -589,6 +619,16 @@ describe('AssignmentActivity', () => {
current_grade: 0.64,
last_grade: 0.64,
},
active: true,
},
// Ignored, because it's not active
{
display_name: 'e',
h_userid: 'foo',
auto_grading_grade: {
current_grade: 0.5,
},
active: false,
},
],
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ describe('GradeIndicator', () => {
].forEach(({ lastGrade, shouldShowBadge, shouldShowPrevGrade }) => {
it('shows the "new" badge if last grade is not set or is different than current grade', () => {
const wrapper = createComponent({ lastGrade });
const badge = wrapper.find('Badge[type="new"]');
const badge = wrapper.find('StudentStatusBadge[type="new"]');

assert.equal(badge.exists(), shouldShowBadge);
if (shouldShowBadge) {
Expand Down Expand Up @@ -244,7 +244,7 @@ describe('GradeIndicator', () => {
].forEach(({ status, expectedBadge, expectedBadgeText }) => {
it('shows the corresponding badge based on status', () => {
const wrapper = createComponent({ status });
const badge = wrapper.find(`Badge[type="${expectedBadge}"]`);
const badge = wrapper.find(`StudentStatusBadge[type="${expectedBadge}"]`);

assert.isTrue(badge.exists());
assert.equal(badge.text(), expectedBadgeText);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { mount } from 'enzyme';

import StudentStatusBadge from '../StudentStatusBadge';

describe('StudentStatusBadge', () => {
[
{ type: 'new', expectedText: 'New' },
{ type: 'error', expectedText: 'Error' },
{ type: 'syncing', expectedText: 'Syncing' },
{ type: 'drop', expectedText: 'Drop' },
].forEach(({ type, expectedText }) => {
it('shows right text based on type', () => {
const badge = mount(<StudentStatusBadge type={type} />);
assert.equal(badge.text(), expectedText);
});
});
});
Loading