scope comment library to users rather than course + user

closes EVAL-1677

flag=assignment_comment_library

test-plan:
- enable feature flag
- launch speedgrader for a course
- add a comment to the comment library
- open speedgrader in another course
- verify the comment you added before is shown
- verify deleting a comment removes it from the library
and the comment is no longer visible in either course

Change-Id: Iaa1c5f78f8ff171c8fbba787241b85589c252a1e
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/264553
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Adrian Packel <apackel@instructure.com>
Reviewed-by: Spencer Olson <solson@instructure.com>
QA-Review: Pablo Gomez <pablo.gomez@instructure.com>
Product-Review: Jody Sailor

app/graphql/types/course_type.rb

@@ -106,17 +106,6 @@ module Types
       course.resolved_outcome_proficiency
     end
 
-    field :comment_bank_items_connection, CommentBankItemType.connection_type, null: true do
-      argument :query, String, <<~DOC, required: false
-        Only include comments that match the query string.
-      DOC
-    end
-    def comment_bank_items_connection(query: nil)
-      comments = course.comment_bank_items_visible_to(current_user)
-      comments = comments.where(ActiveRecord::Base.wildcard("comment", query)) if query
-      comments
-    end
-
     # field :proficiency_ratings_connection, ProficiencyRatingType.connection_type, null: true
     # def proficiency_ratings_connection
     #   # This does a recursive lookup of parent accounts, not sure how we could

app/graphql/types/user_type.rb

@@ -288,6 +288,19 @@ module Types
         end
       end
     end
+
+    field :comment_bank_items_connection, Types::CommentBankItemType.connection_type, null: true do
+      argument :query, String, <<~DOC, required: false
+        Only include comments that match the query string.
+      DOC
+    end
+    def comment_bank_items_connection(query: nil)
+      return unless object == current_user
+
+      comments = current_user.comment_bank_items
+      comments = comments.where(ActiveRecord::Base.wildcard("comment", query.strip)) if query&.strip.present?
+      comments
+    end
   end
 end
 

app/models/course.rb

@@ -3846,10 +3846,6 @@ class Course < ActiveRecord::Base
     !templated_accounts.exists?
   end
 
-  def comment_bank_items_visible_to(user)
-    comment_bank_items.active.where(user: user)
-  end
-
   private
 
   def effective_due_dates

app/models/user.rb

@@ -204,6 +204,8 @@ class User < ActiveRecord::Base
     dependent: :destroy,
     inverse_of: :grader
 
+  has_many :comment_bank_items, -> { where("workflow_state<>'deleted'") }
+
   belongs_to :otp_communication_channel, :class_name => 'CommunicationChannel'
 
   belongs_to :merged_into_user, class_name: 'User'

spec/graphql/types/course_type_spec.rb

@@ -532,30 +532,6 @@ describe Types::CourseType do
     end
   end
 
-  describe "CommentBankItemsConnection" do
-    before do
-      @comment_bank_item = comment_bank_item_model(user: @teacher, context: @course, comment: 'great comment!')
-    end
-
-    it "returns comment bank items" do
-      expect(
-        course_type.resolve("commentBankItemsConnection { nodes { _id } }", current_user: @teacher)
-      ).to eq [@comment_bank_item.id.to_s]
-    end
-
-    describe "with a search query" do
-      before do
-        @comment_bank_item2 = comment_bank_item_model(user: @teacher, context: @course, comment: 'new comment!')
-      end
-
-      it "returns results that match the query" do
-        expect(
-          course_type.resolve("commentBankItemsConnection(query: \"new\") { nodes { _id } }", current_user: @teacher).length
-        ).to eq 1
-      end
-    end
-  end
-
   describe 'Account' do
     it 'works' do
       expect(course_type.resolve("account { _id }")).to eq course.account.id.to_s

spec/graphql/types/user_type_spec.rb

@@ -514,4 +514,49 @@ describe Types::UserType do
       expect(result).to match_array([@group.id.to_s])
     end
   end
