Merge branch 'fix-404-labels-in-todos' into 'master'

Fix 404 page when viewing TODOs that contain milestones or labels in different projects

A user viewing the TODOs page will see a 404 if there are mentioned milestones or labels in multiple different projects. This is likely a caching bug and only occurs
when Markdown rendering occurs across multiple projects, which is why it's so tricky to reproduce. This is what I think is happening:
    
1. LabelReferenceFilter#references_in encounters label ~X for ProjectA and finds the label in the DB as id = 1.
2. LabelReferenceFilter.references_in yields [1, 'X', nil, ...]
3. Since project_ref is nil, AbstractReferenceFilter#project_from_ref_cache caches nil => ProjectA.
4. LabelReferenceFilter#references_in encounters label ~Y for ProjectB and finds the label in the DB as id = 2.
5. LabelReferenceFilter.references_in yields [2, 'Y', nil, ...]
6. AbstractReferenceFilter#project_from_ref_cache lookups nil and returns ProjectA. It was supposed to be ProjectB.
7. A is the wrong project, so the label lookup fails.
   
This MR expands the `project_ref` to the right value as soon as we have it to avoid this caching bug.
    
Closes #17898


See merge request !4312

3 files changed, 47 insertions, 1 deletions

diff --git a/CHANGELOG b/CHANGELOG
index aaec6959a90..d9cf7fd0741 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,5 +1,28 @@
 Please view this file on the master branch, on stable branches it's out of date.
 
+v 8.9.0 (unreleased)
+  - Allow enabling wiki page events from Webhook management UI
+  - Fix wiki page events' webhook to point to the wiki repository
+  - Allow forking projects with restricted visibility level
+  - Improve note validation to prevent errors when creating invalid note via API
+  - Remove project notification settings associated with deleted projects
+  - Fix 404 page when viewing TODOs that contain milestones or labels in different projects
+  - Redesign navigation for project pages
+  - Fix groups API to list only user's accessible projects
+  - Redesign account and email confirmation emails
+  - Use gitlab-shell v3.0.0
+  - Add DB index on users.state
+  - Add rake task 'gitlab:db:configure' for conditionally seeding or migrating the database
+  - Changed the Slack build message to use the singular duration if necessary (Aran Koning)
+  - Fix issues filter when ordering by milestone
+  - Todos will display target state if issuable target is 'Closed' or 'Merged'
+  - Fix bug when sorting issues by milestone due date and filtering by two or more labels
+  - Remove 'main language' feature
+  - Projects pending deletion will render a 404 page
+  - Measure queue duration between gitlab-workhorse and Rails
+  - Make authentication service for Container Registry to be compatible with < Docker 1.11
+  - Add Application Setting to configure Container Registry token expire delay (default 5min)
+
 v 8.8.3
   - Fixed JS error when trying to remove discussion form. !4303
   - Fixed issue with button color when no CI enabled. !4287
diff --git a/lib/banzai/filter/abstract_reference_filter.rb b/lib/banzai/filter/abstract_reference_filter.rb
index b8962379cb5..65283fe96a4 100644
--- a/lib/banzai/filter/abstract_reference_filter.rb
+++ b/lib/banzai/filter/abstract_reference_filter.rb
@@ -236,7 +236,9 @@ module Banzai
         if cache.key?(key)
           cache[key]
         else
-          cache[key] = yield
+          value = yield
+          cache[key] = value if key.present?
+          value
         end
       end
     end
diff --git a/spec/features/todos/todos_spec.rb b/spec/features/todos/todos_spec.rb
index 3354f529295..4e627753cc7 100644
--- a/spec/features/todos/todos_spec.rb
+++ b/spec/features/todos/todos_spec.rb
@@ -43,6 +43,27 @@ describe 'Dashboard Todos', feature: true do
       end
     end
 
+    context 'User has Todos with labels spanning multiple projects' do
+      before do
+        label1 = create(:label, project: project)
+        note1 = create(:note_on_issue, note: "Hello #{label1.to_reference(format: :name)}", noteable_id: issue.id, noteable_type: 'Issue', project: issue.project)
+        create(:todo, :mentioned, project: project, target: issue, user: user, note_id: note1.id)
+
+        project2 = create(:project)
+        label2 = create(:label, project: project2)
+        issue2 = create(:issue, project: project2)
+        note2 = create(:note_on_issue, note: "Test #{label2.to_reference(format: :name)}", noteable_id: issue2.id, noteable_type: 'Issue', project: project2)
+        create(:todo, :mentioned, project: project2, target: issue2, user: user, note_id: note2.id)
+
+        login_as(user)
+        visit dashboard_todos_path
+      end
+
+      it 'shows page with two Todos' do
+        expect(page).to have_selector('.todos-list .todo', count: 2)
+      end
+    end
+
     context 'User has multiple pages of Todos' do
       before do
         allow(Todo).to receive(:default_per_page).and_return(1)