Polish up notification preferences course selector

This change adds grouping (by term) and sorting (by term name, then by
course name) to the course selector on the notification preferences page.

closes LS-2693
flag=notification_settings_course_selector

test plan:
- enable the `notification_settings_course_selector` feature
  - clear your redis cache if necessary
- enroll the current user in several courses, across multiple terms
- visit the account-level notification settings page
- verify that the course selection dropdown groups courses by term
- verify that terms are ordered alphabetically
- verify that courses are ordered alphabetically within their term
- un-publish one of the courses, causing its state to change to something
  other than `available`
- verify it no longer appears in the course selection list

Change-Id: Idfba560934d360649a67caf96e33e1b0ee300a36
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/274998
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jeff Largent <jeff.largent@instructure.com>
QA-Review: Jeff Largent <jeff.largent@instructure.com>
Product-Review: Peyton Craighill <pcraighill@instructure.com>

app/graphql/types/user_type.rb

@@ -87,12 +87,20 @@ module Types
                "only return enrollments for this course",
                required: false,
                prepare: GraphQLHelpers.relay_or_legacy_id_prepare_func("Course")
+      argument :current_only, Boolean,
+               "Whether or not to restrict results to `active` enrollments in `available` courses",
+               required: false
+      argument :order_by, [String],
+               "The fields to order the results by",
+               required: false
     end
 
-    def enrollments(course_id: nil)
+    def enrollments(course_id: nil, current_only: false, order_by: [])
       course_ids = [course_id].compact
       Loaders::UserCourseEnrollmentLoader.for(
-        course_ids: course_ids
+        course_ids: course_ids,
+        order_by: order_by,
+        current_only: current_only
       ).load(object.id).then do |enrollments|
         (enrollments || []).select { |enrollment|
           object == context[:current_user] ||
@@ -316,13 +324,20 @@ end
 
 module Loaders
   class UserCourseEnrollmentLoader < Loaders::ForeignKeyLoader
-    def initialize(course_ids:)
+    def initialize(course_ids:, order_by: [], current_only: false)
       scope = Enrollment.joins(:course)
-                        .where.not(enrollments: { workflow_state: "deleted" })
-                        .where.not(courses: { workflow_state: "deleted" })
+
+      scope = if current_only
+                scope.current.active_by_date
+              else
+                scope.where.not(enrollments: { workflow_state: "deleted" })
+                     .where.not(courses: { workflow_state: "deleted" })
+              end
 
       scope = scope.where(course_id: course_ids) if course_ids.present?
 
+      order_by.each { |o| scope = scope.order(o) }
+
       super(scope, :user_id)
     end
   end

spec/graphql/types/user_type_spec.rb

@@ -205,6 +205,33 @@ describe Types::UserType do
       ).to eq [@student.enrollments.first.to_param]
     end
 
