feat: dismiss alert modal and cancelled assignments

[katrinan029]

Description:

We want to hide cancelled assignment course cards and when a user dismisses an cancellation alert, they should not see the alert again. This is a temporary fix to hide the alert based on local storage.

Screen.Recording.2023-12-14.at.10.11.22.AM.mov

JIRA Ticket

For all changes

• Ensure adequate tests are in place (or reviewed existing tests cover changes)

Only if submitting a visual change

• Ensure to attach screenshots
• Ensure to have UX team confirm screenshots

[iloveagent57]

@adamstankiewicz any concerns with changing this definition of getActiveAssignments()?

[adamstankiewicz]

@iloveagent57 @katrinan029 Yes, by removing ASSIGNMENT_TYPES.CANCELLED here, the CourseEnrollments components that check against "cancelled" assignment states would no longer be provided any canceled assignments to process (i.e., I don't believe the util functions (e.g., getIsActiveCancelledAssignment) would be executed? 🤔 For example, trying to run this locally, I couldn't get the canceled alert to appear unless canceled assignments were considered here.

I might recommend modifying getActiveAssignments to split out arrays for each assignment lifecycle state that's relevant, which would enable more granular control over which types of assignments you're working with in CourseEnrollments, i.e.:

// 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 getRedeemablePoliciesData at this point (as it is on master) as that logic has already been executed further up in the component tree, without needing to run again to get similar results.

[codecov]

Codecov Report

Attention: 145 lines in your changes are missing coverage. Please review.

Comparison is base (d36b78a) 84.87% compared to head (c8e10a5) 85.00%.
Report is 111 commits behind head on master.

Files	Patch %	Lines
src/components/search/Search.jsx	0.00%	29 Missing ⚠️
src/components/dashboard/data/utils.js	26.31%	12 Missing and 2 partials ⚠️
...course-enrollments/course-cards/BaseCourseCard.jsx	89.85%	7 Missing ⚠️
src/components/my-career/VisualizeCareer.jsx	74.07%	6 Missing and 1 partial ⚠️
src/components/my-career/data/service.js	56.25%	7 Missing ⚠️
src/components/course/data/utils.jsx	95.89%	6 Missing ⚠️
src/components/my-career/AddJobRole.jsx	60.00%	5 Missing and 1 partial ⚠️
src/components/my-career/data/hooks.jsx	50.00%	6 Missing ⚠️
...rc/components/search/AssignmentsOnlyEmptyState.jsx	0.00%	6 Missing ⚠️
...components/enterprise-user-subsidy/data/service.js	0.00%	5 Missing ⚠️
... and 28 more

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #900      +/-   ##
==========================================
+ Coverage   84.87%   85.00%   +0.12%     
==========================================
  Files         320      337      +17     
  Lines        6399     7120     +721     
  Branches     1552     1743     +191     
==========================================
+ Hits         5431     6052     +621     
- Misses        941     1035      +94     
- Partials       27       33       +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[adamstankiewicz]

Note: the cron job that runs to automatically cancel assignments will result in the assignment(s) being marked as state=cancelled, which means those assignment cards would actually render as the canceled assignment card variant instead of the expired variant for assignments where the cron job already ran.

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 isAssignmentExpired, i.e. are there expired assignments that haven't been dismissed with an assignment enrollment deadline that is more recent than the last dismissed expired assignments?

[idea] Refactor isAssignmentExpired slightly to return both an isExpired boolean but also the enrollment deadline for the assignment and then check the localStorage date against this returned enrollment deadline.

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;
}

[iloveagent57]

This is a good idea for now, but really we should introduce an expired state on the backend - and then the frontend could determine if an assignment is expired by checking assignment.state === 'expired'.

[adamstankiewicz]

nit: getHasActiveCancelledAssignments as we're working with plural assignments here?

[adamstankiewicz]

nit: getHasActiveExpiredAssignments as we're working with plural assignments here?

[adamstankiewicz]

@iloveagent57 @katrinan029 Yes, by removing ASSIGNMENT_TYPES.CANCELLED here, the CourseEnrollments components that check against "cancelled" assignment states would no longer be provided any canceled assignments to process (i.e., I don't believe the util functions (e.g., getIsActiveCancelledAssignment) would be executed? 🤔 For example, trying to run this locally, I couldn't get the canceled alert to appear unless canceled assignments were considered here.

I might recommend modifying getActiveAssignments to split out arrays for each assignment lifecycle state that's relevant, which would enable more granular control over which types of assignments you're working with in CourseEnrollments, i.e.:

// 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 getRedeemablePoliciesData at this point (as it is on master) as that logic has already been executed further up in the component tree, without needing to run again to get similar results.

[adamstankiewicz]

Good call to make this alert component act as a controlled component instead of uncontrolled.

[adamstankiewicz]

Related to another comment, I believe you could move the call to the (refactored) isAssignmentExpired into getIsActiveExpiredAssignment itself such that it has access to the proposed enrollByDeadline returned by isAssignmentExpired to compare against the localStorage timestamp value.

[adamstankiewicz]

[suggestion] I might propose a bit of a further refactor to this useEffect to clean it up a bit:

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 assignments instead of assignedCourses when passing them as the course runs to the assigned CourseSection component (i.e., the call to getTransformedAllocatedAssignments was moved within the useEffect).

[adamstankiewicz]

Closing this PR in favor of #910 (same changes, just extended / finished up on my own branch to not mess with this one).