Show only outcomes with alignments at the course level

closes OUT-5237 flag=outcome_alignment_summary Test plan: - Enable Improved Outcomes Management FF - Enable Outcome Alignment Summary FF - Go to Account > Outcomes - Create outcome and rubric within the account and align them - Go to Course > Outcomes - Import the aligned account outcome created above in the course - Create two outcomes and rubric within the course and align one of the outcomes to the rubric - Go to Course > Alignments and select filter "With Alignments" - Verify that only the aligned course outcome is listed - Select filter "Without Alignments" - Verify that only the aligned account outcome and the not-aligned course outcome are listed Change-Id: I752ac4a4d63113fce0e96d71c769abaef1c8ba66 Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/298505 Tested-by: Service Cloud Jenkins <[email protected]> Product-Review: Kyle Rosenbaum <[email protected]> Reviewed-by: Angela Gomba <[email protected]> Reviewed-by: Chrystal Langston <[email protected]> QA-Review: Dave Wenzlick <[email protected]>
nehalsep7 · Aug 17, 2022 · cfd674e · cfd674e
1 parent bac95e5
commit cfd674e
Show file tree

Hide file tree

Showing 3 changed files with 71 additions and 36 deletions.
diff --git a/lib/outcomes/learning_outcome_group_children.rb b/lib/outcomes/learning_outcome_group_children.rb
@@ -81,7 +81,7 @@ def not_imported_outcomes(learning_outcome_group_id, args = {})
 
     def suboutcomes_by_group_id(learning_outcome_group_id, args = {})
       relation = outcome_links(learning_outcome_group_id)
-      relation = filter_outcomes(relation, args[:filter]) unless args[:filter].nil? || !outcome_alignment_summary_enabled?(@context)
+      relation = filter_outcomes(relation, args[:filter])
       relation = relation.joins(:learning_outcome_content)
                          .joins("INNER JOIN #{LearningOutcomeGroup.quoted_table_name} AS logs
               ON logs.id = content_tags.associated_asset_id")
@@ -122,18 +122,26 @@ def outcome_links(learning_outcome_group_id)
     end
 
     def filter_outcomes(relation, filter)
-      outcomes = LearningOutcome.preload(:alignments).where(id: relation.map(&:content_id))
-      filtered_tag_ids = if filter == "WITH_ALIGNMENTS"
-                           relation.reject { |tag| outcomes.find(tag.content_id).alignments.empty? }.map(&:id)
-                         elsif filter == "NO_ALIGNMENTS"
-                           relation.select { |tag| outcomes.find(tag.content_id).alignments.empty? }.map(&:id)
-                         end
-      filtered_tag_ids.nil? ? relation : relation.where(id: filtered_tag_ids)
+      if %w[WITH_ALIGNMENTS NO_ALIGNMENTS].include?(filter) && outcome_alignment_summary_enabled?(@context)
+        outcomes_with_alignments_in_context = ContentTag
+                                              .not_deleted
+                                              .where(
+                                                tag_type: "learning_outcome",
+                                                context: @context
+                                              )
+                                              .map(&:learning_outcome_id)
+                                              .uniq
+
+        return relation.where(content_id: outcomes_with_alignments_in_context) if filter == "WITH_ALIGNMENTS"
+        return relation.where.not(content_id: outcomes_with_alignments_in_context) if filter == "NO_ALIGNMENTS"
+      end
+
+      relation
     end
 
     def total_outcomes_for(learning_outcome_group_id, args = {})
       relation = outcome_links(learning_outcome_group_id)
-      relation = filter_outcomes(relation, args[:filter]) unless args[:filter].nil? || !outcome_alignment_summary_enabled?(@context)
+      relation = filter_outcomes(relation, args[:filter])
 
       if args[:search_query]
         relation = relation.joins(:learning_outcome_content)

diff --git a/spec/graphql/types/learning_outcome_group_type_spec.rb b/spec/graphql/types/learning_outcome_group_type_spec.rb
@@ -73,23 +73,29 @@
                                                                                                                                       ])
   end
 
-  it "accepts filter in outcomes" do
-    course = Course.create!
-    course.account.enable_feature!(:outcome_alignment_summary)
-    @outcome2.align(assignment_model, course)
-    expect(outcome_group_type.resolve("outcomes(filter: \"WITH_ALIGNMENTS\") { nodes { ... on LearningOutcome { _id } } }")).to match_array([
-                                                                                                                                              @outcome2.id.to_s
-                                                                                                                                            ])
-  end
+  describe "within course context" do
+    before do
+      @course1 = Course.create!
+      @course_outcome1 = outcome_model(context: @course1, short_description: "CCC")
+      @course_outcome2 = outcome_model(context: @course1, short_description: "DDD")
+      @course_outcome2.align(assignment_model, @course1)
+      @course1.account.enable_feature!(:outcome_alignment_summary)
+    end
 