+    it "excludes unavailable courses when currentOnly is true" do
+      @course1.complete
+
+      expect(user_type.resolve("enrollments(currentOnly: true) { _id }")).to eq []
+    end
+
+    it "excludes concluded courses when currentOnly is true" do
+      @course1.start_at = 2.weeks.ago
+      @course1.conclude_at = 1.week.ago
+      @course1.restrict_enrollments_to_course_dates = true
+      @course1.save!
+
+      expect(user_type.resolve("enrollments(currentOnly: true) { _id }")).to eq []
+    end
+
+    it "sorts correctly when orderBy is provided" do
+      @course2.start_at = 1.week.ago
+      @course2.save!
+
+      expect(user_type.resolve('enrollments(orderBy: ["courses.start_at"]) {
+          _id
+          course {
+            _id
+          }
+        }', current_user: @student).map(&:to_i)).to eq [@course2.id, @course1.id]
+    end
+
     it "doesn't return enrollments for courses the user doesn't have permission for" do
       expect(
         user_type.resolve(%|enrollments(courseId: "#{@course2.id}") { _id }|)

ui/features/account_notification_settings/react/__tests__/AccountNotificationSettingsView.test.js

@@ -16,7 +16,7 @@
  * with this program. If not, see <http://www.gnu.org/licenses/>.
  */
 import {createCache} from '@canvas/apollo'
-import {render, screen} from '@testing-library/react'
+import {render, screen, within} from '@testing-library/react'
 import {MockedProvider} from '@apollo/react-testing'
 import mockGraphqlQuery from '@canvas/graphql-query-mock'
 import React from 'react'
@@ -113,13 +113,17 @@ describe('Notification Settings page', () => {
     ).toBeInTheDocument()
   })
 
-  it('does not include inactive enrollments in the dropdown', async () => {
-    const mocks = (await mockAccountNotificationsQuery({Node: {__typename: 'User'}})).concat(
-      await mockUserEnrollmentsQuery({Node: {__typename: 'User'}})
-    )
+  it('displays course terms in the dropdown', async () => {
+    const accountMocks = await mockAccountNotificationsQuery({Node: {__typename: 'User'}})
+    const enrollmentMocks = await mockUserEnrollmentsQuery({Node: {__typename: 'User'}})
+    enrollmentMocks[0].result.data.legacyNode.enrollments.forEach(e => {
+      e.course.name = 'Course Name'
+      e.course.term.name = 'Term Name'
+      e.course.term.id = '1'
+    })
 
     const {findByLabelText} = render(
-      <MockedProvider mocks={mocks} cache={createCache()}>
+      <MockedProvider mocks={accountMocks.concat(enrollmentMocks)} cache={createCache()}>
         <AccountNotificationSettingsView accountId="1" userId="1" courseSelectorEnabled />
       </MockedProvider>
     )
@@ -127,7 +131,11 @@ describe('Notification Settings page', () => {
     const dropdown = await findByLabelText('Settings for')
     userEvent.click(dropdown)
 
-    expect(await screen.queryByText('Hello World')).not.toBeInTheDocument()
+    const terms = await screen.findAllByText('Term Name')
+    expect(terms.length).toEqual(1)
+
+    const termNameGroupQueries = within(terms[0].parentElement)
+    expect((await termNameGroupQueries.findAllByText('Course Name')).length).toEqual(2)
   })
 
   it('only shows a course with multiple enrollments once', async () => {

ui/shared/notification-preferences/graphql/Queries.js

@@ -22,13 +22,15 @@ export const NOTIFICATION_PREFERENCES_CONTEXT_SELECT_QUERY = gql`
     legacyNode(_id: $userId, type: User) {
       ... on User {
         _id
-        enrollments {
+        enrollments(currentOnly: true, orderBy: ["courses.name", "courses.id"]) {
           course {
-            id
             _id
             name
+            term {
+              _id
+              name
+            }
           }
-          state
           type
         }
       }

ui/shared/notification-preferences/react/NotificationPreferencesContextSelect.js

@@ -22,18 +22,15 @@ import {SimpleSelect} from '@instructure/ui-simple-select'
 import {Flex} from '@instructure/ui-flex'
 import {EnrollmentShape} from './Shape'
 import I18n from 'i18n!*'
+import {groupBy, sortBy, sortedUniqBy} from 'lodash'
 
 export default function NotificationPreferencesContextSelect(props) {
-  const activeUniqueEnrollments = useMemo(() => {
-    const courseIds = new Set()
-    return (
-      props.enrollments?.filter(e => {
-        if (e.state !== 'active') return false
-        const duplicate = courseIds.has(e.course.id)
-        courseIds.add(e.course.id)
-        return !duplicate
-      }) || []
-    )
+  const sortedGroupedUniqueEnrollments = useMemo(() => {
+    if (!props.enrollments) return []
+
+    const uniqueEnrollments = sortedUniqBy(props.enrollments, 'course._id')
+    const groupedEnrollments = Object.entries(groupBy(uniqueEnrollments, 'course.term._id'))
+    return sortBy(groupedEnrollments, [([_, e]) => e[0].course.term.name])
   }, [props.enrollments])
 
   const handleChange = useCallback(
@@ -53,10 +50,14 @@ export default function NotificationPreferencesContextSelect(props) {
         <SimpleSelect.Option id="account" value="account">
           {I18n.t('Account')}
         </SimpleSelect.Option>
-        {activeUniqueEnrollments.map(e => (
-          <SimpleSelect.Option key={e.course.id} id={e.course.id} value={e.course._id}>
-            {e.course.name}
-          </SimpleSelect.Option>
+        {sortedGroupedUniqueEnrollments.map(([termId, enrollments]) => (
+          <SimpleSelect.Group renderLabel={enrollments[0].course.term.name} key={termId}>
+            {enrollments.map(e => (
+              <SimpleSelect.Option key={e.course._id} id={e.course._id} value={e.course._id}>
+                {e.course.name}
+              </SimpleSelect.Option>
+            ))}
+          </SimpleSelect.Group>
         ))}
       </SimpleSelect>
     </Flex>

ui/shared/notification-preferences/react/Shape.js

@@ -44,14 +44,18 @@ export const NotificationPreferencesShape = shape({
   channels: arrayOf(ChannelShape)
 })
 
-const CourseShape = shape({
-  id: string.isRequired,
+const TermShape = shape({
   _id: string.isRequired,
   name: string.isRequired
 })
 
+const CourseShape = shape({
+  _id: string.isRequired,
+  name: string.isRequired,
+  term: TermShape.isRequired
+})
+
 export const EnrollmentShape = shape({
   course: CourseShape.isRequired,
-  state: string.isRequired,
   type: string.isRequired
 })