+
+  context 'CommentBankItemsConnection' do
+    before do
+      @comment_bank_item = comment_bank_item_model(user: @teacher, context: @course, comment: 'great comment!')
+    end
+
+    let(:type) do
+      GraphQLTypeTester.new(
+        @teacher,
+        current_user: @teacher,
+        domain_root_account: @course.account.root_account,
+        request: ActionDispatch::TestRequest.create
+      )
+    end
+
+    it 'returns comment bank items for the queried user' do
+      expect(
+        type.resolve('commentBankItemsConnection { nodes { _id } }')
+      ).to eq [@comment_bank_item.id.to_s]
+    end
+
+    describe 'with a search query' do
+      before do
+        @comment_bank_item2 = comment_bank_item_model(user: @teacher, context: @course, comment: 'new comment!')
+      end
+
+      it 'returns results that match the query' do
+        expect(
+          type.resolve("commentBankItemsConnection(query: \"new\") { nodes { _id } }").length
+        ).to eq 1
+      end
+
+      it 'strips leading/trailing white space' do
+        expect(
+          type.resolve("commentBankItemsConnection(query: \"    new   \") { nodes { _id } }").length
+        ).to eq 1
+      end
+
+      it 'does not query results if query.strip is blank' do
+        expect(
+          type.resolve("commentBankItemsConnection(query: \"  \") { nodes { _id } }").length
+        ).to eq 2
+      end
+    end
+  end
 end

spec/models/course_spec.rb

@@ -1817,25 +1817,6 @@ describe Course do
       end
     end
   end
-
-  describe "comment_bank_items_visible_to" do
-    before do
-      @course = course_factory(active_all: true)
-      @user1 = user_model
-      @user2 = user_model
-      @item = comment_bank_item_model(course: @course, user: @user1)
-    end
-
-    it "should return items visible to the provided user" do
-      expect(@course.comment_bank_items_visible_to(@user2)).to eq []
-      expect(@course.comment_bank_items_visible_to(@user1)).to eq [@item]
-    end
-
-    it "should only return active records" do
-      @item.destroy
-      expect(@course.comment_bank_items_visible_to(@user1)).to eq []
-    end
-  end
 end
 
 

spec/models/user_spec.rb

@@ -3461,4 +3461,17 @@ describe User do
       expect(@ta.can_create_enrollment_for?(@course, nil, 'ObserverEnrollment')).to be_truthy
     end
   end
+
+  describe "comment_bank_items" do
+    before(:once) do
+      course_with_teacher
+      @c1 = comment_bank_item_model({user: @teacher})
+      @c2 = comment_bank_item_model({user: @teacher})
+    end
+
+    it "only returns active records" do
+      @c2.destroy
+      expect(@teacher.comment_bank_items).to eq [@c1]
+    end
+  end
 end

ui/features/speed_grader/jquery/speed_grader.js

@@ -660,7 +660,11 @@ function renderCommentTextArea() {
   }
 
   ReactDOM.render(
-    <CommentArea getTextAreaRef={getTextAreaRef} courseId={ENV.course_id} />,
+    <CommentArea
+      getTextAreaRef={getTextAreaRef}
+      courseId={ENV.course_id}
+      userId={ENV.current_user_id}
+    />,
     document.getElementById(SPEED_GRADER_COMMENT_TEXTAREA_MOUNT_POINT)
   )
 }

ui/features/speed_grader/react/CommentArea.js

@@ -31,7 +31,7 @@ const textAreaProps = {
   resize: 'vertical'
 }
 