-  it "accepts both search_query and filter in outcomes" do
-    course = Course.create!
-    course.account.enable_feature!(:outcome_alignment_summary)
-    @outcome1.align(assignment_model, course)
-    @outcome2.align(assignment_model, course)
-    expect(outcome_group_type.resolve("outcomes(filter: \"WITH_ALIGNMENTS\", searchQuery: \"BBBB\") { nodes { ... on LearningOutcome { _id } } }")).to match_array([
-                                                                                                                                                                     @outcome1.id.to_s
-                                                                                                                                                                   ])
+    let(:course_outcome_group_type) { GraphQLTypeTester.new(@course1.root_outcome_group, current_user: @admin) }
+
+    it "accepts filter in outcomes" do
+      expect(course_outcome_group_type.resolve("outcomes(filter: \"WITH_ALIGNMENTS\") { nodes { ... on LearningOutcome { _id } } }")).to match_array([
+                                                                                                                                                       @course_outcome2.id.to_s
+                                                                                                                                                     ])
+    end
+
+    it "accepts both search_query and filter in outcomes" do
+      @course_outcome1.align(assignment_model, @course1)
+      expect(course_outcome_group_type.resolve("outcomes(filter: \"WITH_ALIGNMENTS\", searchQuery: \"CCC\") { nodes { ... on LearningOutcome { _id } } }")).to match_array([
+                                                                                                                                                                             @course_outcome1.id.to_s
+                                                                                                                                                                           ])
+    end
   end
 
   it "returns isImported for a given context" do

diff --git a/spec/lib/outcomes/learning_outcome_group_children_spec.rb b/spec/lib/outcomes/learning_outcome_group_children_spec.rb
@@ -182,24 +182,43 @@
     end
 
     context "when filter arg is used" do
+      subject { described_class.new(course) }
+
       before do
         course.account.enable_feature!(:outcome_alignment_summary)
-        o3.align(assignment_model, course)
+        cg2.add_outcome o7
       end
 
-      it "returns the total outcomes based on filter argument" do
-        expect(subject.total_outcomes(cg1.id, { filter: "WITH_ALIGNMENTS" })).to eq 1
-        expect(subject.total_outcomes(cg1.id, { filter: "NO_ALIGNMENTS" })).to eq 1
-      end
+      context "when outcomes are aligned in course context" do
+        before do
+          o3.align(assignment_model, course)
+        end
+
+        it "returns the total outcomes based on filter argument" do
+          expect(subject.total_outcomes(cg0.id, { filter: "WITH_ALIGNMENTS" })).to eq 1
+          expect(subject.total_outcomes(cg0.id, { filter: "NO_ALIGNMENTS" })).to eq 2
+        end
+
+        it "returns the total outcomes if filter arg isn't passed in" do
+          expect(subject.total_outcomes(cg0.id, {})).to eq 3
+        end
 
-      it "returns the total outcomes if filter arg isn't passed in" do
-        expect(subject.total_outcomes(cg1.id, {})).to eq 2
+        it "returns the total outcomes without filtering if the FF is disabled" do
+          course.account.disable_feature!(:outcome_alignment_summary)
+          expect(subject.total_outcomes(cg0.id, { filter: "WITH_ALIGNMENTS" })).to eq 3
+          expect(subject.total_outcomes(cg0.id, { filter: "NO_ALIGNMENTS" })).to eq 3
+        end
       end
 
-      it "returns the total outcomes without filtering if the FF is disabled" do
-        course.account.disable_feature!(:outcome_alignment_summary)
-        expect(subject.total_outcomes(cg1.id, { filter: "WITH_ALIGNMENTS" })).to eq 2
-        expect(subject.total_outcomes(cg1.id, { filter: "NO_ALIGNMENTS" })).to eq 2
+      context "when outcomes are aligned in account context and imported in a course" do
+        before do
+          o3.align(assignment_model, context)
+        end
+
+        it "returns the correct number of total outcomes based on filter argument" do
+          expect(subject.total_outcomes(cg0.id, { filter: "WITH_ALIGNMENTS" })).to eq 0
+          expect(subject.total_outcomes(cg0.id, { filter: "NO_ALIGNMENTS" })).to eq 3
+        end
       end
     end
   end
@@ -488,6 +507,8 @@
     end
 
     context "filter" do
+      subject { described_class.new(course) }
+
       before do
         course.account.enable_feature!(:outcome_alignment_summary)
         o3.align(assignment_model, course)