-
Notifications
You must be signed in to change notification settings - Fork 19
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: dismiss alert modal and cancelled assignments #900
Changes from all commits
f8b5af5
7883bfd
0cb6750
4a95b36
fa3eff5
ab42490
53ba280
64c9e72
c8e10a5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,33 @@ | ||
import { ASSIGNMENT_TYPES } from '../../enterprise-user-subsidy/enterprise-offers/data/constants'; | ||
import { ASSIGNMENT_TYPES, ASSIGNMENT_ACTION_TYPES } from '../../enterprise-user-subsidy/enterprise-offers/data/constants'; | ||
import { | ||
LEARNER_ACKNOWLEDGED_ASSIGNMENT_CANCELLATION_ALERT, | ||
LEARNER_ACKNOWLEDGED_ASSIGNMENT_EXPIRATION_ALERT, | ||
} from '../main-content/course-enrollments/data/constants'; | ||
|
||
export function getIsActiveExpiredAssignment(assignments) { | ||
const lastExpiredAlertDismissedTime = global.localStorage.getItem(LEARNER_ACKNOWLEDGED_ASSIGNMENT_EXPIRATION_ALERT); | ||
|
||
const activeExpiredAssignments = assignments.filter((assignment) => ( | ||
assignment?.actions.some((action) => ( | ||
action.actionType === ASSIGNMENT_ACTION_TYPES.AUTOMATIC_CANCELLATION_NOTIFICATION | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note: the cron job that runs to automatically cancel assignments will result in the assignment(s) being marked as I believe at this point, the handing of expired assignments is for the edge case where a learner is viewing an assignment in the UI that hasn't yet been canceled by the cron job. That said, I think the logic here may need to be based on the determined enrollment deadline as depicted by [idea] Refactor export const isAssignmentExpired = (assignment) => {
// Assignment is not in an allocated state, so it cannot be expired.
if (!assignment) {
return false;
}
const currentDate = dayjs();
const allocationDate = dayjs(assignment.created);
const enrollmentEndDate = dayjs(assignment.contentMetadata.enrollByDate);
const subsidyExpirationDate = dayjs(assignment.subsidyExpirationDate);
const isExpired = (
currentDate.diff(allocationDate, 'day') > 90
|| currentDate.isAfter(enrollmentEndDate)
|| currentDate.isAfter(subsidyExpirationDate)
);
const earliestAssignmentExpiryDate = [
dayjs(allocationDate).add(90, 'day'),
enrollmentEndDate,
subsidyExpirationDate,
].sort()[0]?.toDate();
return {
isExpired,
enrollByDeadline: earliestAssignmentExpiryDate,
};
}; export function getHasActiveExpiredAssignment(assignments) {
const lastExpiredAlertDismissedTime = global.localStorage.getItem(LEARNER_ACKNOWLEDGED_ASSIGNMENT_EXPIRATION_ALERT);
const activeExpiredAssignments = assignments.filter((assignment) => {
const {
isExpired,
enrollByDeadline,
} = isAssignmentExpired(assignment);
if (!isExpired) {
return false;
}
return dayjs(enrollByDeadline).isAfter(new Date(lastExpiredAlertDismissedTime));
});
return activeExpiredAssignments.length > 0;
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a good idea for now, but really we should introduce an |
||
&& new Date(action.completedAt) > new Date(lastExpiredAlertDismissedTime) | ||
)) | ||
)); | ||
return activeExpiredAssignments.length > 0; | ||
} | ||
|
||
export function getIsActiveCancelledAssignment(assignments) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: |
||
const lastCancelledAlertDismissedTime = global.localStorage.getItem( | ||
LEARNER_ACKNOWLEDGED_ASSIGNMENT_CANCELLATION_ALERT, | ||
); | ||
const activeCancelledAssignments = assignments.filter((assignment) => ( | ||
assignment?.actions.some((action) => ( | ||
action.actionType === ASSIGNMENT_ACTION_TYPES.CANCELLED_NOTIFICATION | ||
&& new Date(action.completedAt) > new Date(lastCancelledAlertDismissedTime) | ||
)) | ||
)); | ||
return activeCancelledAssignments.length > 0; | ||
} | ||
|
||
/** | ||
* Takes a flattened array of assignments and returns an object containing: | ||
|
@@ -17,7 +46,7 @@ | |
*/ | ||
export default function getActiveAssignments(assignments = []) { | ||
const activeAssignments = assignments.filter((assignment) => [ | ||
ASSIGNMENT_TYPES.CANCELLED, ASSIGNMENT_TYPES.ALLOCATED, | ||
ASSIGNMENT_TYPES.ALLOCATED, | ||
].includes(assignment.state)); | ||
Comment on lines
47
to
50
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @adamstankiewicz any concerns with changing this definition of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @iloveagent57 @katrinan029 Yes, by removing I might recommend modifying // Renamed `getActiveAssignments` to `getAssignmentsByState`
export function getAssignmentsByState(assignments = []) {
const allAssignments = [];
const allocatedAssignments = [];
const canceledAssignments = [];
const acceptedAssignments = [];
assignments.forEach((assignment) => {
allAssignments.push(assignment);
if (assignment.state === ASSIGNMENT_TYPES.ALLOCATED) {
allocatedAssignments.push(assignment);
}
if (assignment.state === ASSIGNMENT_TYPES.CANCELLED) {
canceledAssignments.push(assignment);
}
if (assignment.state === ASSIGNMENT_TYPES.ACCEPTED) {
acceptedAssignments.push(assignment);
}
});
const hasAssignments = allAssignments.length > 0;
const hasAllocatedAssignments = allocatedAssignments.length > 0;
const hasCanceledAssignments = canceledAssignments.length > 0;
const hasAcceptedAssignments = acceptedAssignments.length > 0;
return {
assignments,
hasAssignments,
allocatedAssignments,
hasAllocatedAssignments,
canceledAssignments,
hasCanceledAssignments,
acceptedAssignments,
hasAcceptedAssignments,
}; Note: This function should really only be called within |
||
const hasAssignments = assignments.length > 0; | ||
const hasActiveAssignments = activeAssignments.length > 0; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,7 @@ import { Info } from '@edx/paragon/icons'; | |
import { getContactEmail } from '../../../../utils/common'; | ||
|
||
const CourseAssignmentAlert = ({ | ||
showAlert, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good call to make this alert component act as a controlled component instead of uncontrolled. |
||
onClose, | ||
variant, | ||
}) => { | ||
|
@@ -20,6 +21,7 @@ const CourseAssignmentAlert = ({ | |
return ( | ||
<Alert | ||
variant="danger" | ||
show={showAlert} | ||
icon={Info} | ||
dismissible | ||
actions={[ | ||
|
@@ -38,11 +40,13 @@ const CourseAssignmentAlert = ({ | |
CourseAssignmentAlert.propTypes = { | ||
onClose: PropTypes.func, | ||
variant: PropTypes.string, | ||
showAlert: PropTypes.bool, | ||
}; | ||
|
||
CourseAssignmentAlert.defaultProps = { | ||
onClose: null, | ||
variant: null, | ||
showAlert: null, | ||
}; | ||
|
||
export default CourseAssignmentAlert; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,8 +17,9 @@ | |
} from './data/utils'; | ||
import { UserSubsidyContext } from '../../../enterprise-user-subsidy'; | ||
import { features } from '../../../../config'; | ||
import getActiveAssignments from '../../data/utils'; | ||
import getActiveAssignments, { getIsActiveCancelledAssignment, getIsActiveExpiredAssignment } from '../../data/utils'; | ||
import { ASSIGNMENT_TYPES } from '../../../enterprise-user-subsidy/enterprise-offers/data/constants'; | ||
import { LEARNER_ACKNOWLEDGED_ASSIGNMENT_CANCELLATION_ALERT, LEARNER_ACKNOWLEDGED_ASSIGNMENT_EXPIRATION_ALERT } from './data'; | ||
|
||
export const COURSE_SECTION_TITLES = { | ||
current: 'My courses', | ||
|
@@ -44,7 +45,10 @@ | |
const { redeemableLearnerCreditPolicies } = useContext(UserSubsidyContext); | ||
|
||
const [assignments, setAssignments] = useState([]); | ||
const [showCancelledAssignmentsAlert, setShowCancelledAssignmentsAlert] = useState(false); | ||
const [ | ||
showCancelledAssignmentsAlert, | ||
setShowCancelledAssignmentsAlert, | ||
] = useState(false); | ||
const [showExpiredAssignmentsAlert, setShowExpiredAssignmentsAlert] = useState(false); | ||
|
||
useEffect(() => { | ||
|
@@ -53,12 +57,12 @@ | |
); | ||
setAssignments(assignmentsData); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [suggestion] I might propose a bit of a further refactor to this useEffect(() => {
if (!redeemableLearnerCreditPolicies) {
return;
}
const {
allocatedAssignments,
canceledAssignments,
hasCanceledAssignments,
} = redeemableLearnerCreditPolicies.learnerContentAssignments;
const sortedAllocatedAssignments = sortAssignmentsByAssignmentStatus(allocatedAssignments);
const transformedAllocatedAssignments = getTransformedAllocatedAssignments(
sortedAllocatedAssignments,
slug,
);
setAssignments(transformedAllocatedAssignments);
const hasActiveCancelledAssignments = (
hasCanceledAssignments && getHasActiveCancelledAssignments(canceledAssignments)
);
setShowCancelledAssignmentsAlert(hasActiveCancelledAssignments);
const hasActiveExpiredAssignments = getHasActiveExpiredAssignment(allocatedAssignments);
setShowExpiredAssignmentsAlert(hasActiveExpiredAssignments);
}, [redeemableLearnerCreditPolicies, slug]); By doing so, we could rely on the state variable |
||
|
||
const hasCancelledAssignments = assignmentsData?.some( | ||
assignment => assignment.state === ASSIGNMENT_TYPES.CANCELLED, | ||
); | ||
const hasExpiredAssignments = assignmentsData?.some(assignment => isAssignmentExpired(assignment)); | ||
const hasActiveCancelledAssignments = assignmentsData?.some((assignment) => ( | ||
assignment.state === ASSIGNMENT_TYPES.CANCELLED)) && getIsActiveCancelledAssignment(assignmentsData); | ||
iloveagent57 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
setShowCancelledAssignmentsAlert(hasActiveCancelledAssignments); | ||
|
||
setShowCancelledAssignmentsAlert(hasCancelledAssignments); | ||
const hasExpiredAssignments = assignmentsData?.some(assignment => isAssignmentExpired(assignment)) | ||
&& getIsActiveExpiredAssignment(assignmentsData); | ||
Comment on lines
+64
to
+65
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Related to another comment, I believe you could move the call to the (refactored) |
||
setShowExpiredAssignmentsAlert(hasExpiredAssignments); | ||
}, [redeemableLearnerCreditPolicies]); | ||
|
||
|
@@ -70,9 +74,9 @@ | |
Object.keys(courseEnrollmentsByStatus).forEach((status) => { | ||
courseEnrollmentsByStatus[status] = courseEnrollmentsByStatus[status].map((course) => { | ||
const isAssigned = assignments?.some(assignment => (assignment?.state === ASSIGNMENT_TYPES.ACCEPTED | ||
&& course.courseRunId.includes(assignment?.contentKey))); | ||
if (isAssigned) { | ||
return { ...course, isCourseAssigned: true }; | ||
} | ||
return course; | ||
}); | ||
|
@@ -108,15 +112,24 @@ | |
</CourseEnrollmentsAlert> | ||
); | ||
} | ||
const handleOnCloseCancelAlert = () => { | ||
setShowCancelledAssignmentsAlert(false); | ||
global.localStorage.setItem(LEARNER_ACKNOWLEDGED_ASSIGNMENT_CANCELLATION_ALERT, new Date()); | ||
}; | ||
|
||
const handleOnCloseExpiredAlert = () => { | ||
setShowCancelledAssignmentsAlert(false); | ||
global.localStorage.setItem(LEARNER_ACKNOWLEDGED_ASSIGNMENT_EXPIRATION_ALERT, new Date()); | ||
Check warning on line 122 in src/components/dashboard/main-content/course-enrollments/CourseEnrollments.jsx Codecov / codecov/patchsrc/components/dashboard/main-content/course-enrollments/CourseEnrollments.jsx#L121-L122
|
||
}; | ||
|
||
const hasCourseEnrollments = Object.values(courseEnrollmentsByStatus).flat().length > 0; | ||
return ( | ||
<> | ||
{features.FEATURE_ENABLE_TOP_DOWN_ASSIGNMENT && showCancelledAssignmentsAlert && ( | ||
<CourseAssignmentAlert variant="cancelled" onClose={() => setShowCancelledAssignmentsAlert(false)}> </CourseAssignmentAlert> | ||
{features.FEATURE_ENABLE_TOP_DOWN_ASSIGNMENT && ( | ||
<CourseAssignmentAlert showAlert={showCancelledAssignmentsAlert} variant="cancelled" onClose={handleOnCloseCancelAlert}> </CourseAssignmentAlert> | ||
)} | ||
{features.FEATURE_ENABLE_TOP_DOWN_ASSIGNMENT && showExpiredAssignmentsAlert && ( | ||
<CourseAssignmentAlert variant="expired" onClose={() => setShowExpiredAssignmentsAlert(false)}> </CourseAssignmentAlert> | ||
{features.FEATURE_ENABLE_TOP_DOWN_ASSIGNMENT && ( | ||
<CourseAssignmentAlert showAlert={showExpiredAssignmentsAlert} variant="expired" onClose={handleOnCloseExpiredAlert}> </CourseAssignmentAlert> | ||
)} | ||
{showMarkCourseCompleteSuccess && ( | ||
<CourseEnrollmentsAlert variant="success" onClose={() => setShowMarkCourseCompleteSuccess(false)}> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit:
getHasActiveExpiredAssignments
as we're working with plural assignments here?