-export default function CommentArea({getTextAreaRef, courseId}) {
+export default function CommentArea({getTextAreaRef, courseId, userId}) {
   const [comment, setComment] = useState('')
   const textAreaRef = useRef()
   const showCommentLibrary = ENV.assignment_comment_library_feature_enabled
@@ -44,7 +44,12 @@ export default function CommentArea({getTextAreaRef, courseId}) {
   return (
     <>
       {showCommentLibrary && (
-        <CommentLibrary textAreaRef={textAreaRef} setComment={setComment} courseId={courseId} />
+        <CommentLibrary
+          textAreaRef={textAreaRef}
+          setComment={setComment}
+          courseId={courseId}
+          userId={userId}
+        />
       )}
       <TextArea
         value={comment}
@@ -58,5 +63,6 @@ export default function CommentArea({getTextAreaRef, courseId}) {
 
 CommentArea.propTypes = {
   getTextAreaRef: PropTypes.func.isRequired,
-  courseId: PropTypes.string.isRequired
+  courseId: PropTypes.string.isRequired,
+  userId: PropTypes.string.isRequired
 }

ui/features/speed_grader/react/CommentLibrary/LibraryManager.js

@@ -27,10 +27,10 @@ import {COMMENTS_QUERY} from './graphql/Queries'
 import I18n from 'i18n!CommentLibrary'
 import Library from './Library'
 
-const LibraryManager = ({setComment, courseId, textAreaRef}) => {
+const LibraryManager = ({setComment, courseId, textAreaRef, userId}) => {
   const [removedItemIndex, setRemovedItemIndex] = useState(null)
   const {loading, error, data} = useQuery(COMMENTS_QUERY, {
-    variables: {courseId}
+    variables: {userId}
   })
 
   useEffect(() => {
@@ -47,7 +47,7 @@ const LibraryManager = ({setComment, courseId, textAreaRef}) => {
       JSON.stringify(
         cache.readQuery({
           query: COMMENTS_QUERY,
-          variables: {courseId}
+          variables: {userId}
         })
       )
     )
@@ -56,7 +56,7 @@ const LibraryManager = ({setComment, courseId, textAreaRef}) => {
   const writeComments = (cache, comments) => {
     cache.writeQuery({
       query: COMMENTS_QUERY,
-      variables: {courseId},
+      variables: {userId},
       data: comments
     })
   }
@@ -64,14 +64,14 @@ const LibraryManager = ({setComment, courseId, textAreaRef}) => {
   const removeDeletedCommentFromCache = (cache, result) => {
     const commentsFromCache = getCachedComments(cache)
     const resultId = result.data.deleteCommentBankItem.commentBankItemId
-    const removedIndex = commentsFromCache.course.commentBankItemsConnection.nodes.findIndex(
+    const removedIndex = commentsFromCache.legacyNode.commentBankItemsConnection.nodes.findIndex(
       comment => comment._id === resultId
     )
-    const updatedComments = commentsFromCache.course.commentBankItemsConnection.nodes.filter(
+    const updatedComments = commentsFromCache.legacyNode.commentBankItemsConnection.nodes.filter(
       (_comment, index) => index !== removedIndex
     )
 
-    commentsFromCache.course.commentBankItemsConnection.nodes = updatedComments
+    commentsFromCache.legacyNode.commentBankItemsConnection.nodes = updatedComments
     writeComments(cache, commentsFromCache)
     setRemovedItemIndex(removedIndex)
   }
@@ -80,11 +80,11 @@ const LibraryManager = ({setComment, courseId, textAreaRef}) => {
     const commentsFromCache = getCachedComments(cache)
     const newComment = result.data.createCommentBankItem.commentBankItem
     const updatedComments = [
-      ...commentsFromCache.course.commentBankItemsConnection.nodes,
+      ...commentsFromCache.legacyNode.commentBankItemsConnection.nodes,
       newComment
     ]
 
-    commentsFromCache.course.commentBankItemsConnection.nodes = updatedComments
+    commentsFromCache.legacyNode.commentBankItemsConnection.nodes = updatedComments
     writeComments(cache, commentsFromCache)
   }
 
@@ -143,12 +143,11 @@ const LibraryManager = ({setComment, courseId, textAreaRef}) => {
 
   return (
     <Library
-      comments={data?.course?.commentBankItemsConnection?.nodes || []}
+      comments={data?.legacyNode?.commentBankItemsConnection?.nodes || []}
       setComment={handleSetComment}
       onAddComment={handleAddComment}
       onDeleteComment={id => deleteComment({variables: {id}})}
       isAddingComment={isAddingComment}
-      courseId={courseId}
       removedItemIndex={removedItemIndex}
     />
   )
@@ -159,7 +158,8 @@ LibraryManager.propTypes = {
   courseId: PropTypes.string.isRequired,
   textAreaRef: shape({
     current: instanceOf(Element)
-  }).isRequired
+  }).isRequired,
+  userId: PropTypes.string.isRequired
 }
 
 export default LibraryManager

ui/features/speed_grader/react/CommentLibrary/__tests__/LibraryManager.test.js

@@ -33,6 +33,7 @@ describe('LibraryManager', () => {
       setComment: () => {},
       courseId: '1',
       textAreaRef: {current: inputRef},
+      userId: '1',
       ...props
     }
   }

ui/features/speed_grader/react/CommentLibrary/__tests__/mocks.js

@@ -20,18 +20,18 @@ import mockGraphqlQuery from '@canvas/graphql-query-mock'
 import {DELETE_COMMENT_MUTATION, CREATE_COMMENT_MUTATION} from '../graphql/Mutations'
 import {COMMENTS_QUERY} from '../graphql/Queries'
 
-export const commentBankItemMocks = ({courseId = '1', numberOfComments = 10} = {}) => [
+export const commentBankItemMocks = ({userId = '1', numberOfComments = 10} = {}) => [
   {
     request: {
       query: COMMENTS_QUERY,
       variables: {
-        courseId
+        userId
       }
     },
     result: {
       data: {
-        course: {
-          __typename: 'Course',
+        legacyNode: {
+          __typename: 'User',
           commentBankItemsConnection: {
             __typename: 'CommentBankItemsConnection',
             nodes: new Array(numberOfComments).fill(0).map((_v, i) => ({

ui/features/speed_grader/react/CommentLibrary/graphql/Queries.js

@@ -19,12 +19,14 @@
 import {gql} from '@canvas/apollo'
 
 export const COMMENTS_QUERY = gql`
-  query CommentBankItemQuery($courseId: ID!) {
-    course(id: $courseId) {
-      commentBankItemsConnection {
-        nodes {
-          comment
-          _id
+  query CommentBankItemQuery($userId: ID!) {
+    legacyNode(_id: $userId, type: User) {
+      ... on User {
+        commentBankItemsConnection {
+          nodes {
+            comment
+            _id
+          }
         }
       }
     }

ui/features/speed_grader/react/CommentLibrary/index.js

@@ -23,10 +23,15 @@ import LibraryManager from './LibraryManager'
 
 const client = createClient()
 
-export default function CommentLibrary({setComment, courseId, textAreaRef}) {
+export default function CommentLibrary({setComment, courseId, textAreaRef, userId}) {
   return (
     <ApolloProvider client={client}>
-      <LibraryManager setComment={setComment} courseId={courseId} textAreaRef={textAreaRef} />
+      <LibraryManager
+        setComment={setComment}
+        courseId={courseId}
+        textAreaRef={textAreaRef}
+        userId={userId}
+      />
     </ApolloProvider>
   )
 }
@@ -36,5 +41,6 @@ CommentLibrary.propTypes = {
   courseId: PropTypes.string.isRequired,
   textAreaRef: shape({
     current: instanceOf(Element)
-  }).isRequired
+  }).isRequired,
+  userId: PropTypes.string.isRequired
 }

ui/features/speed_grader/react/__tests__/CommentArea.test.js

@@ -26,7 +26,8 @@ describe('CommentArea', () => {
   const defaultProps = () => {
     return {
       getTextAreaRef: getTextAreaRefMock,
-      courseId: '1'
+      courseId: '1',
+      userId: '1'
     }
   }